From nobody Fri May 16 02:34:47 2025 X-Original-To: dev-commits-src-all@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 4ZzB4S0VfRz5vtDP; Fri, 16 May 2025 02:34:48 +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 "R11" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4ZzB4R4zvfz3xfF; Fri, 16 May 2025 02:34:47 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1747362887; 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=oyIDHLgUhIBix10ggzEKLC3TWpODkr45R9vZQvL2yxM=; b=uSEiqoI2vMQF/dRoZnirwdLr3T3xvmQfcfxj9ipQYipndL8TkesMWHMYSVf2ZahgfAaFAI mGKrGUdZ7F+8ispRYGvlNr7uTWeU4B9nTQ22DAMKwbCoAU1KSi5zXdkCNbxFiWkEaM44GV PUTqUYsylcoMtHSSrm7D3SpIbP/Ib5Sj3gAPeY4Q4m1zrjZX1kWpES4Cwg4+F87iDFB0LP ZOy3H9fGsIqCa4/LP18U6numYgCZB9Iv8F9G22fpNxKRBBrVWXYqTyF4YIhTKNzsQ2zqH2 JdbZyMJ0xp90Q6a30ZoWKp2fpp+L0qujdBsuRotDb2UziULN/tBnKyMxUzMeQQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1747362887; 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=oyIDHLgUhIBix10ggzEKLC3TWpODkr45R9vZQvL2yxM=; b=WUIA8sxl5jCdR+0vxjM71VEX69/VYbj2CWrI0t6pen3+rDnu/IZ9tmaJa6Kv6Jk6+GZA6H oEcqUodO679wIkGt2qshi/rH8KzsWjkXnAUpAQH+tm07RdwXVpIh4GyKSXEr9S0yIB5XpP mAazLlxqA+JaAdky2HgGRc7xp+gb7gjG/B1HkNVGLBiqADmjT+huEsjhGnvq3dh89R9ZFL i/js7v3+BbhnylFOh/c4gfjieTSgg5oOrD9l0bAdpi5JL8HJudn4luc2A4iqlspDY67DER uI5kn9A3cd3+VTKwSnPpKg37UN/r3UUQGT+YxTEQjcKUB03fXlC7Euly7IJgQA== ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1747362887; a=rsa-sha256; cv=none; b=IMlYygd28mgS64pyh7o1ho+6gnIMvm+GtTTdWkpnS8pGi614DBdFOfPyA8KLFrMGEX5m4W 93frpQxVQBBSQk9az+Oct77F2nPeiuZeY4nYyCdgAtL1K1zBSTqdGnvW3iGGogzgCRZaSe efGzB2qmKeVvty/KZfNcncF/WitsnWMm8RmH/H8yFtPc6r7Hmh/o+g+2ATGcrxXa/9s5+8 ITxDu6s4C7qhM7vEhEl2ED2ndUoW3U3GaX0qTQmw3QurVDDijfIGZl3ERjIt4LUyGcePM5 gEEDNGIEf/XyLPgPh8C5jUt9AIF2plaDTtVVPGn2WCD7p6Y8c/wfkP67PMMIgg== ARC-Authentication-Results: i=1; mx1.freebsd.org; none 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 4ZzB4R4GkTzl2f; Fri, 16 May 2025 02:34:47 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 54G2Ylld026275; Fri, 16 May 2025 02:34:47 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 54G2YlcP026272; Fri, 16 May 2025 02:34:47 GMT (envelope-from git) Date: Fri, 16 May 2025 02:34:47 GMT Message-Id: <202505160234.54G2YlcP026272@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Adrian Chadd Subject: git: aa266fad5829 - main - net80211: refactor sequence number assignment code List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: adrian X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: aa266fad58294763dc07b94b33b15318e134826b Auto-Submitted: auto-generated The branch main has been updated by adrian: URL: https://cgit.FreeBSD.org/src/commit/?id=aa266fad58294763dc07b94b33b15318e134826b commit aa266fad58294763dc07b94b33b15318e134826b Author: Adrian Chadd AuthorDate: 2025-01-25 22:25:18 +0000 Commit: Adrian Chadd 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