Download raw body.
iwx: remove A-MPDU Rx reordering
> Date: Mon, 9 Mar 2026 14:55:30 +0100
> From: Stefan Sperling <stsp@stsp.name>
>
> iwx inherited code from iwm which handles reordering of received A-MPDU
> subframes at the driver level. In Linux iwlwifi this code was added for
> 9k devices and multi-queue Rx operation.
> Our drivers only use a single Rx queue, but keeping the code close enough
> to Linux seemed reasonable the time when 9k devices were new, and it didn't
> hurt.
>
> This code made its way into iwx because iwx started out as a copy of iwm.
> But it turns out AX200 and later devices perform Rx reordering in firmware
> already and set flags on Rx descriptors which the driver can use to check
> for duplicate frames which can be discarded.
>
> I have recently become aware of two related changes the iwlwifi developers
> made in 2023:
>
> 1) Multi-queue Rx has been disabled in Linux on iwm 9000 devices.
> (linux commit 29fa9a984b6d1075020f12071a89897fd62ed27f)
> This was done to allow for change number 2:
>
> 2) The Rx-reordering code has been removed from iwiwifi, because it was
> only needed for 9k devices which do not provide the needed information in
> Rx descriptors. And the code was found to occasionally drop legit frames.
> (linux commit ff8e3a40d78bc414213b2724ad775adf98780a5a)
>
> I am not sure to which degree the dropped legit frames are affecting
> our implementation of this in iwm. It seems to be working fine for us?
> Maybe we should look at potential packet loss on iwm 9k devices at some
> point, even though I do not recall any complaints about such an issue.
>
> In any case, this means the Rx reordering code in iwx is pointless.
> We can rely on Rx descriptor flags instead.
>
> Testing on any iwx devices would be appreciated. I have tested this diff
> on AX200 and BZ with no visible change in behaviour.
>
> ok?
Tested on MA. ok kettenis@
> M sys/dev/pci/if_iwx.c | 6+ 199-
> M sys/dev/pci/if_iwxreg.h | 1+ 0-
> M sys/dev/pci/if_iwxvar.h | 0+ 15-
>
> 3 files changed, 7 insertions(+), 214 deletions(-)
>
> path + /usr/src
> commit - bf3713f7dfbf19ede85da952a0d01b849aa8a252
> blob - e731cf032ef66ed7a7eab46a3669fc5d80d9e8d3
> file + sys/dev/pci/if_iwx.c
> --- sys/dev/pci/if_iwx.c
> +++ sys/dev/pci/if_iwx.c
> @@ -329,7 +329,6 @@ int iwx_ampdu_tx_start(struct ieee80211com *, struct i
> void iwx_rx_ba_session_expired(void *);
> void iwx_rx_bar_frame_release(struct iwx_softc *, struct iwx_rx_packet *,
> struct mbuf_list *);
> -void iwx_reorder_timer_expired(void *);
> void iwx_sta_rx_agg(struct iwx_softc *, struct ieee80211_node *, uint8_t,
> uint16_t, uint16_t, int, int);
> void iwx_sta_tx_agg_start(struct iwx_softc *, struct ieee80211_node *,
> @@ -494,12 +493,9 @@ void iwx_dump_driver_status(struct iwx_softc *);
> void iwx_nic_umac_error(struct iwx_softc *);
> int iwx_detect_duplicate(struct iwx_softc *, struct mbuf *,
> struct iwx_rx_mpdu_desc *, struct ieee80211_rxinfo *);
> -int iwx_is_sn_less(uint16_t, uint16_t, uint16_t);
> void iwx_release_frames(struct iwx_softc *, struct ieee80211_node *,
> struct iwx_rxba_data *, struct iwx_reorder_buffer *, uint16_t,
> struct mbuf_list *);
> -int iwx_oldsn_workaround(struct iwx_softc *, struct ieee80211_node *,
> - int, struct iwx_reorder_buffer *, uint32_t, uint32_t);
> int iwx_rx_reorder(struct iwx_softc *, struct mbuf *, int,
> struct iwx_rx_mpdu_desc *, int, int, uint32_t,
> struct ieee80211_rxinfo *, struct mbuf_list *);
> @@ -3192,13 +3188,7 @@ iwx_init_reorder_buffer(struct iwx_reorder_buffer *reo
> reorder_buf->head_sn = ssn;
> reorder_buf->num_stored = 0;
> reorder_buf->buf_size = buf_size;
> - reorder_buf->last_amsdu = 0;
> - reorder_buf->last_sub_index = 0;
> - reorder_buf->removed = 0;
> reorder_buf->valid = 0;
> - reorder_buf->consec_oldsn_drops = 0;
> - reorder_buf->consec_oldsn_ampdu_gp2 = 0;
> - reorder_buf->consec_oldsn_prev_drop = 0;
> }
>
> void
> @@ -3211,11 +3201,8 @@ iwx_clear_reorder_buffer(struct iwx_softc *sc, struct
> for (i = 0; i < reorder_buf->buf_size; i++) {
> entry = &rxba->entries[i];
> ml_purge(&entry->frames);
> - timerclear(&entry->reorder_time);
> }
>
> - reorder_buf->removed = 1;
> - timeout_del(&reorder_buf->reorder_timer);
> timerclear(&rxba->last_rx);
> timeout_del(&rxba->session_timer);
> rxba->baid = IWX_RX_REORDER_DATA_INVALID_BAID;
> @@ -3286,68 +3273,6 @@ iwx_rx_bar_frame_release(struct iwx_softc *sc, struct
> iwx_release_frames(sc, ni, rxba, buf, nssn, ml);
> }
>
> -void
> -iwx_reorder_timer_expired(void *arg)
> -{
> - struct mbuf_list ml = MBUF_LIST_INITIALIZER();
> - struct iwx_reorder_buffer *buf = arg;
> - struct iwx_rxba_data *rxba = iwx_rxba_data_from_reorder_buf(buf);
> - struct iwx_reorder_buf_entry *entries = &rxba->entries[0];
> - struct iwx_softc *sc = rxba->sc;
> - struct ieee80211com *ic = &sc->sc_ic;
> - struct ieee80211_node *ni = ic->ic_bss;
> - int i, s;
> - uint16_t sn = 0, index = 0;
> - int expired = 0;
> - int cont = 0;
> - struct timeval now, timeout, expiry;
> -
> - if (!buf->num_stored || buf->removed)
> - return;
> -
> - s = splnet();
> - getmicrouptime(&now);
> - USEC_TO_TIMEVAL(RX_REORDER_BUF_TIMEOUT_MQ_USEC, &timeout);
> -
> - for (i = 0; i < buf->buf_size ; i++) {
> - index = (buf->head_sn + i) % buf->buf_size;
> -
> - if (ml_empty(&entries[index].frames)) {
> - /*
> - * If there is a hole and the next frame didn't expire
> - * we want to break and not advance SN.
> - */
> - cont = 0;
> - continue;
> - }
> - timeradd(&entries[index].reorder_time, &timeout, &expiry);
> - if (!cont && timercmp(&now, &expiry, <))
> - break;
> -
> - expired = 1;
> - /* continue until next hole after this expired frame */
> - cont = 1;
> - sn = (buf->head_sn + (i + 1)) & 0xfff;
> - }
> -
> - if (expired) {
> - /* SN is set to the last expired frame + 1 */
> - iwx_release_frames(sc, ni, rxba, buf, sn, &ml);
> - if_input(&sc->sc_ic.ic_if, &ml);
> - ic->ic_stats.is_ht_rx_ba_window_gap_timeout++;
> - } else {
> - /*
> - * If no frame expired and there are stored frames, index is now
> - * pointing to the first unexpired frame - modify reorder timeout
> - * accordingly.
> - */
> - timeout_add_usec(&buf->reorder_timer,
> - RX_REORDER_BUF_TIMEOUT_MQ_USEC);
> - }
> -
> - splx(s);
> -}
> -
> #define IWX_MAX_RX_BA_SESSIONS 16
>
> struct iwx_rxba_data *
> @@ -4895,18 +4820,6 @@ iwx_detect_duplicate(struct iwx_softc *sc, struct mbuf
> return 0;
> }
>
> -/*
> - * Returns true if sn2 - buffer_size < sn1 < sn2.
> - * To be used only in order to compare reorder buffer head with NSSN.
> - * We fully trust NSSN unless it is behind us due to reorder timeout.
> - * Reorder timeout can only bring us up to buffer_size SNs ahead of NSSN.
> - */
> -int
> -iwx_is_sn_less(uint16_t sn1, uint16_t sn2, uint16_t buffer_size)
> -{
> - return SEQ_LT(sn1, sn2) && !SEQ_LT(sn1, sn2 - buffer_size);
> -}
> -
> void
> iwx_release_frames(struct iwx_softc *sc, struct ieee80211_node *ni,
> struct iwx_rxba_data *rxba, struct iwx_reorder_buffer *reorder_buf,
> @@ -4915,11 +4828,7 @@ iwx_release_frames(struct iwx_softc *sc, struct ieee80
> struct iwx_reorder_buf_entry *entries = &rxba->entries[0];
> uint16_t ssn = reorder_buf->head_sn;
>
> - /* ignore nssn smaller than head sn - this can happen due to timeout */
> - if (iwx_is_sn_less(nssn, ssn, reorder_buf->buf_size))
> - goto set_timer;
> -
> - while (iwx_is_sn_less(ssn, nssn, reorder_buf->buf_size)) {
> + while (SEQ_LT(ssn, nssn)) {
> int index = ssn % reorder_buf->buf_size;
> struct mbuf *m;
> int chanidx, is_shortpre;
> @@ -4956,63 +4865,8 @@ iwx_release_frames(struct iwx_softc *sc, struct ieee80
> ssn = (ssn + 1) & 0xfff;
> }
> reorder_buf->head_sn = nssn;
> -
> -set_timer:
> - if (reorder_buf->num_stored && !reorder_buf->removed) {
> - timeout_add_usec(&reorder_buf->reorder_timer,
> - RX_REORDER_BUF_TIMEOUT_MQ_USEC);
> - } else
> - timeout_del(&reorder_buf->reorder_timer);
> }
>
> -int
> -iwx_oldsn_workaround(struct iwx_softc *sc, struct ieee80211_node *ni, int tid,
> - struct iwx_reorder_buffer *buffer, uint32_t reorder_data, uint32_t gp2)
> -{
> - struct ieee80211com *ic = &sc->sc_ic;
> -
> - if (gp2 != buffer->consec_oldsn_ampdu_gp2) {
> - /* we have a new (A-)MPDU ... */
> -
> - /*
> - * reset counter to 0 if we didn't have any oldsn in
> - * the last A-MPDU (as detected by GP2 being identical)
> - */
> - if (!buffer->consec_oldsn_prev_drop)
> - buffer->consec_oldsn_drops = 0;
> -
> - /* either way, update our tracking state */
> - buffer->consec_oldsn_ampdu_gp2 = gp2;
> - } else if (buffer->consec_oldsn_prev_drop) {
> - /*
> - * tracking state didn't change, and we had an old SN
> - * indication before - do nothing in this case, we
> - * already noted this one down and are waiting for the
> - * next A-MPDU (by GP2)
> - */
> - return 0;
> - }
> -
> - /* return unless this MPDU has old SN */
> - if (!(reorder_data & IWX_RX_MPDU_REORDER_BA_OLD_SN))
> - return 0;
> -
> - /* update state */
> - buffer->consec_oldsn_prev_drop = 1;
> - buffer->consec_oldsn_drops++;
> -
> - /* if limit is reached, send del BA and reset state */
> - if (buffer->consec_oldsn_drops == IWX_AMPDU_CONSEC_DROPS_DELBA) {
> - ieee80211_delba_request(ic, ni, IEEE80211_REASON_UNSPECIFIED,
> - 0, tid);
> - buffer->consec_oldsn_prev_drop = 0;
> - buffer->consec_oldsn_drops = 0;
> - return 1;
> - }
> -
> - return 0;
> -}
> -
> /*
> * Handle re-ordering of frames which were de-aggregated in hardware.
> * Returns 1 if the MPDU was consumed (buffered or dropped).
> @@ -5034,8 +4888,6 @@ iwx_rx_reorder(struct iwx_softc *sc, struct mbuf *m, i
> int last_subframe =
> (desc->amsdu_info & IWX_RX_MPDU_AMSDU_LAST_SUBFRAME);
> uint8_t tid;
> - uint8_t subframe_idx = (desc->amsdu_info &
> - IWX_RX_MPDU_AMSDU_SUBFRAME_IDX_MASK);
> struct iwx_reorder_buf_entry *entries;
> int index;
> uint16_t nssn, sn;
> @@ -5100,39 +4952,19 @@ iwx_rx_reorder(struct iwx_softc *sc, struct mbuf *m, i
> goto drop;
> }
>
> - /*
> - * If there was a significant jump in the nssn - adjust.
> - * If the SN is smaller than the NSSN it might need to first go into
> - * the reorder buffer, in which case we just release up to it and the
> - * rest of the function will take care of storing it and releasing up to
> - * the nssn.
> - */
> - if (!iwx_is_sn_less(nssn, buffer->head_sn + buffer->buf_size,
> - buffer->buf_size) ||
> - !SEQ_LT(sn, buffer->head_sn + buffer->buf_size)) {
> - uint16_t min_sn = SEQ_LT(sn, nssn) ? sn : nssn;
> - ic->ic_stats.is_ht_rx_frame_above_ba_winend++;
> - iwx_release_frames(sc, ni, rxba, buffer, min_sn, ml);
> - }
> -
> - if (iwx_oldsn_workaround(sc, ni, tid, buffer, reorder_data,
> - device_timestamp)) {
> - /* BA session will be torn down. */
> - ic->ic_stats.is_ht_rx_ba_window_jump++;
> + /* drop any duplicate packets */
> + if (desc->status & htole32(IWX_RX_MPDU_RES_STATUS_DUPLICATE))
> goto drop;
>
> - }
> -
> /* drop any outdated packets */
> - if (SEQ_LT(sn, buffer->head_sn)) {
> + if (reorder_data & IWX_RX_MPDU_REORDER_BA_OLD_SN) {
> ic->ic_stats.is_ht_rx_frame_below_ba_winstart++;
> goto drop;
> }
>
> /* release immediately if allowed by nssn and no stored frames */
> if (!buffer->num_stored && SEQ_LT(sn, nssn)) {
> - if (iwx_is_sn_less(buffer->head_sn, nssn, buffer->buf_size) &&
> - (!is_amsdu || last_subframe))
> + if (!is_amsdu || last_subframe)
> buffer->head_sn = nssn;
> ieee80211_release_node(ic, ni);
> return 0;
> @@ -5155,24 +4987,7 @@ iwx_rx_reorder(struct iwx_softc *sc, struct mbuf *m, i
>
> index = sn % buffer->buf_size;
>
> - /*
> - * Check if we already stored this frame
> - * As AMSDU is either received or not as whole, logic is simple:
> - * If we have frames in that position in the buffer and the last frame
> - * originated from AMSDU had a different SN then it is a retransmission.
> - * If it is the same SN then if the subframe index is incrementing it
> - * is the same AMSDU - otherwise it is a retransmission.
> - */
> - if (!ml_empty(&entries[index].frames)) {
> - if (!is_amsdu) {
> - ic->ic_stats.is_ht_rx_ba_no_buf++;
> - goto drop;
> - } else if (sn != buffer->last_amsdu ||
> - buffer->last_sub_index >= subframe_idx) {
> - ic->ic_stats.is_ht_rx_ba_no_buf++;
> - goto drop;
> - }
> - } else {
> + if (ml_empty(&entries[index].frames)) {
> /* This data is the same for all A-MSDU subframes. */
> entries[index].chanidx = chanidx;
> entries[index].is_shortpre = is_shortpre;
> @@ -5184,13 +4999,7 @@ iwx_rx_reorder(struct iwx_softc *sc, struct mbuf *m, i
> /* put in reorder buffer */
> ml_enqueue(&entries[index].frames, m);
> buffer->num_stored++;
> - getmicrouptime(&entries[index].reorder_time);
>
> - if (is_amsdu) {
> - buffer->last_amsdu = sn;
> - buffer->last_sub_index = subframe_idx;
> - }
> -
> /*
> * We cannot trust NSSN for AMSDU sub-frames that are not the last.
> * The reason is that NSSN advances on the first sub-frame, and may
> @@ -12308,8 +12117,6 @@ iwx_attach(struct device *parent, struct device *self,
> rxba->sc = sc;
> timeout_set(&rxba->session_timer, iwx_rx_ba_session_expired,
> rxba);
> - timeout_set(&rxba->reorder_buf.reorder_timer,
> - iwx_reorder_timer_expired, &rxba->reorder_buf);
> for (j = 0; j < nitems(rxba->entries); j++)
> ml_init(&rxba->entries[j].frames);
> }
> commit - bf3713f7dfbf19ede85da952a0d01b849aa8a252
> blob - b6d027fedb110fa27bf4696ba713e099273adddc
> file + sys/dev/pci/if_iwxreg.h
> --- sys/dev/pci/if_iwxreg.h
> +++ sys/dev/pci/if_iwxreg.h
> @@ -3632,6 +3632,7 @@ struct iwx_rx_mpdu_res_start {
> #define IWX_RX_MPDU_RES_STATUS_ROBUST_MNG_FRAME (1 << 15)
> #define IWX_RX_MPDU_RES_STATUS_HASH_INDEX_MSK (0x3F0000)
> #define IWX_RX_MPDU_RES_STATUS_STA_ID_MSK (0x1f000000)
> +#define IWX_RX_MPDU_RES_STATUS_DUPLICATE (1 << 22)
> #define IWX_RX_MPDU_RES_STATUS_RRF_KILL (1 << 29)
> #define IWX_RX_MPDU_RES_STATUS_FILTERING_MSK (0xc00000)
> #define IWX_RX_MPDU_RES_STATUS2_FILTERING_MSK (0xc0000000)
> commit - bf3713f7dfbf19ede85da952a0d01b849aa8a252
> blob - 7be82818bc54423e510492e7b7e8974a0f63000d
> file + sys/dev/pci/if_iwxvar.h
> --- sys/dev/pci/if_iwxvar.h
> +++ sys/dev/pci/if_iwxvar.h
> @@ -358,11 +358,6 @@ struct iwx_self_init_dram {
> * @num_stored: number of mpdus stored in the buffer
> * @buf_size: the reorder buffer size as set by the last addba request
> * @queue: queue of this reorder buffer
> - * @last_amsdu: track last ASMDU SN for duplication detection
> - * @last_sub_index: track ASMDU sub frame index for duplication detection
> - * @reorder_timer: timer for frames are in the reorder buffer. For AMSDU
> - * it is the time of last received sub-frame
> - * @removed: prevent timer re-arming
> * @valid: reordering is valid for this queue
> * @consec_oldsn_drops: consecutive drops due to old SN
> * @consec_oldsn_ampdu_gp2: A-MPDU GP2 timestamp to track
> @@ -375,25 +370,15 @@ struct iwx_reorder_buffer {
> uint16_t head_sn;
> uint16_t num_stored;
> uint16_t buf_size;
> - uint16_t last_amsdu;
> - uint8_t last_sub_index;
> - struct timeout reorder_timer;
> - int removed;
> int valid;
> - unsigned int consec_oldsn_drops;
> - uint32_t consec_oldsn_ampdu_gp2;
> - unsigned int consec_oldsn_prev_drop;
> -#define IWX_AMPDU_CONSEC_DROPS_DELBA 10
> };
>
> /**
> * struct iwx_reorder_buf_entry - reorder buffer entry per frame sequence number
> * @frames: list of mbufs stored (A-MSDU subframes share a sequence number)
> - * @reorder_time: time the packet was stored in the reorder buffer
> */
> struct iwx_reorder_buf_entry {
> struct mbuf_list frames;
> - struct timeval reorder_time;
> uint32_t rx_pkt_status;
> int chanidx;
> int is_shortpre;
>
>
iwx: remove A-MPDU Rx reordering