From nobody Wed May 22 21:08:20 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 4Vl3n05Z1Dz5L1RD; Wed, 22 May 2024 21:08:20 +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 4Vl3n051Y1z4H7d; Wed, 22 May 2024 21:08:20 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1716412100; 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=eaD6jtPH/IbMlmZ5rM3F//8jgoVkvoXZoR4Bg3EReO8=; b=cW7D+H7Mj6lQkxpgbgDnynlre3pmYSN2tIi9EmbF8jNEpfSc+xsFnhRtnQWmmHcYaez5KI okHlZxUOXgN9SNyyblnS7wHFXLis4W8SvfuJhn8xRDI0/NM/fGNXO4kkERUeWzLQRGN3Yh rgM4KNms7GOSk/dK3WUDM+Q62zaW9W8ftSVquz8QG44yZtXInQTRLojW5z/6eyPGi4SNaK VSn2VgkBne9NOyFYCabOplKVbawL/JvXY1qWOuiAplJ69CBqGxNMjD3DxNSPU+vBqX7aIH V6EAQfOKmX/s1e0SxrTwQQuhLgwSLGipFEnUAhiSvhj0y4Lhtjaw0bzunD1CqA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1716412100; a=rsa-sha256; cv=none; b=p+pWMO8ae1PkGRLp0qgD010Lk9Ho2LZHL0JPQ0giiANpFExinn5Tpt0Sg27kAUak1iaqaq 4IV2Kn0z//H7BaKEMiAknOeSkB2ztUd2mBW42a1AlCVrSEUPQNtJEw74ax9+MbNj27zVl3 4Hyz1VdF3r3aY4HFLLehv5p47mQJ1EzyTnrWX8DXDBc51O5mEPwSy4HIK7jfMTPL0fNdCZ 8Rt1BppTHF0C8jpo0rC0IgVKn2Q9a0iMTFCbATL3Iqu+icg68Aq2WnHufpgOC1vqIyT7zT 2guCYKnD1FrhHDrsYxYHKmEAlnAM7pg1dx3xahFIBkfRJ1IrKSFEniJwnMI0tg== 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=1716412100; 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=eaD6jtPH/IbMlmZ5rM3F//8jgoVkvoXZoR4Bg3EReO8=; b=D8oumlmnr71L2yEAFjk3yggh2HiZ1RIoZhUG8CEi8q2K04dgA0PRJXWrTRv32F6OxQfT8H zR5lteuivO+22Fv0LTtukcePyBMUrv+5U/TmHIscRb3oKNaQHQr0YwV4wXrN/4GNBthcGn Pf9KeS3kLPUJ3dmfyDu/SdkescnGZ1JxpOPI9SfuGNxT0S0kH04TG8ZVErNbEKzZnyBwiO HJ0LOvffmXX3ECCObu3s7pGOUNkQjZCxPgGeGU2h8f6zEZeqC5PsEoGO1QJJvzReGBbiml k26MUlK1SmZP+UKrgrAL6mwu1eqzzMdaNt9x7EQiZrNk/813sIpp5o+i1DeG8w== 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 4Vl3n04cl8zM2f; Wed, 22 May 2024 21:08:20 +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 44ML8KQP082979; Wed, 22 May 2024 21:08:20 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 44ML8K2l082976; Wed, 22 May 2024 21:08:20 GMT (envelope-from git) Date: Wed, 22 May 2024 21:08:20 GMT Message-Id: <202405222108.44ML8K2l082976@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: "Bjoern A. Zeeb" Subject: git: 5a4d24610fc6 - main - 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/main X-Git-Reftype: branch X-Git-Commit: 5a4d24610fc6143ac1d570fe2b5160e8ae893c2c Auto-Submitted: auto-generated The branch main has been updated by bz: URL: https://cgit.FreeBSD.org/src/commit/?id=5a4d24610fc6143ac1d570fe2b5160e8ae893c2c commit 5a4d24610fc6143ac1d570fe2b5160e8ae893c2c Author: Bjoern A. Zeeb AuthorDate: 2024-05-22 02:24:51 +0000 Commit: Bjoern A. Zeeb CommitDate: 2024-05-22 21:04:19 +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 MFC after: 3 days PR: 275255 Tested with: AX210, 8265 (bz); 9260 (Bakul Shah) Differential Revision: https://reviews.freebsd.org/D45293 --- 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 d598e9af0050..b982413382f8 100644 --- a/sys/compat/linuxkpi/common/src/linux_80211.c +++ b/sys/compat/linuxkpi/common/src/linux_80211.c @@ -993,33 +993,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 @@ -1459,6 +1463,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); @@ -1742,16 +1748,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)); @@ -1768,7 +1769,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; @@ -1801,6 +1801,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); @@ -2289,14 +2291,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)); @@ -2310,15 +2331,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); @@ -2326,7 +2351,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 @@ -2346,6 +2370,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);