git: aaaca5f288fa - main - rtwn: add a default OFDM / CCK rate for self-generated frames

From: Adrian Chadd <adrian_at_FreeBSD.org>
Date: Thu, 19 Dec 2024 16:08:36 UTC
The branch main has been updated by adrian:

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

commit aaaca5f288fa257a745743d03e51eef0517b246f
Author:     Adrian Chadd <adrian@FreeBSD.org>
AuthorDate: 2024-12-10 19:03:34 +0000
Commit:     Adrian Chadd <adrian@FreeBSD.org>
CommitDate: 2024-12-19 16:06:29 +0000

    rtwn: add a default OFDM / CCK rate for self-generated frames
    
    I noticed during testing that the MAC was generating MCS7 ACKs and
    MCS7 block-ACK frames in response to MCS frames from its peer.
    This is very suboptimal - it means that unless you're very close
    to your peer (in this case a 2GHz AP), you'll end up failing a lot
    of ACKs.
    
    Linux faced the opposite problem in rtl8xxxu - the rate set being
    programmed in here included a lot MORE rates, including MCS 0->7
    and OFDM 6M->54M.  This meant that they were INTENTIONALLY telling
    the hardware to transmit at higher rates, and their fix was to
    mask out the higher rates so self-generated frames don't try the
    high rates at all.
    
    Now, I am not sure why I'm not seeing any OFDM or HT basic rates.
    We don't mark any OFDM / HT rates as basic in net80211 (in
    ieee80211_phy.c) so I'm going to need to go and do a review of the
    standard to see what's up.  Additionally, the HT rate set that we
    populate isn't tagging any of the HT rates as IEEE80211_RATE_BASIC,
    so the code I added for now is a no-op.
    
    So:
    
    * Extend rtwn_get_rates() and its consumers to populate the HT rateset
      with basic rates if they're provided
    * Add a default 2GHz / 5GHz mask, inspired by linux, applied over the
      basic rates provided.
    * Make sure there's at least an OFDM rate (for 2G/5G) rate available if
      the peer node is HT, which avoids the MAC defaulting to MCS7 when
      generating ACK/block-ACK.
    * Add register definitions for INIDATA/INIRTS, which set the default
      data rate when the driver doesn't specify the initial data / RTS/CTS
      rates in the TX descriptor.
    * Leave a comment about why I've modified the mask from Linux.
    
    Locally tested:
    
    * RTL8192CU, STA mode
    * RTL8188EU, STA mode
    * RTL8192EU, STA mode
    * RTL8812AU, STA mode
    
    Differential Revision:  https://reviews.freebsd.org/D48019
    Reviewed by:    bz
---
 sys/dev/rtwn/if_rtwn.c           | 38 ++++++++++++++++++++++++++++++++++----
 sys/dev/rtwn/if_rtwn_rx.c        | 18 +++++++++++++-----
 sys/dev/rtwn/rtl8192c/r92c_reg.h | 26 ++++++++++++++++++++++++++
 3 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/sys/dev/rtwn/if_rtwn.c b/sys/dev/rtwn/if_rtwn.c
index be01ececf307..3b286d9adba9 100644
--- a/sys/dev/rtwn/if_rtwn.c
+++ b/sys/dev/rtwn/if_rtwn.c
@@ -1202,7 +1202,8 @@ rtwn_calc_basicrates(struct rtwn_softc *sc)
 		struct rtwn_vap *rvp;
 		struct ieee80211vap *vap;
 		struct ieee80211_node *ni;
-		uint32_t rates;
+		struct ieee80211_htrateset *rs_ht;
+		uint32_t rates = 0, htrates = 0;
 
 		rvp = sc->vaps[i];
 		if (rvp == NULL || rvp->curr_mode == R92C_MSR_NOLINK)
@@ -1213,16 +1214,45 @@ rtwn_calc_basicrates(struct rtwn_softc *sc)
 			continue;
 
 		ni = ieee80211_ref_node(vap->iv_bss);
-		/* Only fetches basic rates; no need to add HT/VHT here */
-		rtwn_get_rates(sc, &ni->ni_rates, NULL, &rates, NULL, NULL, 1);
+		if (ni->ni_flags & IEEE80211_NODE_HT)
+			rs_ht = &ni->ni_htrates;
+		else
+			rs_ht = NULL;
+		/*
+		 * Only fetches basic rates; fetch 802.11abg and 11n basic
+		 * rates
+		 */
+		rtwn_get_rates(sc, &ni->ni_rates, rs_ht, &rates, &htrates,
+		    NULL, 1);
+
+		/*
+		 * We need at least /an/ OFDM and/or MCS rate for HT
+		 * operation, or the MAC will generate MCS7 ACK/Block-ACK
+		 * frames and thus performance will suffer.
+		 */
+		if (ni->ni_flags & IEEE80211_NODE_HT) {
+			htrates |= 0x01; /* MCS0 */
+			rates |= (1 << RTWN_RIDX_OFDM6);
+		}
+
 		basicrates |= rates;
+		basicrates |= (htrates << RTWN_RIDX_HT_MCS_SHIFT);
+
+		/* Filter out undesired high rates */
+		if (ni->ni_chan != IEEE80211_CHAN_ANYC &&
+		    IEEE80211_IS_CHAN_5GHZ(ni->ni_chan))
+			basicrates &= R92C_RRSR_RATE_MASK_5GHZ;
+		else
+			basicrates &= R92C_RRSR_RATE_MASK_2GHZ;
+
 		ieee80211_free_node(ni);
 	}
 
