From nobody Wed Jun 12 16:41:22 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4VzrsH3wTVz5N3Jv; Wed, 12 Jun 2024 16:41:23 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4VzrsH0yrKz4gnR; Wed, 12 Jun 2024 16:41:23 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1718210483; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=pT8C+W0CCsHJKriZ8oIznI8vyeMNDdT9ZzSuBU1TmEg=; b=hhIPt2H02Ef+uy42SFShl81o9RzxNflL8VOUiZX3ui4xGhNGDTqVvF+Sl/Bt2DIrIBSxOE 6mdfPBvUEmotYhqIfCVBIL5q4MU7yKICZo9dYV3hZZq7pGuxtNtlqCu/NPmTVGNBmLt/7S IFmCOrvFvaDI8fDYee8r/yKo66AMeG2ueRKPDQTcl/FedA1UosS1hlgcufKg658jtMSWXg TLeW1JfFuDlYEquVk8ezuclF4hxaxDKZ/T1uN8okraUED/H6drI9sHdL80ehcei2CiQX8R TUHAG5gPXVnQurWvADWvWZ3eE0Dc74mHWE0FbC+iZW5MVESG1jFWHtLOHanPyg== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1718210483; a=rsa-sha256; cv=none; b=kkrOFfAsh18MxMhENgp4aXrqh9QG1a73jpWUnIHk7ngl1hwigxrl4IRUqjxZOQQPkw8I62 Ee7VPLxP3gPutSET2WFxjFFlu2tP6z1h822rRzcDVKdSX1T+5dap1A1k5l3y26/gDbW5Zw St4zzfbrUsT9JllbRxIYzeMQpdS0TKWof4EUNjf8/1PT+LgFcL+2b+WDmap97E5MAMsSYy 6ym0fPEjcQBpqc1E2eNbw4j/1wRoBWDT23hZhPnxu2UmirpARWdO5HH7T6H4aGIjlrfSvy WuL4wS0DTnDWPua/d6nag3+L/f5JxYoP7T/Qv9fmp0QFWEzek+NiNDaQhoNaLA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1718210483; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=pT8C+W0CCsHJKriZ8oIznI8vyeMNDdT9ZzSuBU1TmEg=; b=dFC4a3WEdJjK1nwfre3+naRKBFiraQSWtGpIHAoj8cN+oK+eUdF8t8sl7Fp7r3bRKVaQV7 nkxI6BjgOvTU6HfUsXQaO0kSsViGLvkLnWNkPRdX9/8vsEDHiCIgYhuNBylTUefmqbZrAa 78GsqvxDidhnZCTOHM/1tzsM8YJfN6FjaENncPhnT3M9RbL9KwFmQYK/b5yydRm2ASzeAZ sRXHh4c92mdTcfCqa4DigSzyrnb8OMVosDYF1Kf8YGdBCDbscXBMRiELaJb7P15ZBiU1c/ 67GOIjKAgmZFTqmGTzq20S93fwX8jgJk9WiLLnsKwcCS801HhY0Q1DngoblzwA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4VzrsH0bf0z11Hj; Wed, 12 Jun 2024 16:41:23 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.17.1/8.17.1) with ESMTP id 45CGfMvK046681; Wed, 12 Jun 2024 16:41:22 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 45CGfMMA046677; Wed, 12 Jun 2024 16:41:22 GMT (envelope-from git) Date: Wed, 12 Jun 2024 16:41:22 GMT Message-Id: <202406121641.45CGfMMA046677@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-branches@FreeBSD.org From: "Bjoern A. Zeeb" Subject: git: 7ad7453748e2 - stable/14 - LinuxKPI: 802.11: change teardown order to avoid iwlwifi firmware crashes List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: bz X-Git-Repository: src X-Git-Refname: refs/heads/stable/14 X-Git-Reftype: branch X-Git-Commit: 7ad7453748e2adafa1e1a3e44b02fc852d4c5301 Auto-Submitted: auto-generated The branch stable/14 has been updated by bz: URL: https://cgit.FreeBSD.org/src/commit/?id=7ad7453748e2adafa1e1a3e44b02fc852d4c5301 commit 7ad7453748e2adafa1e1a3e44b02fc852d4c5301 Author: Bjoern A. Zeeb AuthorDate: 2024-05-22 02:24:51 +0000 Commit: Bjoern A. Zeeb CommitDate: 2024-06-12 13:58:36 +0000 LinuxKPI: 802.11: change teardown order to avoid iwlwifi firmware crashes While the previous order worked well for iwlwifi 22000 and later chipsets (AXxxx, BE200), earlier chipsets had trouble and ran into firmware crashes. Change the teardown order to avoid these problems. The inline comments in lkpi_sta_run_to_init() (and lkpi_disassoc()) try to document the new order and also the old problems we were seeing (too early sta removal or silent non-removal) leading to follow-up problems. There is a possible further problem still lingering but a lot harder to trigger (see comment in review) and likely related to some other doings so we'll track it separately. Sponsored by: The FreeBSD Foundation PR: 275255 Tested with: AX210, 8265 (bz); 9260 (Bakul Shah) Differential Revision: https://reviews.freebsd.org/D45293 (cherry picked from commit 5a4d24610fc6143ac1d570fe2b5160e8ae893c2c) --- sys/compat/linuxkpi/common/src/linux_80211.c | 84 ++++++++++++++++++---------- 1 file changed, 55 insertions(+), 29 deletions(-) diff --git a/sys/compat/linuxkpi/common/src/linux_80211.c b/sys/compat/linuxkpi/common/src/linux_80211.c index 758db287d613..32b0287db65c 100644 --- a/sys/compat/linuxkpi/common/src/linux_80211.c +++ b/sys/compat/linuxkpi/common/src/linux_80211.c @@ -994,33 +994,37 @@ lkpi_hw_conf_idle(struct ieee80211_hw *hw, bool new) } } -static void +static enum ieee80211_bss_changed lkpi_disassoc(struct ieee80211_sta *sta, struct ieee80211_vif *vif, struct lkpi_hw *lhw) { + enum ieee80211_bss_changed changed; + + changed = 0; sta->aid = 0; if (vif->cfg.assoc) { - struct ieee80211_hw *hw; - enum ieee80211_bss_changed changed; lhw->update_mc = true; lkpi_update_mcast_filter(lhw->ic, true); - changed = 0; vif->cfg.assoc = false; vif->cfg.aid = 0; changed |= BSS_CHANGED_ASSOC; - /* - * This will remove the sta from firmware for iwlwifi. - * So confusing that they use state and flags and ... ^%$%#%$^. - */ IMPROVE(); - hw = LHW_TO_HW(lhw); - lkpi_80211_mo_bss_info_changed(hw, vif, &vif->bss_conf, - changed); - lkpi_hw_conf_idle(hw, true); + /* + * Executing the bss_info_changed(BSS_CHANGED_ASSOC) with + * assoc = false right away here will remove the sta from + * firmware for iwlwifi. + * We no longer do this but only return the BSS_CHNAGED value. + * The caller is responsible for removing the sta gong to + * IEEE80211_STA_NOTEXIST and then executing the + * bss_info_changed() update. + * See lkpi_sta_run_to_init() for more detailed comment. + */ } + + return (changed); } static void @@ -1460,6 +1464,8 @@ lkpi_sta_auth_to_scan(struct ieee80211vap *vap, enum ieee80211_state nstate, int lkpi_80211_mo_unassign_vif_chanctx(hw, vif, &vif->bss_conf, &vif->chanctx_conf); /* NB: vif->chanctx_conf is NULL now. */ + lkpi_hw_conf_idle(hw, true); + /* Remove chan ctx. */ lkpi_80211_mo_remove_chanctx(hw, conf); lchanctx = CHANCTX_CONF_TO_LCHANCTX(conf); @@ -1743,16 +1749,11 @@ _lkpi_sta_assoc_to_down(struct ieee80211vap *vap, enum ieee80211_state nstate, i goto out; } - lkpi_lsta_dump(lsta, ni, __func__, __LINE__); + /* See comment in lkpi_sta_run_to_init(). */ + bss_changed = 0; + bss_changed |= lkpi_disassoc(sta, vif, lhw); - /* 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); + lkpi_lsta_dump(lsta, ni, __func__, __LINE__); /* Adjust sta and change state (from NONE) to NOTEXIST. */ KASSERT(lsta != NULL, ("%s: ni %p lsta is NULL\n", __func__, ni)); @@ -1769,7 +1770,6 @@ _lkpi_sta_assoc_to_down(struct ieee80211vap *vap, enum ieee80211_state nstate, i lkpi_lsta_dump(lsta, ni, __func__, __LINE__); /* sta no longer save to use. */ IMPROVE("Any bss_info changes to announce?"); - bss_changed = 0; vif->bss_conf.qos = 0; bss_changed |= BSS_CHANGED_QOS; vif->cfg.ssid_len = 0; @@ -1802,6 +1802,8 @@ _lkpi_sta_assoc_to_down(struct ieee80211vap *vap, enum ieee80211_state nstate, i lkpi_80211_mo_unassign_vif_chanctx(hw, vif, &vif->bss_conf, &vif->chanctx_conf); /* NB: vif->chanctx_conf is NULL now. */ + lkpi_hw_conf_idle(hw, true); + /* Remove chan ctx. */ lkpi_80211_mo_remove_chanctx(hw, conf); lchanctx = CHANCTX_CONF_TO_LCHANCTX(conf); @@ -2290,14 +2292,33 @@ lkpi_sta_run_to_init(struct ieee80211vap *vap, enum ieee80211_state nstate, int goto out; } - lkpi_lsta_dump(lsta, ni, __func__, __LINE__); - - /* Update bss info (bss_info_changed) (assoc, aid, ..). */ + bss_changed = 0; /* + * Start updating 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. + * See comment there; removes the sta from fw if not careful + * (bss_info_changed() change is executed right away). + * + * 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. + * + * The order which works best so far avoiding early removal or silent + * non-removal seems to be (for iwlwifi::mld-mac80211.c cases; + * the iwlwifi:mac80211.c case still to be tested): + * 1) lkpi_disassoc(): set vif->cfg.assoc = false (aid=0 side effect here) + * 2) call the last sta_state update -> IEEE80211_STA_NOTEXIST + * (removes the sta given assoc is false) + * 3) add the remaining BSS_CHANGED changes and call bss_info_changed() + * 4) call unassign_vif_chanctx + * 5) call lkpi_hw_conf_idle + * 6) call remove_chanctx */ - lkpi_disassoc(sta, vif, lhw); + bss_changed |= lkpi_disassoc(sta, vif, lhw); + + lkpi_lsta_dump(lsta, ni, __func__, __LINE__); /* Adjust sta and change state (from NONE) to NOTEXIST. */ KASSERT(lsta != NULL, ("%s: ni %p lsta is NULL\n", __func__, ni)); @@ -2311,15 +2332,19 @@ lkpi_sta_run_to_init(struct ieee80211vap *vap, enum ieee80211_state nstate, int goto out; } + lkpi_lsta_remove(lsta, lvif); + lkpi_lsta_dump(lsta, ni, __func__, __LINE__); /* sta no longer save to use. */ IMPROVE("Any bss_info changes to announce?"); - bss_changed = 0; vif->bss_conf.qos = 0; bss_changed |= BSS_CHANGED_QOS; vif->cfg.ssid_len = 0; memset(vif->cfg.ssid, '\0', sizeof(vif->cfg.ssid)); bss_changed |= BSS_CHANGED_BSSID; + vif->bss_conf.use_short_preamble = false; + vif->bss_conf.qos = false; + /* XXX BSS_CHANGED_???? */ lkpi_80211_mo_bss_info_changed(hw, vif, &vif->bss_conf, bss_changed); LKPI_80211_LVIF_LOCK(lvif); @@ -2327,7 +2352,6 @@ lkpi_sta_run_to_init(struct ieee80211vap *vap, enum ieee80211_state nstate, int lvif->lvif_bss = NULL; lvif->lvif_bss_synched = false; LKPI_80211_LVIF_UNLOCK(lvif); - lkpi_lsta_remove(lsta, lvif); /* * The very last release the reference on the ni for the ni/lsta on * lvif->lvif_bss. Upon return from this both ni and lsta are invalid @@ -2347,6 +2371,8 @@ lkpi_sta_run_to_init(struct ieee80211vap *vap, enum ieee80211_state nstate, int lkpi_80211_mo_unassign_vif_chanctx(hw, vif, &vif->bss_conf, &vif->chanctx_conf); /* NB: vif->chanctx_conf is NULL now. */ + lkpi_hw_conf_idle(hw, true); + /* Remove chan ctx. */ lkpi_80211_mo_remove_chanctx(hw, conf); lchanctx = CHANCTX_CONF_TO_LCHANCTX(conf);