From nobody Fri Feb 16 15:50:56 2024 X-Original-To: dev-commits-src-main@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 4TbxH50hK2z51rfd; Fri, 16 Feb 2024 15:50:57 +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 4TbxH50FRtz4sbV; Fri, 16 Feb 2024 15:50:57 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1708098657; 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=IBAhm0x9q8yjNTHplHA/NogyRWxeYvZ30/sCtuqMoDY=; b=qSn2dmLZM4pbEpjaa8cc/Jh9/wRbtulDLZopgRNrxvaAc7xHu7elivVeiYHKn7tmNHblVE ehVPKPEqw+9opLBn0orxgFx0wu+lGK+510zIzTkuSIKsH8bcy17OsG6sEWlEU1je2M3MWw VV9otT6uteWJ4GOf2qU9ESPhC1HWYZplE0+g5oPI7Qfc/YNCzF8EQFi3SNxdSd5XkpkxSl mKjft2p98umFuQVOXBZintuOIKOsI+GCWRkod5CPGSXzsckPDpA8LuYUhvfazyfQgBp+pm 4c27OomEGgo4Nhb/T0hDO2YKNm0R4irWFESSAj26ZjzXWCUlFty7ebHukTm2Eg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1708098657; 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=IBAhm0x9q8yjNTHplHA/NogyRWxeYvZ30/sCtuqMoDY=; b=tdiH3/3cM+guGxu3GVgN2URX8Oa+6PaHLGzMSYE1NtvpGWEFYmyNNvw2ch7Q9Rx1nEpAXu cyxV/+QMLxEap0HD4Xf8fr2ivslkTwkaRqg8P3S1MnzzrL0NubnZIcmMkJIosi03zwS8LA bN3dqcQnAg6mE3EyeppI0d2JShpNpZeiSYyAMG0YLB1/2Aa/wzqTx3saJVE7hxp2A/O35M of153FXwqew1cC4nyPzf6N1ddmWk+sJLTyuRyqsve9Hm7srN0ClozzWmZsXG50hKoBFniI z29enEMSSQo/4rBRz9fXtwriZ9reO9SNM4fNXGfUJ1I89CqEGwci5inMgJG98A== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1708098657; a=rsa-sha256; cv=none; b=JYzgmlvYsQmkTdYsx4QY3tqteSpC/w/LFSr1ZPlwBzcPnEYii9VVpJd0Ic59EzomBjsCCK 2jgHOE/xTlYqjEEm5PkYDqdiRfPrwn7e0R66mTmBLBFTPU/HGAWw+Ad7WS3ad7Liak4ZIj EtwWyOnp8PBC6tjbCo1gLmTS8dGUZlYKgyvnfjNSXsPkBSbWw2UOZYnbJlfaeWjUeYExVQ PfCDAiP8cICanjeG7PGiPmSw0wK8KeLZsfr9y/aAh5oozFBgGeTz4Kk741CphxacRFwW/j 5QXdFC9m7YNg+W4Q20yZmAXXJLSXIQ2K120WB5J1DSAqmZM9TGncxZ0BSH/OxA== 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 4TbxH46Q7tzWv9; Fri, 16 Feb 2024 15:50:56 +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 41GFouCp018322; Fri, 16 Feb 2024 15:50:56 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.17.1/8.17.1/Submit) id 41GFouxo018319; Fri, 16 Feb 2024 15:50:56 GMT (envelope-from git) Date: Fri, 16 Feb 2024 15:50:56 GMT Message-Id: <202402161550.41GFouxo018319@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: fa4e42579436 - main - LinuxKPI: 802.11: lsta txq locking cleanup List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@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: fa4e4257943650c0b5f58c01bb0bdfadea61dfb2 Auto-Submitted: auto-generated The branch main has been updated by bz: URL: https://cgit.FreeBSD.org/src/commit/?id=fa4e4257943650c0b5f58c01bb0bdfadea61dfb2 commit fa4e4257943650c0b5f58c01bb0bdfadea61dfb2 Author: Bjoern A. Zeeb AuthorDate: 2024-02-14 21:56:48 +0000 Commit: Bjoern A. Zeeb CommitDate: 2024-02-16 15:50:11 +0000 LinuxKPI: 802.11: lsta txq locking cleanup Rename the LSTA lock to LSTA_TXQ lock as that is really what it is and put down the full set of macros. Replace the init and destroy with the macro invocation rather than direct code. Put locking around the txq_ready unset and check. Move the taskq_enqueue call under lock to be sure we do not call it anymore after txq_ready got unset. Leave a comment related to the node reference which is passed into the TX path on the recvif mbuf pointer. Fixes: 0936c648ad0ee PR: 274382 (possibly) MFC after: 1 day Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D43909 --- sys/compat/linuxkpi/common/src/linux_80211.c | 31 ++++++++++++++++++---------- sys/compat/linuxkpi/common/src/linux_80211.h | 14 +++++++++++-- 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/sys/compat/linuxkpi/common/src/linux_80211.c b/sys/compat/linuxkpi/common/src/linux_80211.c index 99984569f35e..6ed5722ab998 100644 --- a/sys/compat/linuxkpi/common/src/linux_80211.c +++ b/sys/compat/linuxkpi/common/src/linux_80211.c @@ -366,7 +366,7 @@ lkpi_lsta_alloc(struct ieee80211vap *vap, const uint8_t mac[IEEE80211_ADDR_LEN], } /* Deferred TX path. */ - mtx_init(&lsta->txq_mtx, "lsta_txq", NULL, MTX_DEF); + LKPI_80211_LSTA_TXQ_LOCK_INIT(lsta); TASK_INIT(&lsta->txq_task, 0, lkpi_80211_txq_task, lsta); mbufq_init(&lsta->txq, IFQ_MAXLEN); lsta->txq_ready = true; @@ -398,8 +398,11 @@ lkpi_lsta_free(struct lkpi_sta *lsta, struct ieee80211_node *ni) /* XXX-BZ free resources, ... */ IMPROVE(); - /* XXX locking */ + /* Drain sta->txq[] */ + + LKPI_80211_LSTA_TXQ_LOCK(lsta); lsta->txq_ready = false; + LKPI_80211_LSTA_TXQ_UNLOCK(lsta); /* Drain taskq, won't be restarted until added_to_drv is set again. */ while (taskqueue_cancel(taskqueue_thread, &lsta->txq_task, NULL) != 0) @@ -418,9 +421,7 @@ lkpi_lsta_free(struct lkpi_sta *lsta, struct ieee80211_node *ni) } KASSERT(mbufq_empty(&lsta->txq), ("%s: lsta %p has txq len %d != 0\n", __func__, lsta, mbufq_len(&lsta->txq))); - - /* Drain sta->txq[] */ - mtx_destroy(&lsta->txq_mtx); + LKPI_80211_LSTA_TXQ_LOCK_DESTROY(lsta); /* Remove lsta from vif; that is done by the state machine. Should assert it? */ @@ -3536,16 +3537,21 @@ lkpi_ic_raw_xmit(struct ieee80211_node *ni, struct mbuf *m, struct lkpi_sta *lsta; lsta = ni->ni_drv_data; - /* XXX locking */ + LKPI_80211_LSTA_TXQ_LOCK(lsta); if (!lsta->txq_ready) { + LKPI_80211_LSTA_TXQ_UNLOCK(lsta); + /* + * Free the mbuf (do NOT release ni ref for the m_pkthdr.rcvif! + * ieee80211_raw_output() does that in case of error). + */ m_free(m); return (ENETDOWN); } /* Queue the packet and enqueue the task to handle it. */ - LKPI_80211_LSTA_LOCK(lsta); mbufq_enqueue(&lsta->txq, m); - LKPI_80211_LSTA_UNLOCK(lsta); + taskqueue_enqueue(taskqueue_thread, &lsta->txq_task); + LKPI_80211_LSTA_TXQ_UNLOCK(lsta); #ifdef LINUXKPI_DEBUG_80211 if (linuxkpi_debug_80211 & D80211_TRACE_TX) @@ -3554,7 +3560,6 @@ lkpi_ic_raw_xmit(struct ieee80211_node *ni, struct mbuf *m, mbufq_len(&lsta->txq)); #endif - taskqueue_enqueue(taskqueue_thread, &lsta->txq_task); return (0); } @@ -3769,9 +3774,13 @@ lkpi_80211_txq_task(void *ctx, int pending) mbufq_init(&mq, IFQ_MAXLEN); - LKPI_80211_LSTA_LOCK(lsta); + LKPI_80211_LSTA_TXQ_LOCK(lsta); + /* + * Do not re-check lsta->txq_ready here; we may have a pending + * disassoc frame still. + */ mbufq_concat(&mq, &lsta->txq); - LKPI_80211_LSTA_UNLOCK(lsta); + LKPI_80211_LSTA_TXQ_UNLOCK(lsta); m = mbufq_dequeue(&mq); while (m != NULL) { diff --git a/sys/compat/linuxkpi/common/src/linux_80211.h b/sys/compat/linuxkpi/common/src/linux_80211.h index b36b27168566..d25614de56dc 100644 --- a/sys/compat/linuxkpi/common/src/linux_80211.h +++ b/sys/compat/linuxkpi/common/src/linux_80211.h @@ -310,8 +310,18 @@ struct lkpi_wiphy { #define LKPI_80211_LVIF_LOCK(_lvif) mtx_lock(&(_lvif)->mtx) #define LKPI_80211_LVIF_UNLOCK(_lvif) mtx_unlock(&(_lvif)->mtx) -#define LKPI_80211_LSTA_LOCK(_lsta) mtx_lock(&(_lsta)->txq_mtx) -#define LKPI_80211_LSTA_UNLOCK(_lsta) mtx_unlock(&(_lsta)->txq_mtx) +#define LKPI_80211_LSTA_TXQ_LOCK_INIT(_lsta) \ + mtx_init(&(_lsta)->txq_mtx, "lsta-txq", NULL, MTX_DEF); +#define LKPI_80211_LSTA_TXQ_LOCK_DESTROY(_lsta) \ + mtx_destroy(&(_lsta)->txq_mtx); +#define LKPI_80211_LSTA_TXQ_LOCK(_lsta) \ + mtx_lock(&(_lsta)->txq_mtx) +#define LKPI_80211_LSTA_TXQ_UNLOCK(_lsta) \ + mtx_unlock(&(_lsta)->txq_mtx) +#define LKPI_80211_LSTA_TXQ_LOCK_ASSERT(_lsta) \ + mtx_assert(&(_lsta)->txq_mtx, MA_OWNED) +#define LKPI_80211_LSTA_TXQ_UNLOCK_ASSERT(_lsta) \ + mtx_assert(&(_lsta)->txq_mtx, MA_NOTOWNED) #define LKPI_80211_LTXQ_LOCK_INIT(_ltxq) \ mtx_init(&(_ltxq)->ltxq_mtx, "ltxq", NULL, MTX_DEF);