net80211 hostap HT protection fixes

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

net80211 hostap HT protection fixes

Stefan Sperling-5
Our setting up of HT protection in hostap mode has two problems:

Some 11n STAs are misclassified as 11b/g. This happens if a bogus
HTCAP IE is sent in probe requests which advertise no supported MCS.
I have photographed such a probe request in the wild:
https://media.bsd.network/media_attachments/files/000/933/963/original/4e0109a0b8c08499.png
So checking just the advertised Rx MCS is insufficient.
We need to take the mere presence of the HTCAP IE at face value.

The second problem is that we currently mandate RTS for HT protection,
while CTS-to-self is preferrable and recommended over RTS in wifi
literature I have sitting on my book shelf.
So switch our 11n hostap to using CTS-to-self instead of RTS for HT
protection (which becomes active when non-11n devices are around).
I just made a related commit to iwm(4) to make sure this gets picked up,
so when testing this diff with an iwm(4) client, please update to -current.

At the top of this diff there's a small client-side fix to pick up the
AP's ERP protection setting in 11n mode.
Otherwise this diff affects 11n access points only, i.e. athn(4).
It makes my athn(4) AP run circles (up to 15 Mbps) on a 2 GHz channel
with overlapping networks.

Any more testers?

diff 3c480dbb36b2cb6b6571151522ce1db2410d346c /usr/src
blob - d04fadc01879cbb7e60af3784012744a306db9fc
file + sys/net80211/ieee80211_input.c
--- sys/net80211/ieee80211_input.c
+++ sys/net80211/ieee80211_input.c
@@ -1565,7 +1565,9 @@ ieee80211_recv_probe_resp(struct ieee80211com *ic, str
  DPRINTF(("[%s] erp change: was 0x%x, now 0x%x\n",
     ether_sprintf((u_int8_t *)wh->i_addr2),
     ni->ni_erp, erp));
- if (ic->ic_curmode == IEEE80211_MODE_11G &&
+ if ((ic->ic_curmode == IEEE80211_MODE_11G ||
+    (ic->ic_curmode == IEEE80211_MODE_11N &&
+    IEEE80211_IS_CHAN_2GHZ(ni->ni_chan))) &&
     (erp & IEEE80211_ERP_USE_PROTECTION))
  ic->ic_flags |= IEEE80211_F_USEPROT;
  else
blob - d6794f3c98116536cf4ffa8eaae3b9da84315b3b
file + sys/net80211/ieee80211_node.c
--- sys/net80211/ieee80211_node.c
+++ sys/net80211/ieee80211_node.c
@@ -894,11 +894,15 @@ ieee80211_create_ibss(struct ieee80211com* ic, struct
  int aci;
 
  /*
- * Default to non-member HT protection. This will be updated
- * later based on the number of non-HT nodes in the node cache.
+ * Configure HT protection. This will be updated later
+ * based on the number of non-HT nodes in the node cache.
  */
- ni->ni_htop1 = IEEE80211_HTPROT_NONMEMBER;
- ic->ic_protmode = IEEE80211_PROT_RTSCTS;
+ ic->ic_protmode = IEEE80211_PROT_NONE;
+ ni->ni_htop1 = IEEE80211_HTPROT_NONE;
+ /* Disallow Greenfield mode. None of our drivers support it. */
+ ni->ni_htop1 |= IEEE80211_HTOP1_NONGF_STA;
+ if (ic->ic_update_htprot)
+ ic->ic_update_htprot(ic, ni);
 
  /* Configure QoS EDCA parameters. */
  for (aci = 0; aci < EDCA_NUM_AC; aci++) {
@@ -1985,7 +1989,14 @@ ieee80211_clean_nodes(struct ieee80211com *ic, int cac
 #ifndef IEEE80211_STA_ONLY
  nnodes++;
  if ((ic->ic_flags & IEEE80211_F_HTON) && cache_timeout) {
- if (!ieee80211_node_supports_ht(ni)) {
+ /*
+ * Check if node supports 802.11n.
+ * Only require HT capabilities IE for this check.
+ * Nodes might never reveal their supported MCS to us
+ * unless they go through a full association sequence.
+ * ieee80211_node_supports_ht() could misclassify them.
+ */
+ if ((ni->ni_flags & IEEE80211_NODE_HTCAP) == 0) {
  nonht++;
  if (ni->ni_state == IEEE80211_STA_ASSOC)
  nonhtassoc++;
@@ -2031,7 +2042,7 @@ ieee80211_clean_nodes(struct ieee80211com *ic, int cac
 #ifndef IEEE80211_STA_ONLY
  nnodes--;
  if ((ic->ic_flags & IEEE80211_F_HTON) && cache_timeout) {
- if (!ieee80211_node_supports_ht(ni)) {
+ if ((ni->ni_flags & IEEE80211_NODE_HTCAP) == 0) {
  nonht--;
  if (ni->ni_state == IEEE80211_STA_ASSOC)
  nonhtassoc--;
@@ -2052,16 +2063,20 @@ ieee80211_clean_nodes(struct ieee80211com *ic, int cac
 
 #ifndef IEEE80211_STA_ONLY
  if ((ic->ic_flags & IEEE80211_F_HTON) && cache_timeout) {
+ uint16_t htop1 = ic->ic_bss->ni_htop1;
+
  /* Update HT protection settings. */
  if (nonht) {
- protmode = IEEE80211_PROT_RTSCTS;
+ protmode = IEEE80211_PROT_CTSONLY;
  if (nonhtassoc)
  htprot = IEEE80211_HTPROT_NONHT_MIXED;
  else
  htprot = IEEE80211_HTPROT_NONMEMBER;
  }
- if (ic->ic_bss->ni_htop1 != htprot) {
- ic->ic_bss->ni_htop1 = htprot;
+ if ((htop1 & IEEE80211_HTOP1_PROT_MASK) != htprot) {
+ htop1 &= ~IEEE80211_HTOP1_PROT_MASK;
+ htop1 |= htprot;
+ ic->ic_bss->ni_htop1 |= htop1;
  ic->ic_protmode = protmode;
  if (ic->ic_update_htprot)
  ic->ic_update_htprot(ic, ic->ic_bss);
@@ -2128,6 +2143,8 @@ ieee80211_setup_htcaps(struct ieee80211_node *ni, cons
  ni->ni_txbfcaps = (data[21] | (data[22] << 8) | (data[23] << 16) |
  (data[24] << 24));
  ni->ni_aselcaps = data[25];
+
+ ni->ni_flags |= IEEE80211_NODE_HTCAP;
 }
 
 #ifndef IEEE80211_STA_ONLY
@@ -2147,7 +2164,7 @@ ieee80211_clear_htcaps(struct ieee80211_node *ni)
  ni->ni_aselcaps = 0;
 
  ni->ni_flags &= ~(IEEE80211_NODE_HT | IEEE80211_NODE_HT_SGI20 |
-    IEEE80211_NODE_HT_SGI40);
+    IEEE80211_NODE_HT_SGI40 | IEEE80211_NODE_HTCAP);
 
 }
 #endif
@@ -2262,7 +2279,10 @@ ieee80211_node_join_ht(struct ieee80211com *ic, struct
 
  /* Update HT protection setting. */
  if ((ni->ni_flags & IEEE80211_NODE_HT) == 0) {
- ic->ic_bss->ni_htop1 = IEEE80211_HTPROT_NONHT_MIXED;
+ uint16_t htop1 = ic->ic_bss->ni_htop1;
+ htop1 &= ~IEEE80211_HTOP1_PROT_MASK;
+ htop1 |= IEEE80211_HTPROT_NONHT_MIXED;
+ ic->ic_bss->ni_htop1 = htop1;
  if (ic->ic_update_htprot)
  ic->ic_update_htprot(ic, ic->ic_bss);
  }
blob - a012f6cb7458719ca26325b951a3ebcdd9401c40
file + sys/net80211/ieee80211_node.h
--- sys/net80211/ieee80211_node.h
+++ sys/net80211/ieee80211_node.h
@@ -368,6 +368,7 @@ struct ieee80211_node {
 #define IEEE80211_NODE_HT_SGI20 0x4000 /* SGI on 20 MHz negotiated */
 #define IEEE80211_NODE_HT_SGI40 0x8000 /* SGI on 40 MHz negotiated */
 #define IEEE80211_NODE_VHT 0x10000 /* VHT negotiated */
+#define IEEE80211_NODE_HTCAP 0x20000 /* claims to support HT */
 
  /* If not NULL, this function gets called when ni_refcnt hits zero. */
  void (*ni_unref_cb)(struct ieee80211com *,
@@ -427,13 +428,14 @@ ieee80211_unref_node(struct ieee80211_node **ni)
 
 /*
  * Check if the peer supports HT.
- * Require at least one of the mandatory MCS.
+ * Require a HT capabilities IE and at least one of the mandatory MCS.
  * MCS 0-7 are mandatory but some APs have particular MCS disabled.
  */
 static inline int
 ieee80211_node_supports_ht(struct ieee80211_node *ni)
 {
- return (ni->ni_rxmcs[0] & 0xff);
+ return ((ni->ni_flags & IEEE80211_NODE_HTCAP) &&
+    ni->ni_rxmcs[0] & 0xff);
 }
 
 /* Check if the peer supports HT short guard interval (SGI) on 20 MHz. */
blob - a18fa24052439fb4796cc896ec69bc163bbdc646
file + sys/net80211/ieee80211_var.h
--- sys/net80211/ieee80211_var.h
+++ sys/net80211/ieee80211_var.h
@@ -255,7 +255,7 @@ struct ieee80211com {
  enum ieee80211_state ic_state; /* 802.11 state */
  u_int32_t *ic_aid_bitmap;
  u_int16_t ic_max_aid;
- enum ieee80211_protmode ic_protmode; /* 802.11g protection mode */
+ enum ieee80211_protmode ic_protmode; /* 802.11g/n protection mode */
  struct ifmedia ic_media; /* interface media config */
  caddr_t ic_rawbpf; /* packet filter structure */
  struct ieee80211_node *ic_bss; /* information for this node */


Reply | Threaded
Open this post in threaded view
|

Re: net80211 hostap HT protection fixes

Lauri Tirkkonen-2
On Wed, Feb 27 2019 15:06:49 +0100, Stefan Sperling wrote:
> Otherwise this diff affects 11n access points only, i.e. athn(4).
> It makes my athn(4) AP run circles (up to 15 Mbps) on a 2 GHz channel
> with overlapping networks.
>
> Any more testers?

Based on cursory testing, seems to work fine on my athn(4) 11n 5GHz AP.
But I didn't observe any improvements in bandwidth, using an Android
phone station as traffic sink (~9MBit/s maximum both before and after).

--
Lauri Tirkkonen | lotheac @ IRCnet

Reply | Threaded
Open this post in threaded view
|

Re: net80211 hostap HT protection fixes

Stefan Sperling-5
On Fri, Mar 01, 2019 at 02:25:26PM +0200, Lauri Tirkkonen wrote:

> On Wed, Feb 27 2019 15:06:49 +0100, Stefan Sperling wrote:
> > Otherwise this diff affects 11n access points only, i.e. athn(4).
> > It makes my athn(4) AP run circles (up to 15 Mbps) on a 2 GHz channel
> > with overlapping networks.
> >
> > Any more testers?
>
> Based on cursory testing, seems to work fine on my athn(4) 11n 5GHz AP.
> But I didn't observe any improvements in bandwidth, using an Android
> phone station as traffic sink (~9MBit/s maximum both before and after).
>

That sounds OK. The actual benefits will depend on the environment.
I am seeing around 12Mbit/s on average on 5GHz, and about 9Mbit/s on 2.
The main benefit for me is on 2 GHz, where I typically saw less than
500kbit/s and hence never used it. What's important is that we now have
a baseline for further improvements (such as Tx aggregration and 40 MHz
channels) which isn't ridiculously low.

Thanks for testing! Meanwhile, the diff has been committed and should be
in the next snapshot. I am still interested in additional test reports.