From nobody Fri Jun 14 18:43:05 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 4W17Sn5Lbnz5PKP0; Fri, 14 Jun 2024 18:43:05 +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 4W17Sn2bfkz4GTp; Fri, 14 Jun 2024 18:43:05 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1718390585; 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=S+aGR2hapBbRPQPk8iRJC4K9/Xdxr0b0huqKORt+Pj0=; b=KVy4FB5DOCK8MqoB3V7adqDvsL9Geo4ABHyki7MKehCCPDaa5wfwVyXw006rlxnaYWCQLM 0z2RwTqlj2/e6JLTFGY6PLOlESF9TNJ7A8PVQXy9QqpkWl8EGcfiC4XDkzpM6yaW4FT5Q/ QR4Mrb/veQuMeFS0fKDtaYsHUpzV024nKuHbge+uD0mtPxRPjvlyM9OlqRmWmtpdxTDMIV hZA5basVpd2JuZbUdLvflznLQOd8hwBRwdzeHY04/i1dVRi/tNAXXeJKzjxU3wuswaDcyK y6v1fSzk+DuEqJuHZF7OC6ZwCzdwrwGRd4NmqHjWN4bfNJf1dp5QR2Na/R1Kfw== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1718390585; a=rsa-sha256; cv=none; b=qF8aGOcyhaZoNkyqJwG0MTDmJtMeUtNFso2cIPpaTXDbOEW2kskjj8FJOrzw42F4gGo3jS Mg1fcBUNkEduRxT9lEjOwSaC+OP07xKL5NiPBOy5dKzvKAI6C3dr9LfcU+1fjJmPNRbX1P QDbesf7REPErZcS/94mpibmZ+3FZtANXk9r5mn5KHEQQQ8Dt5YkrY23tbRM6zGg/pdUN+5 LwnYr+Xfl9oYdgD3i+Yokad5kWZ9o8l5YWCnR7zQDc6yqm497wIZSgcq0erL8YwTuaziUE 6GJmJ/vhDnwWrA7TGBPEsPdP/MatqA9GVNbq61F21a2KTQNHaihXiph+n1DkQg== 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=1718390585; 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=S+aGR2hapBbRPQPk8iRJC4K9/Xdxr0b0huqKORt+Pj0=; b=B7g40mCZoNvdv1ZJ8nST/Df8aS/HK8qSaGREZ3WQ6omuxDDH+vjA/yNuuHhjLTHMxoQG1c dCUARORnBxW/h6MHmYUfkP+PmC6cL8anMzfseg8ZxYRkNNm8AOV8/xjxTSDz3Rlp/1L9Cb Q2YAOMHy0qoOy3PcIVvhlf1t737abR1If7SW28LaDxZA8jyM4djGV/PwRFgXmCzp8yH425 yvXj4vg2/SgKXqfbhSCcjx3z34mg87k8yuyLnxQG7vBQxaCYcsNG7g1E626ImBtPPLMPhB CMXivXkBhQcf0vU/L6VlgWFUfpjleAmmJH38aQqhdRNkQp6zEEyYtywo/qrVlg== 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 4W17Sn2CFwzXLx; Fri, 14 Jun 2024 18:43:05 +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 45EIh5h2020600; Fri, 14 Jun 2024 18:43:05 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 45EIh5LC020597; Fri, 14 Jun 2024 18:43:05 GMT (envelope-from git) Date: Fri, 14 Jun 2024 18:43:05 GMT Message-Id: <202406141843.45EIh5LC020597@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: def43d8a4a3c - stable/13 - 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/13 X-Git-Reftype: branch X-Git-Commit: def43d8a4a3c0a2868fe74ef6aefe16c435ea19c Auto-Submitted: auto-generated The branch stable/13 has been updated by bz: URL: https://cgit.FreeBSD.org/src/commit/?id=def43d8a4a3c0a2868fe74ef6aefe16c435ea19c commit def43d8a4a3c0a2868fe74ef6aefe16c435ea19c Author: Bjoern A. Zeeb AuthorDate: 2024-05-22 02:24:51 +0000 Commit: Bjoern A. Zeeb CommitDate: 2024-06-14 14:55:16 +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 d90f89cdcbcd..632645a116b2 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);