Re: fix ADDBA params in ADDBA requests and responses

2016-02-04 Thread Stefan Sperling
On Thu, Feb 04, 2016 at 09:42:03AM +0100, Stefan Sperling wrote:
> This fixes up parameters in ADDBA requests and responses.
> 
> Again, the current code is using the wrong bitmask to set the ack
> policy bit. IEEE80211_BA_ACK_POLICY is for QoS BlockAck request
> and response frames, not ADDBA request and response frames.

Slightly nicer diff below, initializing ba_params on TX in a
better place (we don't use the TX part yet, since we don't send
A-MPDU yet but only receive them).

> It's important to echo back the set of parameters we got from the AP.
> This part is necessary to make the iwn/iwm apple fix I sent yesterday
> actually work.

Actually, it turns out this change makes airport APs work reliably
independently of any driver changes.

Index: ieee80211_input.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.156
diff -u -p -r1.156 ieee80211_input.c
--- ieee80211_input.c   4 Feb 2016 16:23:40 -   1.156
+++ ieee80211_input.c   4 Feb 2016 16:52:24 -
@@ -2499,6 +2499,7 @@ ieee80211_recv_addba_req(struct ieee8021
else if (ba->ba_timeout_val > IEEE80211_BA_MAX_TIMEOUT)
ba->ba_timeout_val = IEEE80211_BA_MAX_TIMEOUT;
ba->ba_ni = ni;
+   ba->ba_params = params;
timeout_set(&ba->ba_to, ieee80211_rx_ba_timeout, ba);
timeout_set(&ba->ba_gap_to, ieee80211_input_ba_gap_timeout, ba);
ba->ba_winsize = bufsz;
Index: ieee80211_node.h
===
RCS file: /cvs/src/sys/net80211/ieee80211_node.h,v
retrieving revision 1.55
diff -u -p -r1.55 ieee80211_node.h
--- ieee80211_node.h4 Feb 2016 16:23:40 -   1.55
+++ ieee80211_node.h4 Feb 2016 16:49:33 -
@@ -120,6 +120,9 @@ struct ieee80211_tx_ba {
 #define IEEE80211_BA_REQUESTED 1
 #define IEEE80211_BA_AGREED2
 
+   /* ADDBA parameter set field for this BA agreement. */
+   u_int16_t   ba_params;
+
/* These values are IEEE802.11 frame sequence numbers (0x0-0xfff) */
u_int16_t   ba_winstart;
u_int16_t   ba_winend;
@@ -140,6 +143,7 @@ struct ieee80211_rx_ba {
struct timeout  ba_to;
int ba_timeout_val;
int ba_state;
+   u_int16_t   ba_params;
u_int16_t   ba_winstart;
u_int16_t   ba_winend;
u_int16_t   ba_winsize;
Index: ieee80211_output.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_output.c,v
retrieving revision 1.108
diff -u -p -r1.108 ieee80211_output.c
--- ieee80211_output.c  21 Jan 2016 20:33:20 -  1.108
+++ ieee80211_output.c  4 Feb 2016 16:32:14 -
@@ -1414,7 +1414,6 @@ ieee80211_get_addba_req(struct ieee80211
struct ieee80211_tx_ba *ba = &ni->ni_tx_ba[tid];
struct mbuf *m;
u_int8_t *frm;
-   u_int16_t params;
 
m = ieee80211_getmgmt(M_DONTWAIT, MT_DATA, 9);
if (m == NULL)
@@ -1424,12 +1423,7 @@ ieee80211_get_addba_req(struct ieee80211
*frm++ = IEEE80211_CATEG_BA;
*frm++ = IEEE80211_ACTION_ADDBA_REQ;
*frm++ = ba->ba_token;
-   params = ba->ba_winsize << IEEE80211_ADDBA_BUFSZ_SHIFT |
-   tid << IEEE80211_ADDBA_TID_SHIFT |
-   IEEE80211_ADDBA_AMSDU;
-   if ((ic->ic_htcaps & IEEE80211_HTCAP_DELAYEDBA) == 0)
-   params |= IEEE80211_ADDBA_BA_POLICY; /* use immediate BA */
-   LE_WRITE_2(frm, params); frm += 2;
+   LE_WRITE_2(frm, ba->ba_params); frm += 2;
LE_WRITE_2(frm, ba->ba_timeout_val / IEEE80211_DUR_TU); frm += 2;
LE_WRITE_2(frm, ba->ba_winstart); frm += 2;
 
@@ -1465,9 +1459,10 @@ ieee80211_get_addba_resp(struct ieee8021
*frm++ = IEEE80211_ACTION_ADDBA_RESP;
*frm++ = token;
LE_WRITE_2(frm, status); frm += 2;
-   params = tid << 2 | IEEE80211_BA_ACK_POLICY;
if (status == 0)
-   params |= ba->ba_winsize << 6;
+   params = ba->ba_params;
+   else
+   params = tid << IEEE80211_ADDBA_TID_SHIFT;
LE_WRITE_2(frm, params); frm += 2;
if (status == 0)
LE_WRITE_2(frm, ba->ba_timeout_val / IEEE80211_DUR_TU);
Index: ieee80211_proto.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_proto.c,v
retrieving revision 1.62
diff -u -p -r1.62 ieee80211_proto.c
--- ieee80211_proto.c   4 Feb 2016 16:23:40 -   1.62
+++ ieee80211_proto.c   4 Feb 2016 16:50:50 -
@@ -646,6 +646,12 @@ ieee80211_addba_request(struct ieee80211
ba->ba_winsize = IEEE80211_BA_MAX_WINSZ;
ba->ba_winstart = ssn;
ba->ba_winend = (ba->ba_winstart + ba->ba_winsize - 1) & 0xfff;
+   ba->ba_params =
+   (ba->ba_winsize << IEEE80211_ADDBA_BUFSZ_SHIFT) |
+   

fix ADDBA params in ADDBA requests and responses

2016-02-04 Thread Stefan Sperling
This fixes up parameters in ADDBA requests and responses.

Again, the current code is using the wrong bitmask to set the ack
policy bit. IEEE80211_BA_ACK_POLICY is for QoS BlockAck request
and response frames, not ADDBA request and response frames.

It's important to echo back the set of parameters we got from the AP.
This part is necessary to make the iwn/iwm apple fix I sent yesterday
actually work.

Index: ieee80211_input.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.155
diff -u -p -r1.155 ieee80211_input.c
--- ieee80211_input.c   1 Feb 2016 18:43:22 -   1.155
+++ ieee80211_input.c   4 Feb 2016 08:28:18 -
@@ -2497,6 +2497,7 @@ ieee80211_recv_addba_req(struct ieee8021
ba->ba_state = IEEE80211_BA_INIT;
ba->ba_timeout_val = timeout * IEEE80211_DUR_TU;
ba->ba_ni = ni;
+   ba->ba_params = params;
timeout_set(&ba->ba_to, ieee80211_rx_ba_timeout, ba);
timeout_set(&ba->ba_gap_to, ieee80211_input_ba_gap_timeout, ba);
ba->ba_winsize = bufsz;
Index: ieee80211_node.h
===
RCS file: /cvs/src/sys/net80211/ieee80211_node.h,v
retrieving revision 1.54
diff -u -p -r1.54 ieee80211_node.h
--- ieee80211_node.h1 Feb 2016 18:43:22 -   1.54
+++ ieee80211_node.h4 Feb 2016 08:20:23 -
@@ -117,6 +117,9 @@ struct ieee80211_tx_ba {
 #define IEEE80211_BA_REQUESTED 1
 #define IEEE80211_BA_AGREED2
 
+   /* ADDBA parameter set field for this BA agreement. */
+   u_int16_t   ba_params;
+
/* These values are IEEE802.11 frame sequence numbers (0x0-0xfff) */
u_int16_t   ba_winstart;
u_int16_t   ba_winend;
@@ -137,6 +140,7 @@ struct ieee80211_rx_ba {
struct timeout  ba_to;
int ba_timeout_val;
int ba_state;
+   u_int16_t   ba_params;
u_int16_t   ba_winstart;
u_int16_t   ba_winend;
u_int16_t   ba_winsize;
Index: ieee80211_output.c
===
RCS file: /cvs/src/sys/net80211/ieee80211_output.c,v
retrieving revision 1.108
diff -u -p -r1.108 ieee80211_output.c
--- ieee80211_output.c  21 Jan 2016 20:33:20 -  1.108
+++ ieee80211_output.c  3 Feb 2016 16:48:06 -
@@ -1414,7 +1414,6 @@ ieee80211_get_addba_req(struct ieee80211
struct ieee80211_tx_ba *ba = &ni->ni_tx_ba[tid];
struct mbuf *m;
u_int8_t *frm;
-   u_int16_t params;
 
m = ieee80211_getmgmt(M_DONTWAIT, MT_DATA, 9);
if (m == NULL)
@@ -1424,12 +1423,15 @@ ieee80211_get_addba_req(struct ieee80211
*frm++ = IEEE80211_CATEG_BA;
*frm++ = IEEE80211_ACTION_ADDBA_REQ;
*frm++ = ba->ba_token;
-   params = ba->ba_winsize << IEEE80211_ADDBA_BUFSZ_SHIFT |
-   tid << IEEE80211_ADDBA_TID_SHIFT |
-   IEEE80211_ADDBA_AMSDU;
-   if ((ic->ic_htcaps & IEEE80211_HTCAP_DELAYEDBA) == 0)
-   params |= IEEE80211_ADDBA_BA_POLICY; /* use immediate BA */
-   LE_WRITE_2(frm, params); frm += 2;
+   if (ba->ba_params == 0) {
+   ba->ba_params =
+   (ba->ba_winsize << IEEE80211_ADDBA_BUFSZ_SHIFT) |
+   (tid << IEEE80211_ADDBA_TID_SHIFT) | IEEE80211_ADDBA_AMSDU;
+   if ((ic->ic_htcaps & IEEE80211_HTCAP_DELAYEDBA) == 0)
+   /* immediate BA */
+   ba->ba_params |= IEEE80211_ADDBA_BA_POLICY;
+   }
+   LE_WRITE_2(frm, ba->ba_params); frm += 2;
LE_WRITE_2(frm, ba->ba_timeout_val / IEEE80211_DUR_TU); frm += 2;
LE_WRITE_2(frm, ba->ba_winstart); frm += 2;
 
@@ -1465,9 +1467,10 @@ ieee80211_get_addba_resp(struct ieee8021
*frm++ = IEEE80211_ACTION_ADDBA_RESP;
*frm++ = token;
LE_WRITE_2(frm, status); frm += 2;
-   params = tid << 2 | IEEE80211_BA_ACK_POLICY;
if (status == 0)
-   params |= ba->ba_winsize << 6;
+   params = ba->ba_params;
+   else
+   params = tid << IEEE80211_ADDBA_TID_SHIFT;
LE_WRITE_2(frm, params); frm += 2;
if (status == 0)
LE_WRITE_2(frm, ba->ba_timeout_val / IEEE80211_DUR_TU);