11n: don't require a BlockAck timeout

[Stefan Sperling]

We currently force BlockAck agreements to time out after a certain
period of inactivity. Some APs, notably Broadcom-based ones (like
Apple Airport) don't cope with this nicely and drop packets while
the BA session is reestablished. The result is unusable wifi in
11n mode with such APs (> 50% packet loss).

In fact, these APs request a zero timeout in their ADDBA frames.
802.11 2012 section "8.4.1.15 Block Ack Timeout Value field" says
"A value of 0 disables the timeout."
So I'm not sure why our code requires a minimum timeout.
The original timeout values added by damien@ were very short (order
of miliseconds) and I had to extend them to make BlockAck work with
any AP at all.

This patch makes us honour the timeout only if the AP has requested it.
Linux treats this timeout value in the same way.

Doesn't seem to break any APs which were already working well.

ok?

Index: ieee80211_input.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.154
diff -u -p -r1.154 ieee80211_input.c
--- ieee80211_input.c 25 Jan 2016 15:14:22 -0000 1.154
+++ ieee80211_input.c 31 Jan 2016 16:19:56 -0000
@@ -704,7 +704,8 @@ ieee80211_input_ba(struct ieee80211com *
sn = letoh16(*(u_int16_t *)wh->i_seq) >> IEEE80211_SEQ_SEQ_SHIFT;

/* reset Block Ack inactivity timer */
- timeout_add_usec(&ba->ba_to, ba->ba_timeout_val);
+ if (ba->ba_timeout_val != 0)
+ timeout_add_usec(&ba->ba_to, ba->ba_timeout_val);

if (SEQ_LT(sn, ba->ba_winstart)) { /* SN < WinStartB */
ifp->if_ierrors++;
@@ -2457,7 +2458,8 @@ ieee80211_recv_addba_req(struct ieee8021
if (ba->ba_state == IEEE80211_BA_AGREED) {
/* XXX should we update the timeout value? */
/* reset Block Ack inactivity timer */
- timeout_add_usec(&ba->ba_to, ba->ba_timeout_val);
+ if (ba->ba_timeout_val != 0)
+ timeout_add_usec(&ba->ba_to, ba->ba_timeout_val);

/* check if it's a Protected Block Ack agreement */
if (!(ni->ni_flags & IEEE80211_NODE_MFP) ||
@@ -2494,10 +2496,6 @@ ieee80211_recv_addba_req(struct ieee8021
/* setup Block Ack agreement */
ba->ba_state = IEEE80211_BA_INIT;
ba->ba_timeout_val = timeout * IEEE80211_DUR_TU;
- if (ba->ba_timeout_val < IEEE80211_BA_MIN_TIMEOUT)
- ba->ba_timeout_val = IEEE80211_BA_MIN_TIMEOUT;
- else if (ba->ba_timeout_val > IEEE80211_BA_MAX_TIMEOUT)
- ba->ba_timeout_val = IEEE80211_BA_MAX_TIMEOUT;
ba->ba_ni = ni;
timeout_set(&ba->ba_to, ieee80211_rx_ba_timeout, ba);
timeout_set(&ba->ba_gap_to, ieee80211_input_ba_gap_timeout, ba);
@@ -2526,7 +2524,8 @@ ieee80211_recv_addba_req(struct ieee8021
}
ba->ba_state = IEEE80211_BA_AGREED;
/* start Block Ack inactivity timer */
- timeout_add_usec(&ba->ba_to, ba->ba_timeout_val);
+ if (ba->ba_timeout_val != 0)
+ timeout_add_usec(&ba->ba_to, ba->ba_timeout_val);
status = IEEE80211_STATUS_SUCCESS;
resp:
/* MLME-ADDBA.response */
@@ -2988,7 +2987,8 @@ ieee80211_bar_tid(struct ieee80211com *i
return; /* PBAC, do not move window */
}
/* reset Block Ack inactivity timer */
- timeout_add_usec(&ba->ba_to, ba->ba_timeout_val);
+ if (ba->ba_timeout_val != 0)
+ timeout_add_usec(&ba->ba_to, ba->ba_timeout_val);

if (SEQ_LT(ba->ba_winstart, ssn))
ieee80211_ba_move_window(ic, ni, tid, ssn);
Index: ieee80211_node.h
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_node.h,v
retrieving revision 1.53
diff -u -p -r1.53 ieee80211_node.h
--- ieee80211_node.h 25 Jan 2016 15:10:37 -0000 1.53
+++ ieee80211_node.h 31 Jan 2016 16:05:12 -0000
@@ -112,9 +112,6 @@ struct ieee80211_tx_ba {
struct ieee80211_node *ba_ni; /* backpointer for callbacks */
struct timeout ba_to;
int ba_timeout_val;
-#define IEEE80211_BA_MIN_TIMEOUT (1000 * 1000) /* 1 sec */
-#define IEEE80211_BA_MAX_TIMEOUT (5000 * 1000) /* 5 sec */
-
int ba_state;
#define IEEE80211_BA_INIT 0
#define IEEE80211_BA_REQUESTED 1
Index: ieee80211_proto.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_proto.c,v
retrieving revision 1.60
diff -u -p -r1.60 ieee80211_proto.c
--- ieee80211_proto.c 12 Jan 2016 09:28:09 -0000 1.60
+++ ieee80211_proto.c 31 Jan 2016 16:18:48 -0000
@@ -641,7 +641,7 @@ ieee80211_addba_request(struct ieee80211
/* setup Block Ack */
ba->ba_state = IEEE80211_BA_REQUESTED;
ba->ba_token = ic->ic_dialog_token++;
- ba->ba_timeout_val = IEEE80211_BA_MAX_TIMEOUT;
+ ba->ba_timeout_val = 0;
timeout_set(&ba->ba_to, ieee80211_tx_ba_timeout, ba);
ba->ba_winsize = IEEE80211_BA_MAX_WINSZ;
ba->ba_winstart = ssn;

[Stuart Henderson]

Post by Stefan Sperling
We currently force BlockAck agreements to time out after a certain
period of inactivity. Some APs, notably Broadcom-based ones (like
Apple Airport) don't cope with this nicely and drop packets while
the BA session is reestablished. The result is unusable wifi in
11n mode with such APs (> 50% packet loss).
In fact, these APs request a zero timeout in their ADDBA frames.
802.11 2012 section "8.4.1.15 Block Ack Timeout Value field" says
"A value of 0 disables the timeout."
So I'm not sure why our code requires a minimum timeout.
of miliseconds) and I had to extend them to make BlockAck work with
any AP at all.
This patch makes us honour the timeout only if the AP has requested it.
Linux treats this timeout value in the same way.
Doesn't seem to break any APs which were already working well.

Could this capping be an attempt to mitigate DoS from people playing
games with ADDBAs? Anyway I think avoiding the regression with these
APs is pretty important and the diff makes sense, so OK with me.

[Stefan Sperling]

Post by Stuart Henderson
Could this capping be an attempt to mitigate DoS from people playing
games with ADDBAs?

That kind of attack doesn't make much sense.
A lot of stuff in wifi depends on others being cooperative.
Far more disruption can already be caused in other ways.

There's only a small fixed number of BA agreements per node structure,
i.e. per MAC address. So using up "all agreements" only affects one peer.
And normal Acks are always an option.