Re: Warning when terminating wpa_supplicant (wext driver)

Johannes Berg
2009-05-09T11:09:09Z

[-- Attachment #1: Type: text/plain, Size: 3551 bytes --]

On Fri, 2009-05-08 at 17:02 -0700, Marcel Holtmann wrote:

> phy0: AddBA: ssn=277, dialog_token=1 tid=0 timeout=0ba_policy=1

> Rx A-MPDU request on tid 0 result 0
> wlan0: CTS protection enabled (BSSID=00:1c:f0:xx:xx:xx)
> wlan0: CTS protection disabled (BSSID=00:1c:f0:xx:xx:xx)
> wlan0: no probe response from AP 00:1c:f0:xx:xx:xx - disassociating

At this point we tell the driver we're no longer associated. I think the
driver takes that as a reason to remove the station struct it keeps
track of in the driver. However,

> Tx BA session stop requested for 00:1c:f0:xx:xx:xx tid 0

now we stop the TX BA session

> iwlagn 0000:03:00.0: Invalid station for AGG tid 0

and iwlwifi no longer finds the station for it, returns an error

> ------------[ cut here ]------------
> WARNING: at net/mac80211/agg-tx.c:142 ___ieee80211_stop_tx_ba_session+0x6b/0x7e [mac80211]()

which causes the warning

> Rx BA session stop requested for 00:1c:f0:xx:xx:xx tid 0
> iwlagn 0000:03:00.0: Invalid station for AGG tid 0
> HW problem - can not stop rx aggregation for tid 0

same here -- but it's not a WARN_ON (though it probably should be!)

> phy0: device now idle
> phy0: Removed STA 00:1c:f0:xx:xx:xx

_Now_ mac80211 starts removing the station info.

> mac80211-phy0: failed to remove key (0, 00:1c:f0:xx:xx:xx) from hardware (-22)

before it asks the driver to remove the station with sta_notify, it
removes the key for the station. But even that fails, since iwlwifi
knows nothing about the station.

> phy0: Destroyed STA 00:1c:f0:xx:xx:xx

Now mac80211 has gotten rid of the sta struct completely.

I really really think the problem is that iwlwifi insists on keeping
track of station information with the wrong callback. It should be using
the sta_notify() callback, but it uses the bss_info_changed() callback,
which leads to ordering issues because we first tell the driver that we
disassociated, and _then_ remove all the information about the STA.

Now, I guess it's debatable whether or not we should tell the driver
that we disassociated _before_ we remove the station for the AP. But if
we reorder, other drivers might get confused because, well, we first
remove the AP STA and then tell them we disassociated -- but what from?

Therefore, I think disassociating first and then removing the station
information is correct.

However, to test my theory, you could apply the below patch. I suspect
it will no longer happen with this patch and iwlwifi.

Ultimately though I think iwlwifi just needs to be fixed wrt. station
handling. A whole lot of complex lookup code will go away because
mac80211 passes station structs wherever needed, which will improve
performance and reduce code size significantly.

johannes

--- wireless-testing.orig/net/mac80211/mlme.c 2009-05-09 13:06:54.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c 2009-05-09 13:07:59.000000000 +0200
@@ -1062,9 +1062,6 @@ static void ieee80211_set_disassoc(struc
config_changed |= IEEE80211_CONF_CHANGE_PS;
}

- ieee80211_hw_config(local, config_changed);
- ieee80211_bss_info_change_notify(sdata, changed);
-
rcu_read_lock();

sta = sta_info_get(local, ifmgd->bssid);
@@ -1078,6 +1075,9 @@ static void ieee80211_set_disassoc(struc
rcu_read_unlock();

sta_info_destroy(sta);
+
+ ieee80211_hw_config(local, config_changed);
+ ieee80211_bss_info_change_notify(sdata, changed);
}

static int ieee80211_sta_wep_configured(struct ieee80211_sub_if_data *sdata)

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]