git: 00b4d712e2be - main - iwx: clean up TX AMPDU session establishment and checking
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 14 Nov 2025 02:37:36 UTC
The branch main has been updated by adrian:
URL: https://cgit.FreeBSD.org/src/commit/?id=00b4d712e2be54b8c87a7fd7f215ad5ef7845d5b
commit 00b4d712e2be54b8c87a7fd7f215ad5ef7845d5b
Author: Adrian Chadd <adrian@FreeBSD.org>
AuthorDate: 2025-11-13 04:33:52 +0000
Commit: Adrian Chadd <adrian@FreeBSD.org>
CommitDate: 2025-11-14 02:37:16 +0000
iwx: clean up TX AMPDU session establishment and checking
* Send a TX A-MPDU exchange successfully; we were allocating the
A-MPDU TX queue but returning 0 to net80211 was telling it
to not establish the TX A-MPDU session and none of the BA session
tracking stuff would work.
* Clean up the TX A-MPDU queue lookup in the transmit path - only
QoS data frames are allowed, not qos null-data, cf/ack/etc frames;
only send them if the A-MPDU session is established.
* Tell net80211 that we've established the TX A-MPDU session once
the firmware queue has been created.
* Check to make sure we're not double (or more) creating TX AMDPU
sessions - only allocate a qid if we're not doing A-MPDU yet.
* Delete IWX_FLAG_A-MPDUTX - it's now being properly tracked!
Locally tested:
* AX210, STA mode - gets 50/50mbit on 2GHz HT20, and 100/100mbit on
5GHz VHT/40.
Differential Revision: https://reviews.freebsd.org/D53725
Reviewed by: thj
---
sys/dev/iwx/if_iwx.c | 109 ++++++++++++++++++++++++++++++++++-----------
sys/dev/iwx/if_iwx_debug.h | 3 +-
sys/dev/iwx/if_iwxvar.h | 1 -
3 files changed, 85 insertions(+), 28 deletions(-)
diff --git a/sys/dev/iwx/if_iwx.c b/sys/dev/iwx/if_iwx.c
index dac1c563c593..e317ff9e271c 100644
--- a/sys/dev/iwx/if_iwx.c
+++ b/sys/dev/iwx/if_iwx.c
@@ -3429,6 +3429,14 @@ iwx_sta_rx_agg(struct iwx_softc *sc, struct ieee80211_node *ni, uint8_t tid,
sc->sc_rx_ba_sessions--;
}
+/**
+ * @brief Allocate an A-MPDU / aggregation session for the given node and TID.
+ *
+ * This allocates a TX queue specifically for that TID.
+ *
+ * Note that this routine currently doesn't return any status/errors,
+ * so the caller can't know if the aggregation session was setup or not.
+ */
static void
iwx_sta_tx_agg_start(struct iwx_softc *sc, struct ieee80211_node *ni,
uint8_t tid)
@@ -3502,6 +3510,14 @@ iwx_ba_rx_task(void *arg, int npending __unused)
IWX_UNLOCK(sc);
}
+/**
+ * @brief Task called to setup a deferred block-ack session.
+ *
+ * This sets up any/all pending blockack sessions as defined
+ * in sc->ba_tx.start_tidmask.
+ *
+ * Note: the call to iwx_sta_tx_agg_start() isn't being error checked.
+ */
static void
iwx_ba_tx_task(void *arg, int npending __unused)
{
@@ -3509,22 +3525,38 @@ iwx_ba_tx_task(void *arg, int npending __unused)
struct ieee80211com *ic = &sc->sc_ic;
struct ieee80211vap *vap = TAILQ_FIRST(&ic->ic_vaps);
struct ieee80211_node *ni = vap->iv_bss;
+ uint32_t started_mask = 0;
int tid;
IWX_LOCK(sc);
for (tid = 0; tid < IWX_MAX_TID_COUNT; tid++) {
+ const struct ieee80211_tx_ampdu *tap;
+
if (sc->sc_flags & IWX_FLAG_SHUTDOWN)
break;
+ tap = &ni->ni_tx_ampdu[tid];
+ if (IEEE80211_AMPDU_RUNNING(tap))
+ break;
if (sc->ba_tx.start_tidmask & (1 << tid)) {
- DPRINTF(("%s: ampdu tx start for tid %i\n", __func__,
- tid));
+ IWX_DPRINTF(sc, IWX_DEBUG_AMPDU_MGMT,
+ "%s: ampdu tx start for tid %i\n", __func__, tid);
iwx_sta_tx_agg_start(sc, ni, tid);
sc->ba_tx.start_tidmask &= ~(1 << tid);
- sc->sc_flags |= IWX_FLAG_AMPDUTX;
+ started_mask |= (1 << tid);
}
}
IWX_UNLOCK(sc);
+
+ /* Iterate over the sessions we started; mark them as active */
+ for (tid = 0; tid < IWX_MAX_TID_COUNT; tid++) {
+ if (started_mask & (1 << tid)) {
+ IWX_DPRINTF(sc, IWX_DEBUG_AMPDU_MGMT,
+ "%s: informing net80211 to start ampdu on tid %i\n",
+ __func__, tid);
+ ieee80211_ampdu_tx_request_active_ext(ni, tid, 1);
+ }
+ }
}
static void
@@ -5627,7 +5659,6 @@ iwx_tx(struct iwx_softc *sc, struct mbuf *m, struct ieee80211_node *ni)
u_int hdrlen;
uint32_t rate_n_flags;
uint16_t num_tbs, flags, offload_assist = 0;
- uint8_t type, subtype;
int i, totlen, err, pad, qid;
#define IWM_MAX_SCATTER 20
bus_dma_segment_t *seg, segs[IWM_MAX_SCATTER];
@@ -5638,38 +5669,32 @@ iwx_tx(struct iwx_softc *sc, struct mbuf *m, struct ieee80211_node *ni)
IWX_ASSERT_LOCKED(sc);
wh = mtod(m, struct ieee80211_frame *);
- type = wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK;
- subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK;
hdrlen = ieee80211_anyhdrsize(wh);
qid = sc->first_data_qid;
/* Put QoS frames on the data queue which maps to their TID. */
- if (IEEE80211_QOS_HAS_SEQ(wh) && (sc->sc_flags & IWX_FLAG_AMPDUTX)) {
+ if (IEEE80211_QOS_HAS_SEQ(wh)) {
uint16_t qos = ieee80211_gettid(wh);
uint8_t tid = qos & IEEE80211_QOS_TID;
-#if 0
+ struct ieee80211_tx_ampdu *tap = &ni->ni_tx_ampdu[tid];
+
/*
- * XXX-THJ: TODO when we enable ba we need to manage the
- * mappings
+ * Note: we're currently putting all frames into one queue
+ * except for A-MPDU queues. We should be able to choose
+ * other WME queues but first we need to verify they've been
+ * correctly setup for data.
*/
- struct ieee80211_tx_ba *ba;
- ba = &ni->ni_tx_ba[tid];
- if (!IEEE80211_IS_MULTICAST(wh->i_addr1) &&
- type == IEEE80211_FC0_TYPE_DATA &&
- subtype != IEEE80211_FC0_SUBTYPE_NODATA &&
- subtype != IEEE80211_FC0_SUBTYPE_BAR &&
- sc->aggqid[tid] != 0 /*&&
- ba->ba_state == IEEE80211_BA_AGREED*/) {
- qid = sc->aggqid[tid];
-#else
- if (!IEEE80211_IS_MULTICAST(wh->i_addr1) &&
- type == IEEE80211_FC0_TYPE_DATA &&
- subtype != IEEE80211_FC0_SUBTYPE_NODATA &&
+ /*
+ * Only QoS data goes into an A-MPDU queue;
+ * don't add QoS null, the other data types, etc.
+ */
+ if (IEEE80211_AMPDU_RUNNING(tap) &&
+ IEEE80211_IS_QOSDATA(wh) &&
+ !IEEE80211_IS_MULTICAST(wh->i_addr1) &&
sc->aggqid[tid] != 0) {
qid = sc->aggqid[tid];
-#endif
}
}
@@ -10903,6 +10928,26 @@ iwx_ampdu_rx_stop(struct ieee80211_node *ni, struct ieee80211_rx_ampdu *rap)
return;
}
+/**
+ * @brief Called by net80211 to request an A-MPDU session be established.
+ *
+ * This is called by net80211 to see if an A-MPDU session can be established.
+ * However, the iwx(4) firmware will take care of establishing the BA
+ * session for us. net80211 doesn't have to send any action frames here;
+ * it just needs to plumb up the ampdu session once the BA has been sent.
+ *
+ * If we return 0 here then the firmware will set up the state but net80211
+ * will not; so it's on us to actually complete it via a call to
+ * ieee80211_ampdu_tx_request_active_ext() .
+ *
+ * @param ni ieee80211_node to establish A-MPDU session for
+ * @param tap pointer to the per-TID state struct
+ * @param dialogtoken dialogtoken field from the BA request
+ * @param baparamset baparamset field from the BA request
+ * @param batimeout batimeout field from the BA request
+ *
+ * @returns 0 so net80211 doesn't send the BA action frame to establish A-MPDU.
+ */
static int
iwx_addba_request(struct ieee80211_node *ni, struct ieee80211_tx_ampdu *tap,
int dialogtoken, int baparamset, int batimeout)
@@ -10911,10 +10956,22 @@ iwx_addba_request(struct ieee80211_node *ni, struct ieee80211_tx_ampdu *tap,
int tid;
tid = _IEEE80211_MASKSHIFT(le16toh(baparamset), IEEE80211_BAPS_TID);
- DPRINTF(("%s: tid=%i\n", __func__, tid));
+ IWX_DPRINTF(sc, IWX_DEBUG_AMPDU_MGMT,
+ "%s: queuing AMPDU start on tid %i\n", __func__, tid);
+
+ /* There's no nice way right now to tell net80211 that we're in the
+ * middle of an asynchronous ADDBA setup session. So, bump the timeout
+ * to hz ticks, hopefully we'll get a response by then.
+ */
+ tap->txa_nextrequest = ticks + hz;
+
+ IWX_LOCK(sc);
sc->ba_tx.start_tidmask |= (1 << tid);
+ IWX_UNLOCK(sc);
+
taskqueue_enqueue(sc->sc_tq, &sc->ba_tx_task);
- return 0;
+
+ return (0);
}
diff --git a/sys/dev/iwx/if_iwx_debug.h b/sys/dev/iwx/if_iwx_debug.h
index ab8284a59e0f..5fc127d986a9 100644
--- a/sys/dev/iwx/if_iwx_debug.h
+++ b/sys/dev/iwx/if_iwx_debug.h
@@ -38,7 +38,8 @@ enum {
IWX_DEBUG_LAR = 0x00400000, /* Location Aware Regulatory */
IWX_DEBUG_TE = 0x00800000, /* Time Event handling */
IWX_DEBUG_KEYMGMT = 0x01000000, /* Encryption key management */
- /* 0x0e000000 are available */
+ IWX_DEBUG_AMPDU_MGMT = 0x02000000, /* AMPDU TX/RX management */
+ /* 0x0c000000 are available */
IWX_DEBUG_NI = 0x10000000, /* Not Implemented */
IWX_DEBUG_REGISTER = 0x20000000, /* print chipset register */
IWX_DEBUG_TRACE = 0x40000000, /* Print begin and start driver function */
diff --git a/sys/dev/iwx/if_iwxvar.h b/sys/dev/iwx/if_iwxvar.h
index 1ac0bc24577c..5ed749db631e 100644
--- a/sys/dev/iwx/if_iwxvar.h
+++ b/sys/dev/iwx/if_iwxvar.h
@@ -290,7 +290,6 @@ struct iwx_rx_ring {
#define IWX_FLAG_BGSCAN 0x200 /* background scan in progress */
#define IWX_FLAG_TXFLUSH 0x400 /* Tx queue flushing in progress */
#define IWX_FLAG_HW_INITED 0x800 /* Hardware initialized */
-#define IWX_FLAG_AMPDUTX 0x1000
struct iwx_ucode_status {
uint32_t uc_lmac_error_event_table[2];