Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 29 Nov 2023 16:38:37 GMT
From:      "Bjoern A. Zeeb" <bz@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org
Subject:   git: 6b816fdfada1 - stable/13 - LinuxKPI: 802.11: fix problem removing sta
Message-ID:  <202311291638.3ATGcbBn046334@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch stable/13 has been updated by bz:

URL: https://cgit.FreeBSD.org/src/commit/?id=6b816fdfada19b56750053c02f0c32f562a77b94

commit 6b816fdfada19b56750053c02f0c32f562a77b94
Author:     Bjoern A. Zeeb <bz@FreeBSD.org>
AuthorDate: 2023-09-30 07:59:48 +0000
Commit:     Bjoern A. Zeeb <bz@FreeBSD.org>
CommitDate: 2023-11-29 16:36:09 +0000

    LinuxKPI: 802.11: fix problem removing sta
    
    Move the call to lkpi_disassoc() before the state change to
    IEEE80211_STA_NOTEXIST which will remove the sta (from firmware).
    Based on a comment it seems that originally we had to do it the
    other way round.
    iwlwifi(4) has a check and will silently not remove the sta from
    the firmware when vif->cfg.assoc is still set (see iwl_mvm_sta_del()).
    This leads to the LinuxKPI 802.11 code thinking the sta is gone as
    downward state changes by the mac80211 op (*sta_state)() may not fail.
    Once we try to assoc with a newly added station later, iwlwifi firmware
    will run into an assert having two ap_sta set on the same vif.
    We can observe that problem in iwlwifi(4) having __le32 sta_id = 1
    instead of sta_id = 0 in iwl_mvm_sta_cfg_cmd{} on the HCMD.
    
    This should fix one of the most seen problems with iwlwifi(4), e.g.,
    when running service netif restart wlan0 or
    service wpa_supplicant restart wlan0.
    
    Sponsored by:   The FreeBSD Foundation
    
    (cherry picked from commit 16e688b2a403a3dbc05de3b9f8a3132838613790)
---
 sys/compat/linuxkpi/common/src/linux_80211.c | 37 ++++++++++++++--------------
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/sys/compat/linuxkpi/common/src/linux_80211.c b/sys/compat/linuxkpi/common/src/linux_80211.c
index 85ee4f22434c..54c7c5de6627 100644
--- a/sys/compat/linuxkpi/common/src/linux_80211.c
+++ b/sys/compat/linuxkpi/common/src/linux_80211.c
@@ -1426,6 +1426,15 @@ _lkpi_sta_assoc_to_down(struct ieee80211vap *vap, enum ieee80211_state nstate, i
 
 	lkpi_lsta_dump(lsta, ni, __func__, __LINE__);
 
+	/* Update bss info (bss_info_changed) (assoc, aid, ..). */
+	/*
+	 * We need to do this now, before sta changes to IEEE80211_STA_NOTEXIST
+	 * as otherwise drivers (iwlwifi at least) will silently not remove
+	 * the sta from the firmware and when we will add a new one trigger
+	 * a fw assert.
+	 */
+	lkpi_disassoc(sta, vif, lhw);
+
 	/* Adjust sta and change state (from NONE) to NOTEXIST. */
 	KASSERT(lsta != NULL, ("%s: ni %p lsta is NULL\n", __func__, ni));
 	KASSERT(lsta->state == IEEE80211_STA_NONE, ("%s: lsta %p state not "
@@ -1435,15 +1444,8 @@ _lkpi_sta_assoc_to_down(struct ieee80211vap *vap, enum ieee80211_state nstate, i
 		IMPROVE("do we need to undo the chan ctx?");
 		goto out;
 	}
-#if 0
-	lsta->added_to_drv = false;	/* mo manages. */
-#endif
 
-	lkpi_lsta_dump(lsta, ni, __func__, __LINE__);
-
-	/* Update bss info (bss_info_changed) (assoc, aid, ..). */
-	/* We need to do this now, can only do after sta is IEEE80211_STA_NOTEXIST. */
-	lkpi_disassoc(sta, vif, lhw);
+	lkpi_lsta_dump(lsta, ni, __func__, __LINE__);	/* sta no longer save to use. */
 
 	IMPROVE("Any bss_info changes to announce?");
 	bss_changed = 0;
@@ -1887,6 +1889,13 @@ lkpi_sta_run_to_init(struct ieee80211vap *vap, enum ieee80211_state nstate, int
 
 	lkpi_lsta_dump(lsta, ni, __func__, __LINE__);
 
+	/* Update bss info (bss_info_changed) (assoc, aid, ..). */
+	/*
+	 * One would expect this to happen when going off AUTHORIZED.
+	 * See comment there; removes the sta from fw.
+	 */
+	lkpi_disassoc(sta, vif, lhw);
+
 	/* Adjust sta and change state (from NONE) to NOTEXIST. */
 	KASSERT(lsta != NULL, ("%s: ni %p lsta is NULL\n", __func__, ni));
 	KASSERT(lsta->state == IEEE80211_STA_NONE, ("%s: lsta %p state not "
@@ -1896,18 +1905,8 @@ lkpi_sta_run_to_init(struct ieee80211vap *vap, enum ieee80211_state nstate, int
 		IMPROVE("do we need to undo the chan ctx?");
 		goto out;
 	}
-#if 0
-	lsta->added_to_drv = false;	/* mo manages. */
-#endif
 
-	lkpi_lsta_dump(lsta, ni, __func__, __LINE__);
-
-	/* Update bss info (bss_info_changed) (assoc, aid, ..). */
-	/*
-	 * One would expect this to happen when going off AUTHORIZED.
-	 * See comment there; removes the sta from fw.
-	 */
-	lkpi_disassoc(sta, vif, lhw);
+	lkpi_lsta_dump(lsta, ni, __func__, __LINE__);	/* sta no longer save to use. */
 
 	IMPROVE("Any bss_info changes to announce?");
 	bss_changed = 0;



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202311291638.3ATGcbBn046334>