git: 37e54466cf7a - main - net80211: clean up / add more macros to check the frame types

Go to: [ bottom of page ] [ top of archives ] [ this month ]
From: Adrian Chadd <adrian_at_FreeBSD.org>
Date: Mon, 11 Nov 2024 01:12:20 UTC
The branch main has been updated by adrian:

URL: https://cgit.FreeBSD.org/src/commit/?id=37e54466cf7a7c60f3d57c23c09b832db876e2fc

commit 37e54466cf7a7c60f3d57c23c09b832db876e2fc
Author:     Adrian Chadd <adrian@FreeBSD.org>
AuthorDate: 2024-11-09 21:10:08 +0000
Commit:     Adrian Chadd <adrian@FreeBSD.org>
CommitDate: 2024-11-11 01:11:58 +0000

    net80211: clean up / add more macros to check the frame types
    
    * Add new macros to check the version+type and version+type+subtype of a frame.
    * Use these for existing frame checks.
    * Convert the flag checks in net80211 to use the macros, rather than direct
      header poking.
    
    Notably I'm callign out things like QOS any versus QOS data, the
    kind of NULL frames, etc.  Eg, in the TKIP code it's checking whether
    a frame is ANY kind of QOS frame, not just QOS data.
    
    These macros should hopefully make the header checks clearer and less
    error prone.  They're also useful in drivers that are doing their
    own header parsing.
    
    Locally:
    
    * ath(4), AP, STA, AP+STA modes
    * local ath10k/athp - AP, STA modes
    * rtwn - STA mode
    
    Differential Revision: https://reviews.freebsd.org/D36615
---
 sys/net80211/ieee80211.h             | 54 ++++++++++++++++++++++++++++--------
 sys/net80211/ieee80211_adhoc.c       |  3 +-
 sys/net80211/ieee80211_crypto_tkip.c |  3 +-
 sys/net80211/ieee80211_hostap.c      |  3 +-
 sys/net80211/ieee80211_ht.c          |  2 +-
 sys/net80211/ieee80211_mesh.c        |  3 +-
 sys/net80211/ieee80211_output.c      |  3 +-
 sys/net80211/ieee80211_sta.c         |  3 +-
 sys/net80211/ieee80211_wds.c         |  3 +-
 9 files changed, 52 insertions(+), 25 deletions(-)