+
 	if (basicrates == 0)
 		return;
 
-	/* XXX initial RTS rate? */
+	/* XXX also set initial RTS rate? */
 	rtwn_set_basicrates(sc, basicrates);
 }
 
diff --git a/sys/dev/rtwn/if_rtwn_rx.c b/sys/dev/rtwn/if_rtwn_rx.c
index 977c1d17a08a..b1465dd80ee7 100644
--- a/sys/dev/rtwn/if_rtwn_rx.c
+++ b/sys/dev/rtwn/if_rtwn_rx.c
@@ -62,7 +62,7 @@
  * maxrate_p is set to the ridx value.
  *
  * If basic_rates is 1 then only the 11abg basic rate logic will
- * be applied; HT/VHT will be ignored.
+ * be applied; the HT rateset will be applied to 11n rates.
  */
 void
 rtwn_get_rates(struct rtwn_softc *sc, const struct ieee80211_rateset *rs,
@@ -92,12 +92,19 @@ rtwn_get_rates(struct rtwn_softc *sc, const struct ieee80211_rateset *rs,
 	}
 
 	/* If we're doing 11n, enable 11n rates */
-	if (rs_ht != NULL && !basic_rates) {
+	if (rs_ht != NULL) {
 		for (i = 0; i < rs_ht->rs_nrates; i++) {
+			uint8_t rate = rs_ht->rs_rates[i] & 0x7f;
+			bool is_basic = rs_ht->rs_rates[i] &
+			    IEEE80211_RATE_BASIC;
 			/* Only do up to 2-stream rates for now */
-			if ((rs_ht->rs_rates[i] & 0x7f) > 0xf)
+			if ((rate) > 0xf)
 				continue;
-			ridx = rs_ht->rs_rates[i] & 0xf;
+
+			if (basic_rates && is_basic == false)
+				continue;
+
+			ridx = rate & 0xf;
 			htrates |= (1 << ridx);
 
 			/* Guard against the rate table being oddly ordered */
@@ -107,7 +114,8 @@ rtwn_get_rates(struct rtwn_softc *sc, const struct ieee80211_rateset *rs,
 	}
 
 	RTWN_DPRINTF(sc, RTWN_DEBUG_RA,
-	    "%s: rates 0x%08X, maxrate %d\n", __func__, rates, maxrate);
+	    "%s: rates 0x%08X htrates 0x%08X, maxrate %d\n",
+	    __func__, rates, htrates, maxrate);
 
 	if (rates_p != NULL)
 		*rates_p = rates;
diff --git a/sys/dev/rtwn/rtl8192c/r92c_reg.h b/sys/dev/rtwn/rtl8192c/r92c_reg.h
index 6ca4a4eca031..e6d232a88834 100644
--- a/sys/dev/rtwn/rtl8192c/r92c_reg.h
+++ b/sys/dev/rtwn/rtl8192c/r92c_reg.h
@@ -519,6 +519,23 @@
 #define R92C_RRSR_RATE_BITMAP_M		0x000fffff
 #define R92C_RRSR_RATE_BITMAP_S		0
 #define R92C_RRSR_RATE_CCK_ONLY_1M	0xffff1
+/* Suitable low-rate defaults for 2/5GHz CTS/ACK/Block-ACK */
+/*
+ * Note: the RTL8192CU vendor driver disables 2M CCK as a
+ * basic rate due to "Low TXEVM" causing issues with other
+ * vendor devices.  Since we want to maximise basic rate
+ * reliability to prevent retries (due to missing RTS/CTS
+ * and ACK/Block-ACK), do the same here.
+ *
+ * And, unfortunately, enabling MCS rates for self-generated
+ * and management/control frames can result in the peer AP
+ * just plainly ignoring you.  This happened with older
+ * D-Link 802.11n era APs.  The masks will exclude MCS management
+ * rates, it's easy to add it to the mask in rtwn_set_basicrates().
+ * (Just |= 0x100, bit 12 == MCS 0.)
+ */
+#define R92C_RRSR_RATE_MASK_2GHZ	0x015d
+#define R92C_RRSR_RATE_MASK_5GHZ	0x0150
 #define R92C_RRSR_RATE_ALL		0xfffff
 #define R92C_RRSR_RSC_SUBCHNL_MASK	0x00600000
 #define R92C_RRSR_RSC_LOWSUBCHNL	0x00200000
@@ -535,6 +552,15 @@
 #define R92C_EDCA_PARAM_TXOP_M		0xffff0000
 #define R92C_EDCA_PARAM_TXOP_S		16
 
+/* Bits for R92C_INIRTS_RATE_SEL. */
+#define R92C_INIRTS_RATE_SEL_RATE_M	0x3f
+#define R92C_INIRTS_RATE_SEL_RATE_S	0
+
+/* Bits for R92C_INIDATA_RATE_SEL. */
+#define R92C_INIDATA_RATE_SEL_RATE_M	0x3f
+#define R92C_INIDATA_RATE_SEL_RATE_S	0
+#define R92C_INIDATA_RATE_SEL_SHORTGI	0x40
+
 /* Bits for R92C_HWSEQ_CTRL / R92C_TXPAUSE. */
 #define R92C_TX_QUEUE_VO		0x01
 #define R92C_TX_QUEUE_VI		0x02