Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
replace ieee80211_chan2mode()
To:
tech@openbsd.org
Date:
Fri, 1 Aug 2025 13:16:19 +0200

Download raw body.

Thread
ieee80211_chan2mode() is broken in several ways.

chan2mode does not detect 11g APs properly. This can be fixed now that
we are setting the ERP flag not only in hostap mode but also in client
mode.

If chan2mode is called while our phy mode is fixed, then it always
returns our fixed phy mode. This means it wrongly selects 11g rates if
our phy mode is fixed to 11g even if we are talking to an 11b-only peer
which cannot receive 11g rates.

chan2mode tries to handle a NULL channel which is not relevant to any
of the callers.

I want to replace chan2mode with a new ieee80211_node_abg_mode() function
which serves the same purpose but does not have any of the above issues.

Tested with qwx(4), which can now automatically associate with an AP which
denies clients that use 11b data rates. Previously, 'ifconfig qwx0 mode 11g'
was required to associate with such APs.

ok?

(The chunk in ieee80211_create_ibss() is not using node_abg_mode() because
when we are starting a new hostap we only need to take our own capabilities
into account. And the ERP flag won't be set on ic_bss, which causes
node_abg_mode() to return 11b while in 11g mode. An 11g hostap needs 11g.)

M  sys/dev/ic/ath.c                 |   6+   5-
M  sys/net80211/ieee80211.c         |   0+  49-
M  sys/net80211/ieee80211_input.c   |   1+   1-
M  sys/net80211/ieee80211_node.c    |  58+   6-
M  sys/net80211/ieee80211_node.h    |   2+   0-
M  sys/net80211/ieee80211_output.c  |   1+   1-
M  sys/net80211/ieee80211_proto.c   |   3+   3-
M  sys/net80211/ieee80211_var.h     |   0+   2-

8 files changed, 71 insertions(+), 67 deletions(-)

commit - aec0df7811c350695d61f4bd9fd523d5a656341b
commit + 488ec86079a70aab9c7bbd976d40b5811ea37ee1
blob - 8a991f09c1ecafe9ee705a243f15603894ec2078
blob + 4e65814965c5795a9b1a20487a0ba666e61ae7a1
--- sys/dev/ic/ath.c
+++ sys/dev/ic/ath.c
@@ -113,7 +113,7 @@ void	ath_rx_proc(void *, int);
 int	ath_tx_start(struct ath_softc *, struct ieee80211_node *,
 	    struct ath_buf *, struct mbuf *);
 void	ath_tx_proc(void *, int);
-int	ath_chan_set(struct ath_softc *, struct ieee80211_channel *);
+int	ath_chan_set(struct ath_softc *, struct ieee80211_node *);
 void	ath_draintxq(struct ath_softc *);
 void	ath_stoprecv(struct ath_softc *);
 int	ath_startrecv(struct ath_softc *);
@@ -675,7 +675,7 @@ ath_init1(struct ath_softc *sc)
 	 */
 	ni = ic->ic_bss;
 	ni->ni_chan = ic->ic_ibss_chan;
