Commit 94c3e614 authored by Sara Sharon's avatar Sara Sharon Committed by Luca Coelho

iwlwifi: mvm: fix pending frame counter calculation

In DQA mode the check whether to decrement the pending frames
counter relies on the tid status and not on the txq id.
This may result in an inconsistent state of the pending frames
counter in case frame is queued on a non aggregation queue but
with this TID, and will be followed by a failure to remove the
station and later on SYSASSERT 0x3421 when trying to remove the
MAC.
Such frames are for example bar and qos NDPs.
Fix it by aligning the condition of incrementing the counter
with the condition of decrementing it - rely on TID state for
DQA mode.
Also, avoid internal error like this affecting station removal
for DQA mode - since we can know for sure it is an internal
error.

Fixes: cf961e16 ("iwlwifi: mvm: support dqa-mode agg on non-shared queue")
Signed-off-by: default avatarSara Sharon <sara.sharon@intel.com>
Signed-off-by: default avatarLuca Coelho <luciano.coelho@intel.com>
parent 2c6262b7
...@@ -1501,6 +1501,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm, ...@@ -1501,6 +1501,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm,
{ {
struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(vif); struct iwl_mvm_vif *mvmvif = iwl_mvm_vif_from_mac80211(vif);
struct iwl_mvm_sta *mvm_sta = iwl_mvm_sta_from_mac80211(sta); struct iwl_mvm_sta *mvm_sta = iwl_mvm_sta_from_mac80211(sta);
u8 sta_id = mvm_sta->sta_id;
int ret; int ret;
lockdep_assert_held(&mvm->mutex); lockdep_assert_held(&mvm->mutex);
...@@ -1509,7 +1510,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm, ...@@ -1509,7 +1510,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm,
kfree(mvm_sta->dup_data); kfree(mvm_sta->dup_data);
if ((vif->type == NL80211_IFTYPE_STATION && if ((vif->type == NL80211_IFTYPE_STATION &&
mvmvif->ap_sta_id == mvm_sta->sta_id) || mvmvif->ap_sta_id == sta_id) ||
iwl_mvm_is_dqa_supported(mvm)){ iwl_mvm_is_dqa_supported(mvm)){
ret = iwl_mvm_drain_sta(mvm, mvm_sta, true); ret = iwl_mvm_drain_sta(mvm, mvm_sta, true);
if (ret) if (ret)
...@@ -1525,8 +1526,17 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm, ...@@ -1525,8 +1526,17 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm,
ret = iwl_mvm_drain_sta(mvm, mvm_sta, false); ret = iwl_mvm_drain_sta(mvm, mvm_sta, false);
/* If DQA is supported - the queues can be disabled now */ /* If DQA is supported - the queues can be disabled now */
if (iwl_mvm_is_dqa_supported(mvm)) if (iwl_mvm_is_dqa_supported(mvm)) {
iwl_mvm_disable_sta_queues(mvm, vif, mvm_sta); iwl_mvm_disable_sta_queues(mvm, vif, mvm_sta);
/*
* If pending_frames is set at this point - it must be
* driver internal logic error, since queues are empty
* and removed successuly.
* warn on it but set it to 0 anyway to avoid station
* not being removed later in the function
*/
WARN_ON(atomic_xchg(&mvm->pending_frames[sta_id], 0));
}
/* If there is a TXQ still marked as reserved - free it */ /* If there is a TXQ still marked as reserved - free it */
if (iwl_mvm_is_dqa_supported(mvm) && if (iwl_mvm_is_dqa_supported(mvm) &&
...@@ -1544,7 +1554,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm, ...@@ -1544,7 +1554,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm,
if (WARN((*status != IWL_MVM_QUEUE_RESERVED) && if (WARN((*status != IWL_MVM_QUEUE_RESERVED) &&
(*status != IWL_MVM_QUEUE_FREE), (*status != IWL_MVM_QUEUE_FREE),
"sta_id %d reserved txq %d status %d", "sta_id %d reserved txq %d status %d",
mvm_sta->sta_id, reserved_txq, *status)) { sta_id, reserved_txq, *status)) {
spin_unlock_bh(&mvm->queue_info_lock); spin_unlock_bh(&mvm->queue_info_lock);
return -EINVAL; return -EINVAL;
} }
...@@ -1554,7 +1564,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm, ...@@ -1554,7 +1564,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm,
} }
if (vif->type == NL80211_IFTYPE_STATION && if (vif->type == NL80211_IFTYPE_STATION &&
mvmvif->ap_sta_id == mvm_sta->sta_id) { mvmvif->ap_sta_id == sta_id) {
/* if associated - we can't remove the AP STA now */ /* if associated - we can't remove the AP STA now */
if (vif->bss_conf.assoc) if (vif->bss_conf.assoc)
return ret; return ret;
...@@ -1563,7 +1573,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm, ...@@ -1563,7 +1573,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm,
mvmvif->ap_sta_id = IWL_MVM_STATION_COUNT; mvmvif->ap_sta_id = IWL_MVM_STATION_COUNT;
/* clear d0i3_ap_sta_id if no longer relevant */ /* clear d0i3_ap_sta_id if no longer relevant */
if (mvm->d0i3_ap_sta_id == mvm_sta->sta_id) if (mvm->d0i3_ap_sta_id == sta_id)
mvm->d0i3_ap_sta_id = IWL_MVM_STATION_COUNT; mvm->d0i3_ap_sta_id = IWL_MVM_STATION_COUNT;
} }
} }
...@@ -1572,7 +1582,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm, ...@@ -1572,7 +1582,7 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm,
* This shouldn't happen - the TDLS channel switch should be canceled * This shouldn't happen - the TDLS channel switch should be canceled
* before the STA is removed. * before the STA is removed.
*/ */
if (WARN_ON_ONCE(mvm->tdls_cs.peer.sta_id == mvm_sta->sta_id)) { if (WARN_ON_ONCE(mvm->tdls_cs.peer.sta_id == sta_id)) {
mvm->tdls_cs.peer.sta_id = IWL_MVM_STATION_COUNT; mvm->tdls_cs.peer.sta_id = IWL_MVM_STATION_COUNT;
cancel_delayed_work(&mvm->tdls_cs.dwork); cancel_delayed_work(&mvm->tdls_cs.dwork);
} }
...@@ -1582,21 +1592,20 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm, ...@@ -1582,21 +1592,20 @@ int iwl_mvm_rm_sta(struct iwl_mvm *mvm,
* calls the drain worker. * calls the drain worker.
*/ */
spin_lock_bh(&mvm_sta->lock); spin_lock_bh(&mvm_sta->lock);
/* /*
* There are frames pending on the AC queues for this station. * There are frames pending on the AC queues for this station.
* We need to wait until all the frames are drained... * We need to wait until all the frames are drained...
*/ */
if (atomic_read(&mvm->pending_frames[mvm_sta->sta_id])) { if (atomic_read(&mvm->pending_frames[sta_id])) {
rcu_assign_pointer(mvm->fw_id_to_mac_id[mvm_sta->sta_id], rcu_assign_pointer(mvm->fw_id_to_mac_id[sta_id],
ERR_PTR(-EBUSY)); ERR_PTR(-EBUSY));
spin_unlock_bh(&mvm_sta->lock); spin_unlock_bh(&mvm_sta->lock);
/* disable TDLS sta queues on drain complete */ /* disable TDLS sta queues on drain complete */
if (sta->tdls) { if (sta->tdls) {
mvm->tfd_drained[mvm_sta->sta_id] = mvm->tfd_drained[sta_id] = mvm_sta->tfd_queue_msk;
mvm_sta->tfd_queue_msk; IWL_DEBUG_TDLS(mvm, "Draining TDLS sta %d\n", sta_id);
IWL_DEBUG_TDLS(mvm, "Draining TDLS sta %d\n",
mvm_sta->sta_id);
} }
ret = iwl_mvm_drain_sta(mvm, mvm_sta, true); ret = iwl_mvm_drain_sta(mvm, mvm_sta, true);
......
...@@ -1015,7 +1015,10 @@ static int iwl_mvm_tx_mpdu(struct iwl_mvm *mvm, struct sk_buff *skb, ...@@ -1015,7 +1015,10 @@ static int iwl_mvm_tx_mpdu(struct iwl_mvm *mvm, struct sk_buff *skb,
spin_unlock(&mvmsta->lock); spin_unlock(&mvmsta->lock);
/* Increase pending frames count if this isn't AMPDU */ /* Increase pending frames count if this isn't AMPDU */
if (!is_ampdu) if ((iwl_mvm_is_dqa_supported(mvm) &&
mvmsta->tid_data[tx_cmd->tid_tspec].state != IWL_AGG_ON &&
mvmsta->tid_data[tx_cmd->tid_tspec].state != IWL_AGG_STARTING) ||
(!iwl_mvm_is_dqa_supported(mvm) && !is_ampdu))
atomic_inc(&mvm->pending_frames[mvmsta->sta_id]); atomic_inc(&mvm->pending_frames[mvmsta->sta_id]);
return 0; return 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment