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

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);