git: aa266fad5829 - main - net80211: refactor sequence number assignment code
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Fri, 16 May 2025 02:34:47 UTC
The branch main has been updated by adrian:
URL: https://cgit.FreeBSD.org/src/commit/?id=aa266fad58294763dc07b94b33b15318e134826b
commit aa266fad58294763dc07b94b33b15318e134826b
Author: Adrian Chadd <adrian@FreeBSD.org>
AuthorDate: 2025-01-25 22:25:18 +0000
Commit: Adrian Chadd <adrian@FreeBSD.org>
CommitDate: 2025-05-16 02:33:27 +0000
net80211: refactor sequence number assignment code
Refactor out the sequence number assignment code for normal frames
and beacons.
Document the behaviour around fragments and packet lists.
Right now this should be a no-op; there's some verification code
in here to make sure that the TID selected by the existing code
matches the TID populated in the passed in mbuf/frame.
Locally tested:
* rtwn(4), STA mode
Differential Revision: https://reviews.freebsd.org/D49764
Reviewed by: bz
---
sys/net80211/ieee80211_output.c | 159 +++++++++++++++++++++++++---------------
sys/net80211/ieee80211_proto.h | 4 +
2 files changed, 103 insertions(+), 60 deletions(-)
diff --git a/sys/net80211/ieee80211_output.c b/sys/net80211/ieee80211_output.c
index 1ebf7fa8723f..1f726f75b6c6 100644
--- a/sys/net80211/ieee80211_output.c
+++ b/sys/net80211/ieee80211_output.c
@@ -892,7 +892,6 @@ ieee80211_send_setup(
struct ieee80211vap *vap = ni->ni_vap;
struct ieee80211_tx_ampdu *tap;
struct ieee80211_frame *wh = mtod(m, struct ieee80211_frame *);
- ieee80211_seq seqno;
IEEE80211_TX_LOCK_ASSERT(ni->ni_ic);
@@ -975,25 +974,8 @@ ieee80211_send_setup(
/* NB: zero out i_seq field (for s/w encryption etc) */
*(uint16_t *)&wh->i_seq[0] = 0;
- } else {
- if (IEEE80211_HAS_SEQ(type & IEEE80211_FC0_TYPE_MASK,
- type & IEEE80211_FC0_SUBTYPE_MASK))
- /*
- * 802.11-2012 9.3.2.10 - QoS multicast frames
- * come out of a different seqno space.
- */
- if (IEEE80211_IS_MULTICAST(wh->i_addr1)) {
- seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++;
- } else {
- seqno = ni->ni_txseqs[tid]++;
- }
- else
- seqno = 0;
-
- *(uint16_t *)&wh->i_seq[0] =
- htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT);
- M_SEQNO_SET(m, seqno);
- }
+ } else
+ ieee80211_output_seqno_assign(ni, tid, m);
if (IEEE80211_IS_MULTICAST(wh->i_addr1))
m->m_flags |= M_MCAST;
@@ -1483,7 +1465,6 @@ ieee80211_encap(struct ieee80211vap *vap, struct ieee80211_node *ni,
struct ieee80211_key *key;
struct llc *llc;
int hdrsize, hdrspace, datalen, addqos, txfrag, is4addr, is_mcast;
- ieee80211_seq seqno;
int meshhdrsize, meshae;
uint8_t *qos;
int is_amsdu = 0;
@@ -1829,22 +1810,8 @@ ieee80211_encap(struct ieee80211vap *vap, struct ieee80211_node *ni,
* and we don't need the TX lock held.
*/
if ((m->m_flags & M_AMPDU_MPDU) == 0) {
- /*
- * 802.11-2012 9.3.2.10 -
- *
- * If this is a multicast frame then we need
- * to ensure that the sequence number comes from
- * a separate seqno space and not the TID space.
- *
- * Otherwise multicast frames may actually cause
- * holes in the TX blockack window space and
- * upset various things.
- */
- if (IEEE80211_IS_MULTICAST(wh->i_addr1))
- seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++;
- else
- seqno = ni->ni_txseqs[tid]++;
-
+ ieee80211_output_seqno_assign(ni, tid, m);
+ } else {
/*
* NB: don't assign a sequence # to potential
* aggregates; we expect this happens at the
@@ -1857,24 +1824,11 @@ ieee80211_encap(struct ieee80211vap *vap, struct ieee80211_node *ni,
* capability; this may also change when we pull
* aggregation up into net80211
*/
- *(uint16_t *)wh->i_seq =
- htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT);
- M_SEQNO_SET(m, seqno);
- } else {
/* NB: zero out i_seq field (for s/w encryption etc) */
*(uint16_t *)wh->i_seq = 0;
}
} else {
- /*
- * XXX TODO TX lock is needed for atomic updates of sequence
- * numbers. If the driver does it, then don't do it here;
- * and we don't need the TX lock held.
- */
- seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++;
- *(uint16_t *)wh->i_seq =
- htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT);
- M_SEQNO_SET(m, seqno);
-
+ ieee80211_output_seqno_assign(ni, IEEE80211_NONQOS_TID, m);
/*
* XXX TODO: we shouldn't allow EAPOL, etc that would
* be forced to be non-QoS traffic to be A-MSDU encapsulated.
@@ -1975,7 +1929,6 @@ ieee80211_free_mbuf(struct mbuf *m)
* This implements the fragmentation part of 802.11-2016 10.2.7
* (Fragmentation/defragmentation overview.)
*
- * Fragment the frame according to the specified mtu.
* The size of the 802.11 header (w/o padding) is provided
* so we don't need to recalculate it. We create a new
* mbuf for each fragment and chain it through m_nextpkt;
@@ -3827,8 +3780,6 @@ ieee80211_beacon_update(struct ieee80211_node *ni, struct mbuf *m, int mcast)
struct ieee80211com *ic = ni->ni_ic;
int len_changed = 0;
uint16_t capinfo;
- struct ieee80211_frame *wh;
- ieee80211_seq seqno;
IEEE80211_LOCK(ic);
/*
@@ -3896,8 +3847,6 @@ ieee80211_beacon_update(struct ieee80211_node *ni, struct mbuf *m, int mcast)
return 1; /* just assume length changed */
}
- wh = mtod(m, struct ieee80211_frame *);
-
/*
* XXX TODO Strictly speaking this should be incremented with the TX
* lock held so as to serialise access to the non-qos TID sequence
@@ -3906,10 +3855,7 @@ ieee80211_beacon_update(struct ieee80211_node *ni, struct mbuf *m, int mcast)
* If the driver identifies it does its own TX seqno management then
* we can skip this (and still not do the TX seqno.)
*/
- seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++;
- *(uint16_t *)&wh->i_seq[0] =
- htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT);
- M_SEQNO_SET(m, seqno);
+ ieee80211_output_beacon_seqno_assign(ni, m);
/* XXX faster to recalculate entirely or just changes? */
capinfo = ieee80211_getcapinfo(vap, ni->ni_chan);
@@ -4241,3 +4187,96 @@ ieee80211_tx_complete(struct ieee80211_node *ni, struct mbuf *m, int status)
}
m_freem(m);
}
+
+/**
+ * @brief Assign a sequence number to the given frame.
+ *
+ * Check the frame type and TID and assign a suitable sequence number
+ * from the correct sequence number space.
+ *
+ * It assumes the mbuf has been encapsulated, and has the TID assigned
+ * if it is a QoS frame.
+ *
+ * Note this also clears any existing fragment ID in the header, so it
+ * must be called first before assigning fragment IDs.
+ *
+ * For now this implements parts of 802.11-2012; it doesn't do all of
+ * the needed checks for full compliance (notably QoS-Data NULL frames).
+ *
+ * TODO: update to 802.11-2020 10.3.2.14.2 (Transmitter Requirements)
+ *
+ * @param ni ieee80211_node this frame will be transmitted to
+ * @param arg_tid A temporary check, existing callers may set
+ * this to a TID variable they were using, and this routine
+ * will verify it against what's in the frame and complain if
+ * they don't match. For new callers, use -1.
+ * @param m mbuf to populate the sequence number into
+ */
+void
+ieee80211_output_seqno_assign(struct ieee80211_node *ni, int arg_tid,
+ struct mbuf *m)
+{
+ struct ieee80211_frame *wh;
+ ieee80211_seq seqno;
+ uint8_t tid, type, subtype;
+
+ wh = mtod(m, struct ieee80211_frame *);
+ tid = ieee80211_gettid(wh);
+ type = wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK;
+ subtype = wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK;
+
+ /*
+ * Find places where the passed in TID doesn't match gettid()
+ * and log. I'll have to then go and chase those down.
+ *
+ * If the caller knows its already setup the TID in the frame
+ * correctly then it can pass in -1 and this check will be
+ * skipped.
+ */
+ if (arg_tid != -1 && tid != arg_tid)
+ ic_printf(ni->ni_vap->iv_ic,
+ "%s: called; TID mismatch; tid=%u, arg_tid=%d\n",
+ __func__, tid, arg_tid);
+
+ if (IEEE80211_HAS_SEQ(type, subtype)) {
+ /*
+ * 802.11-2012 9.3.2.10 - QoS multicast frames
+ * come out of a different seqno space.
+ */
+ if (IEEE80211_IS_MULTICAST(wh->i_addr1))
+ seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++;
+ else
+ seqno = ni->ni_txseqs[tid]++;
+ } else
+ seqno = 0;
+
+ /*
+ * Assign the sequence number, clearing out any existing
+ * sequence and fragment numbers.
+ */
+ *(uint16_t *)&wh->i_seq[0] =
+ htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT);
+ M_SEQNO_SET(m, seqno);
+}
+
+/**
+ * @brief Assign a sequence number to the given beacon frame.
+ *
+ * TODO: update to 802.11-2020 10.3.2.14.2 (Transmitter Requirements)
+ *
+ * @param ni ieee80211_node this frame will be transmitted to
+ * @param m mbuf to populate the sequence number into
+ */
+void
+ieee80211_output_beacon_seqno_assign(struct ieee80211_node *ni, struct mbuf *m)
+{
+ struct ieee80211_frame *wh;
+ ieee80211_seq seqno;
+
+ wh = mtod(m, struct ieee80211_frame *);
+
+ seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++;
+ *(uint16_t *)&wh->i_seq[0] =
+ htole16(seqno << IEEE80211_SEQ_SEQ_SHIFT);
+ M_SEQNO_SET(m, seqno);
+}
diff --git a/sys/net80211/ieee80211_proto.h b/sys/net80211/ieee80211_proto.h
index 9ab7e644c89d..32745264b0ab 100644
--- a/sys/net80211/ieee80211_proto.h
+++ b/sys/net80211/ieee80211_proto.h
@@ -123,6 +123,10 @@ struct mbuf * ieee80211_ff_encap1(struct ieee80211vap *, struct mbuf *,
const struct ether_header *);
void ieee80211_tx_complete(struct ieee80211_node *,
struct mbuf *, int);
+void ieee80211_output_seqno_assign(struct ieee80211_node *,
+ int, struct mbuf *);
+void ieee80211_output_beacon_seqno_assign(struct ieee80211_node *,
+ struct mbuf *);
/*
* The formation of ProbeResponse frames requires guidance to