-	mode = ieee80211_chan2mode(ic, ni->ni_chan);
+	mode = ieee80211_node_abg_mode(ic, ni);
 	if (mode != sc->sc_curmode)
 		ath_setcurmode(sc, mode);
 	if (ic->ic_opmode != IEEE80211_M_MONITOR) {
@@ -2570,11 +2570,12 @@ ath_startrecv(struct ath_softc *sc)
  * ath_init.
  */
 int
-ath_chan_set(struct ath_softc *sc, struct ieee80211_channel *chan)
+ath_chan_set(struct ath_softc *sc, struct ieee80211_node *ni)
 {
 	struct ath_hal *ah = sc->sc_ah;
 	struct ieee80211com *ic = &sc->sc_ic;
 	struct ifnet *ifp = &ic->ic_if;
+	struct ieee80211_channel *chan = ni->ni_chan;
 
 	DPRINTF(ATH_DEBUG_ANY, ("%s: %u (%u MHz) -> %u (%u MHz)\n", __func__,
 	    ieee80211_chan2ieee(ic, ic->ic_ibss_chan),
@@ -2631,7 +2632,7 @@ ath_chan_set(struct ath_softc *sc, struct ieee80211_ch
 		 * if we're switching; e.g. 11a to 11b/g.
 		 */
 		ic->ic_ibss_chan = chan;
-		mode = ieee80211_chan2mode(ic, chan);
+		mode = ieee80211_node_abg_mode(ic, ni);
 		if (mode != sc->sc_curmode)
 			ath_setcurmode(sc, mode);
 
@@ -2775,7 +2776,7 @@ ath_newstate(struct ieee80211com *ic, enum ieee80211_s
 		return (*sc->sc_newstate)(ic, nstate, arg);
 	}
 	ni = ic->ic_bss;
-	error = ath_chan_set(sc, ni->ni_chan);
+	error = ath_chan_set(sc, ni);
 	if (error != 0)
 		goto bad;
 	rfilt = ath_calcrxfilter(sc);
blob - 8f76831260110c7d26ba6f2cb28fc333f76efa83
blob + 382c993fa6d94475165208278cc0df39d933d428
--- sys/net80211/ieee80211.c
+++ sys/net80211/ieee80211.c
@@ -1174,55 +1174,6 @@ ieee80211_next_mode(struct ifnet *ifp)
 }
 
 /*
- * Return the phy mode for with the specified channel so the
- * caller can select a rate set.  This is problematic and the
- * work here assumes how things work elsewhere in this code.
- *
- * Because the result of this function is ultimately used to select a
- * rate from the rate set of the returned mode, it must return one of the
- * legacy 11a/b/g modes; 11n and 11ac modes use MCS instead of rate sets.
- */
-enum ieee80211_phymode
-ieee80211_chan2mode(struct ieee80211com *ic,
-    const struct ieee80211_channel *chan)
-{
-	/*
-	 * Are we fixed in 11a/b/g mode?
-	 * NB: this assumes the channel would not be supplied to us
-	 *     unless it was already compatible with the current mode.
-	 */
-	if (ic->ic_curmode == IEEE80211_MODE_11A ||
-	    ic->ic_curmode == IEEE80211_MODE_11B ||
-	    ic->ic_curmode == IEEE80211_MODE_11G)
-		return ic->ic_curmode;
-
-	/* If no channel was provided, return the most suitable legacy mode. */
-	if (chan == IEEE80211_CHAN_ANYC) {
-		switch (ic->ic_curmode) {
-		case IEEE80211_MODE_AUTO:
-		case IEEE80211_MODE_11N:
-			if (ic->ic_modecaps & (1 << IEEE80211_MODE_11A))
-				return IEEE80211_MODE_11A;
-			if (ic->ic_modecaps & (1 << IEEE80211_MODE_11G))
-				return IEEE80211_MODE_11G;
-			return IEEE80211_MODE_11B;
-		case IEEE80211_MODE_11AC:
-			return IEEE80211_MODE_11A;
-		default:
-			return ic->ic_curmode;
-		}
-	}
-
-	/* Deduce a legacy mode based on the channel characteristics. */
-	if (IEEE80211_IS_CHAN_5GHZ(chan))
-		return IEEE80211_MODE_11A;
-	else if (chan->ic_flags & (IEEE80211_CHAN_OFDM|IEEE80211_CHAN_DYN))
-		return IEEE80211_MODE_11G;
-	else
-		return IEEE80211_MODE_11B;
-}
-
-/*
  * Convert IEEE80211 MCS index to ifmedia subtype.
  */
 uint64_t
blob - 61ccf144a35f2ac870da130b2ae8a2f2881532b6
blob + 7400db3fd4e2ad3d8ceefff7869110f7aa007ea4
--- sys/net80211/ieee80211_input.c
+++ sys/net80211/ieee80211_input.c
@@ -2671,7 +2671,7 @@ ieee80211_recv_assoc_resp(struct ieee80211com *ic, str
 	else if (ni->ni_flags & IEEE80211_NODE_HT)
 		ieee80211_setmode(ic, IEEE80211_MODE_11N);
 	else
-		ieee80211_setmode(ic, ieee80211_chan2mode(ic, ni->ni_chan));
+		ieee80211_setmode(ic, ieee80211_node_abg_mode(ic, ni));
 	/*
 	 * Reset the erp state (mostly the slot time) now that
 	 * our operating mode has been nailed down.
blob - 2e02e1fff93930e65fc955599b4f7f1d85843259
blob + a9abfb9366f3bc80f5abaaabceb9480bd06d9a5f
--- sys/net80211/ieee80211_node.c
+++ sys/net80211/ieee80211_node.c
@@ -931,8 +931,28 @@ ieee80211_create_ibss(struct ieee80211com* ic, struct 
 		mode = IEEE80211_MODE_11AC;
 	else if (ic->ic_flags & IEEE80211_F_HTON)
 		mode = IEEE80211_MODE_11N;
-	else
-		mode = ieee80211_chan2mode(ic, ni->ni_chan);
+	else {
+		switch (IFM_MODE(ic->ic_media.ifm_cur->ifm_media)) {
+		case IFM_IEEE80211_11A:
+			mode = IEEE80211_MODE_11A;
+			break;
+		case IFM_IEEE80211_11G:
+			mode = IEEE80211_MODE_11G;
+			break;
+		case IFM_IEEE80211_11B:
+			mode = IEEE80211_MODE_11B;
+			break;
+		default:
+			if (IEEE80211_IS_CHAN_5GHZ(ni->ni_chan))
+				mode = IEEE80211_MODE_11A;
+			else if ((ni->ni_chan->ic_flags &
+			    (IEEE80211_CHAN_OFDM | IEEE80211_CHAN_DYN)) != 0)
+				mode = IEEE80211_MODE_11G;
+			else
+				mode = IEEE80211_MODE_11B;
+			break;
+		}
+	}
 	ieee80211_setmode(ic, mode);
 	/* Pick an appropriate mode for supported legacy rates. */
 	if (ic->ic_curmode == IEEE80211_MODE_11AC) {
@@ -1303,7 +1323,7 @@ ieee80211_node_join_bss(struct ieee80211com *ic, struc
 	uint32_t assoc_fail = 0;
 
 	/* Reinitialize media mode and channels if needed. */
-	mode = ieee80211_chan2mode(ic, selbs->ni_chan);
+	mode = ieee80211_node_abg_mode(ic, selbs);
 	if (mode != ic->ic_curmode)
 		ieee80211_setmode(ic, mode);
 
@@ -1317,8 +1337,6 @@ ieee80211_node_join_bss(struct ieee80211com *ic, struc
 	ni = ic->ic_bss;
 	ni->ni_assoc_fail |= assoc_fail;
 
-	ic->ic_curmode = ieee80211_chan2mode(ic, ni->ni_chan);
-
 	/* Make sure we send valid rates in an association request. */
 	if (ic->ic_opmode == IEEE80211_M_STA)
 		ieee80211_fix_rate(ic, ni,
@@ -1561,7 +1579,7 @@ ieee80211_end_scan(struct ifnet *ifp)
 				ieee80211_setmode(ic, IEEE80211_MODE_11N);
 			else
 				ieee80211_setmode(ic,
-				    ieee80211_chan2mode(ic, ni->ni_chan));
+				    ieee80211_node_abg_mode(ic, ni));
 			return;
 		}
 	
@@ -2641,6 +2659,40 @@ ieee80211_setup_rates(struct ieee80211com *ic, struct 
 	return ieee80211_fix_rate(ic, ni, flags);
 }
 
+/* 
+ * Return the 11a/b/g mode mutually supported for the given node.
+ * ni->ni_chan must be set before calling this, and ieee80211_setup_rates()
+ * should be called beforehand to properly differentiate 11b and 11g.
+ */
+enum ieee80211_phymode
+ieee80211_node_abg_mode(struct ieee80211com *ic, struct ieee80211_node *ni)
+{
+	/* Handle the case where our own phy mode was fixed by ifconfig. */
+	switch (IFM_MODE(ic->ic_media.ifm_cur->ifm_media)) {
+	case IFM_IEEE80211_11A:
+		return IEEE80211_MODE_11A; /* Peer uses 11a. */
+	case IFM_IEEE80211_11B:
+		return IEEE80211_MODE_11B; /* Peer uses 11b. */
+	case IFM_IEEE80211_11G:
+		/* Peer could be using either 11g or 11b, check below. */
+		break;
+	default:
+		break;
+	}
+
+	/* Our own phy mode is either 11G or AUTO. */
+
+	if (IEEE80211_IS_CHAN_5GHZ(ni->ni_chan))
+		return IEEE80211_MODE_11A;
+
+	if ((ni->ni_flags & IEEE80211_NODE_ERP) &&
+	    (ni->ni_chan->ic_flags &
+	    (IEEE80211_CHAN_OFDM | IEEE80211_CHAN_DYN)) != 0)
+		return IEEE80211_MODE_11G;
+
+	return IEEE80211_MODE_11B;
+}
+
 void
 ieee80211_node_trigger_addba_req(struct ieee80211_node *ni, int tid)
 {
blob - 1544370e1d93dce970deeb793df29ecb0b58e260
blob + baebec02ff4252023680b115fc78bcdfa0ecb16b
--- sys/net80211/ieee80211_node.h
+++ sys/net80211/ieee80211_node.h
@@ -644,6 +644,8 @@ int ieee80211_setup_vhtop(struct ieee80211_node *, con
     uint8_t, int);
 int ieee80211_setup_rates(struct ieee80211com *,
 	    struct ieee80211_node *, const u_int8_t *, const u_int8_t *, int);
+enum ieee80211_phymode ieee80211_node_abg_mode(struct ieee80211com *,
+	    struct ieee80211_node *);
 void ieee80211_node_trigger_addba_req(struct ieee80211_node *, int);
 void ieee80211_count_longslotsta(void *, struct ieee80211_node *);
 void ieee80211_count_nonerpsta(void *, struct ieee80211_node *);
blob - c628a8b0d29698382ab9bd3fc38b8fdb4bc5f980
blob + 12665c601268bb12329555728e85f170cd1330bd
--- sys/net80211/ieee80211_output.c
+++ sys/net80211/ieee80211_output.c
@@ -1258,7 +1258,7 @@ struct mbuf *
 ieee80211_get_probe_req(struct ieee80211com *ic, struct ieee80211_node *ni)
 {
 	const struct ieee80211_rateset *rs =
-	    &ic->ic_sup_rates[ieee80211_chan2mode(ic, ni->ni_chan)];
+	    &ic->ic_sup_rates[ieee80211_node_abg_mode(ic, ni)];
 	struct mbuf *m;
 	u_int8_t *frm;
 
blob - 3d8d649c8f0ff5e79c98fc9b6ec95d0c4d68b7f2
blob + 2463eec03cbbb297d18a40f1016e9c8800859f7f
--- sys/net80211/ieee80211_proto.c
+++ sys/net80211/ieee80211_proto.c
@@ -221,7 +221,7 @@ ieee80211_fix_rate(struct ieee80211com *ic, struct iee
 
 	error = 0;
 	okrate = badrate = fixedrate = 0;
-	srs = &ic->ic_sup_rates[ieee80211_chan2mode(ic, ni->ni_chan)];
+	srs = &ic->ic_sup_rates[ieee80211_node_abg_mode(ic, ni)];
 	nrs = &ni->ni_rates;
 	for (i = 0; i < nrs->rs_nrates; ) {
 		ignore = 0;
@@ -1153,8 +1153,8 @@ justcleanup:
 		/* initialize bss for probe request */
 		IEEE80211_ADDR_COPY(ni->ni_macaddr, etherbroadcastaddr);
 		IEEE80211_ADDR_COPY(ni->ni_bssid, etherbroadcastaddr);
-		ni->ni_rates = ic->ic_sup_rates[
-			ieee80211_chan2mode(ic, ni->ni_chan)];
+		ni->ni_rates = ic->ic_sup_rates[ieee80211_node_abg_mode(ic,
+		    ni)];
 		ni->ni_associd = 0;
 		ni->ni_rstamp = 0;
 		ni->ni_rsn_supp_state = RSNA_SUPP_INITIALIZE;
blob - 2a427b581ccdf4a9eec573107bec2aefe4c4e06b
blob + 236c37a8a241ba6228205f3902c53c0f63dea330
--- sys/net80211/ieee80211_var.h
+++ sys/net80211/ieee80211_var.h
@@ -496,8 +496,6 @@ int	ieee80211_min_basic_rate(struct ieee80211com *);
 int	ieee80211_max_basic_rate(struct ieee80211com *);
 int	ieee80211_setmode(struct ieee80211com *, enum ieee80211_phymode);
 enum ieee80211_phymode ieee80211_next_mode(struct ifnet *);
-enum ieee80211_phymode ieee80211_chan2mode(struct ieee80211com *,
-		const struct ieee80211_channel *);
 void	ieee80211_disable_wep(struct ieee80211com *); 
 void	ieee80211_disable_rsn(struct ieee80211com *); 
 int	ieee80211_add_ess(struct ieee80211com *, struct ieee80211_join *);