diff --git a/sys/net80211/ieee80211.h b/sys/net80211/ieee80211.h
index 4203952367ca..a2d6b3040032 100644
--- a/sys/net80211/ieee80211.h
+++ b/sys/net80211/ieee80211.h
@@ -161,6 +161,7 @@ struct ieee80211_qosframe_addr4 {
 /* 0001-0011 Reserved				0x10-0x30 */	/* Were: CF_ACK, CF_POLL, CF_ACPL */
 #define	IEEE80211_FC0_SUBTYPE_NODATA		0x40	/* Null */
 /* 0101-0111 Reserved				0x50-0x70 */	/* Were: CFACK, CFPOLL, CF_ACK_CF_ACK */
+#define	IEEE80211_FC0_SUBTYPE_QOS_MASK_ANY	0x80	/* QoS mask - matching any subtypes 8..15 */
 #define	IEEE80211_FC0_SUBTYPE_QOS_DATA		0x80	/* QoS Data */
 #define	IEEE80211_FC0_SUBTYPE_QOS_DATA_CFACK	0x90	/* QoS Data +CF-Ack */
 #define	IEEE80211_FC0_SUBTYPE_QOS_DATA_CFPOLL	0xa0	/* QoS Data +CF-Poll */
@@ -190,24 +191,55 @@ struct ieee80211_qosframe_addr4 {
 #define	IEEE80211_CTL_EXT_TDD_BF		0x0b	/* TDD Beamforming, 80211ay-2021 */
 /* 1100-1111 Reserved				0xc-0xf */
 
-#define	IEEE80211_IS_MGMT(wh)					\
-	(!! (((wh)->i_fc[0] & IEEE80211_FC0_TYPE_MASK)		\
-	    == IEEE80211_FC0_TYPE_MGT))
+/* Check the version field */
+#define	IEEE80211_IS_FC0_CHECK_VER(wh, v)			\
+	(((wh)->i_fc[0] & IEEE80211_FC0_VERSION_MASK) == (v))
+
+/* Check the version and type field */
+#define	IEEE80211_IS_FC0_CHECK_VER_TYPE(wh, v, t)			\
+	(((((wh)->i_fc[0] & IEEE80211_FC0_VERSION_MASK) == (v))) &&	\
+	  (((wh)->i_fc[0] & IEEE80211_FC0_TYPE_MASK) == (t)))
+
+/* Check the version, type and subtype field */
+#define	IEEE80211_IS_FC0_CHECK_VER_TYPE_SUBTYPE(wh, v, t, st)		\
+	(((((wh)->i_fc[0] & IEEE80211_FC0_VERSION_MASK) == (v))) &&	\
+	  (((wh)->i_fc[0] & IEEE80211_FC0_TYPE_MASK) == (t)) &&		\
+	  (((wh)->i_fc[0] & IEEE80211_FC0_SUBTYPE_MASK) == (st)))
+
+#define	IEEE80211_IS_MGMT(wh)						\
+	(IEEE80211_IS_FC0_CHECK_VER_TYPE(wh, IEEE80211_FC0_VERSION_0,	\
+	 IEEE80211_FC0_TYPE_MGT))
 #define	IEEE80211_IS_CTL(wh)					\
-	(!! (((wh)->i_fc[0] & IEEE80211_FC0_TYPE_MASK)		\
-	    == IEEE80211_FC0_TYPE_CTL))
+	(IEEE80211_IS_FC0_CHECK_VER_TYPE(wh, IEEE80211_FC0_VERSION_0,	\
+	 IEEE80211_FC0_TYPE_CTL))
 #define	IEEE80211_IS_DATA(wh)					\
-	(!! (((wh)->i_fc[0] & IEEE80211_FC0_TYPE_MASK)		\
-	    == IEEE80211_FC0_TYPE_DATA))
+	(IEEE80211_IS_FC0_CHECK_VER_TYPE(wh, IEEE80211_FC0_VERSION_0,	\
+	 IEEE80211_FC0_TYPE_DATA))
 #define	IEEE80211_IS_EXT(wh)					\
-	(!! (((wh)->i_fc[0] & IEEE80211_FC0_TYPE_MASK)		\
-	    == IEEE80211_FC0_TYPE_EXT))
+	(IEEE80211_IS_FC0_CHECK_VER_TYPE(wh, IEEE80211_FC0_VERSION_0,	\
+	 IEEE80211_FC0_TYPE_EXT))
 
 #define	IEEE80211_FC0_QOSDATA \
 	(IEEE80211_FC0_TYPE_DATA|IEEE80211_FC0_SUBTYPE_QOS_DATA|IEEE80211_FC0_VERSION_0)
 
-#define	IEEE80211_IS_QOSDATA(wh) \
-	((wh)->i_fc[0] == IEEE80211_FC0_QOSDATA)
+/*
+ * Return true if the frame is any of the QOS frame types, not just
+ * data frames.  Matching on the IEEE80211_FC0_SUBTYPE_QOS_ANY bit
+ * being set also matches on subtypes 8..15.
+ */
+#define	IEEE80211_IS_QOS_ANY(wh)					\
+	((IEEE80211_IS_FC0_CHECK_VER_TYPE(wh, IEEE80211_FC0_VERSION_0,	\
+	 IEEE80211_FC0_TYPE_DATA)) &&					\
+	 ((wh)->i_fc[0] & IEEE80211_FC0_SUBTYPE_QOS_MASK_ANY))
+
+/*
+ * Return true if this frame is QOS data, and only QOS data.
+ */
+#define	IEEE80211_IS_QOSDATA(wh)			\
+	(IEEE80211_IS_FC0_CHECK_VER_TYPE_SUBTYPE(wh,	\
+	 IEEE80211_FC0_VERSION_0,			\
+	 IEEE80211_FC0_TYPE_DATA,			\
+	 IEEE80211_FC0_SUBTYPE_QOS_DATA))
 
 #define	IEEE80211_FC1_DIR_MASK			0x03
 #define	IEEE80211_FC1_DIR_NODS			0x00	/* STA->STA */
diff --git a/sys/net80211/ieee80211_adhoc.c b/sys/net80211/ieee80211_adhoc.c
index f5e8a301ad28..d252b75899a2 100644
--- a/sys/net80211/ieee80211_adhoc.c
+++ b/sys/net80211/ieee80211_adhoc.c
@@ -362,8 +362,7 @@ adhoc_input(struct ieee80211_node *ni, struct mbuf *m,
 	 */
 	wh = mtod(m, struct ieee80211_frame *);
 
-	if ((wh->i_fc[0] & IEEE80211_FC0_VERSION_MASK) !=
-	    IEEE80211_FC0_VERSION_0) {
+	if (!IEEE80211_IS_FC0_CHECK_VER(wh, IEEE80211_FC0_VERSION_0)) {
 		IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_ANY,
 		    ni->ni_macaddr, NULL, "wrong version, fc %02x:%02x",
 		    wh->i_fc[0], wh->i_fc[1]);
diff --git a/sys/net80211/ieee80211_crypto_tkip.c b/sys/net80211/ieee80211_crypto_tkip.c
index d2bc281f15a2..4cfb7542f8ff 100644
--- a/sys/net80211/ieee80211_crypto_tkip.c
+++ b/sys/net80211/ieee80211_crypto_tkip.c
@@ -860,7 +860,8 @@ michael_mic_hdr(const struct ieee80211_frame *wh0, uint8_t hdr[16])
 		break;
 	}
 
-	if (wh->i_fc[0] & IEEE80211_FC0_SUBTYPE_QOS_DATA) {
+	/* Match on any QOS frame, not just data */
+	if (IEEE80211_IS_QOS_ANY(wh)) {
 		const struct ieee80211_qosframe *qwh =
 			(const struct ieee80211_qosframe *) wh;
 		hdr[12] = qwh->i_qos[0] & IEEE80211_QOS_TID;
diff --git a/sys/net80211/ieee80211_hostap.c b/sys/net80211/ieee80211_hostap.c
index ac97889a9cef..1dce9a6b5923 100644
--- a/sys/net80211/ieee80211_hostap.c
+++ b/sys/net80211/ieee80211_hostap.c
@@ -532,8 +532,7 @@ hostap_input(struct ieee80211_node *ni, struct mbuf *m,
 	 */
 	wh = mtod(m, struct ieee80211_frame *);
 
-	if ((wh->i_fc[0] & IEEE80211_FC0_VERSION_MASK) !=
-	    IEEE80211_FC0_VERSION_0) {
+	if (!IEEE80211_IS_FC0_CHECK_VER(wh, IEEE80211_FC0_VERSION_0)) {
 		IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_ANY,
 		    ni->ni_macaddr, NULL, "wrong version, fc %02x:%02x",
 		    wh->i_fc[0], wh->i_fc[1]);
diff --git a/sys/net80211/ieee80211_ht.c b/sys/net80211/ieee80211_ht.c
index c747d29735a6..28c329ce3d32 100644
--- a/sys/net80211/ieee80211_ht.c
+++ b/sys/net80211/ieee80211_ht.c
@@ -1014,7 +1014,7 @@ ieee80211_ampdu_reorder(struct ieee80211_node *ni, struct mbuf *m,
 
 	/* NB: m_len known to be sufficient */
 	wh = mtod(m, struct ieee80211_qosframe *);
-	if (wh->i_fc[0] != IEEE80211_FC0_QOSDATA) {
+	if (!IEEE80211_IS_QOSDATA(wh)) {
 		/*
 		 * Not QoS data, shouldn't get here but just
 		 * return it to the caller for processing.
diff --git a/sys/net80211/ieee80211_mesh.c b/sys/net80211/ieee80211_mesh.c
index 8359ea8878d2..c52122ebeb13 100644
--- a/sys/net80211/ieee80211_mesh.c
+++ b/sys/net80211/ieee80211_mesh.c
@@ -1564,8 +1564,7 @@ mesh_input(struct ieee80211_node *ni, struct mbuf *m,
 	*/
 	wh = mtod(m, struct ieee80211_frame *);
 
-	if ((wh->i_fc[0] & IEEE80211_FC0_VERSION_MASK) !=
-	    IEEE80211_FC0_VERSION_0) {
+	if (!IEEE80211_IS_FC0_CHECK_VER(wh, IEEE80211_FC0_VERSION_0)) {
 		IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_ANY,
 		    ni->ni_macaddr, NULL, "wrong version %x", wh->i_fc[0]);
 		vap->iv_stats.is_rx_badversion++;
diff --git a/sys/net80211/ieee80211_output.c b/sys/net80211/ieee80211_output.c
index 22bbfe98f4b5..44903ed366fd 100644
--- a/sys/net80211/ieee80211_output.c
+++ b/sys/net80211/ieee80211_output.c
@@ -603,8 +603,7 @@ ieee80211_validate_frame(struct mbuf *m,
 		return (EINVAL);
 
 	wh = mtod(m, struct ieee80211_frame *);
-	if ((wh->i_fc[0] & IEEE80211_FC0_VERSION_MASK) !=
-	    IEEE80211_FC0_VERSION_0)
+	if (!IEEE80211_IS_FC0_CHECK_VER(wh, IEEE80211_FC0_VERSION_0))
 		return (EINVAL);
 
 	type = wh->i_fc[0] & IEEE80211_FC0_TYPE_MASK;
diff --git a/sys/net80211/ieee80211_sta.c b/sys/net80211/ieee80211_sta.c
index 8fd4de162359..97ed52295d6d 100644
--- a/sys/net80211/ieee80211_sta.c
+++ b/sys/net80211/ieee80211_sta.c
@@ -568,8 +568,7 @@ sta_input(struct ieee80211_node *ni, struct mbuf *m,
 		vap->iv_stats.is_rx_tooshort++;
 		goto err;
 	}
-	if ((wh->i_fc[0] & IEEE80211_FC0_VERSION_MASK) !=
-	    IEEE80211_FC0_VERSION_0) {
+	if (!IEEE80211_IS_FC0_CHECK_VER(wh, IEEE80211_FC0_VERSION_0)) {
 		IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_ANY,
 		    ni->ni_macaddr, NULL, "wrong version, fc %02x:%02x",
 		    wh->i_fc[0], wh->i_fc[1]);
diff --git a/sys/net80211/ieee80211_wds.c b/sys/net80211/ieee80211_wds.c
index 79c2d8a14e00..19827e78706a 100644
--- a/sys/net80211/ieee80211_wds.c
+++ b/sys/net80211/ieee80211_wds.c
@@ -465,8 +465,7 @@ wds_input(struct ieee80211_node *ni, struct mbuf *m,
 	if (!IEEE80211_IS_MULTICAST(wh->i_addr1))
 		ni->ni_inact = ni->ni_inact_reload;
 
-	if ((wh->i_fc[0] & IEEE80211_FC0_VERSION_MASK) !=
-	    IEEE80211_FC0_VERSION_0) {
+	if (!IEEE80211_IS_FC0_CHECK_VER(wh, IEEE80211_FC0_VERSION_0)) {
 		IEEE80211_DISCARD_MAC(vap, IEEE80211_MSG_ANY,
 		    ni->ni_macaddr, NULL, "wrong version, fc %02x:%02x",
 		    wh->i_fc[0], wh->i_fc[1]);