Stefan Sperling
2016-02-11 15:07:09 UTC
The current A-MPDU reordering code is being fed with not just QoS data
frames but also QoS "no data" frames. Such frames do not contain sequence
numbers and our reordering buffer logic will treat them as if they did.
QoS frame subtype fields (in binary) and their corresponding names are:
1000 QoS Data
1001 QoS Data + CF-Ack
1010 QoS Data + CF-Poll
1011 QoS Data + CF-Ack + CF-Poll
1100 QoS Null (no data)
1101 Reserved
1110 QoS CF-Poll (no data)
1111 QoS CF-Ack + CF-Poll (no data)
^
|
+--- the "no data" bit
This diff prevents us from trying to reorder QoS "no data" frames, which
end up confusing the reordering logic and causing traffic to stall.
The only APs I have which send such frames are Broadcom-based, so I had
been blaming this bug on their APs. Apologies, Broadcom :)
These debug messages I put in place a few days ago disappear:
ieee80211_input_ba: received frame with bad sequence number 1478, expecting 228:291
Because ieee80211_has_qos() only looks at the most signicant subtype bit
checking just the 'hasqos' variable by itself is not sufficient.
We need to check the "no data" bit, too.
Index: ieee80211_input.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.162
diff -u -p -r1.162 ieee80211_input.c
--- ieee80211_input.c 9 Feb 2016 13:48:31 -0000 1.162
+++ ieee80211_input.c 11 Feb 2016 14:48:04 -0000
@@ -275,6 +275,7 @@ ieee80211_input(struct ifnet *ifp, struc
}
if (type == IEEE80211_FC0_TYPE_DATA && hasqos &&
+ (subtype & IEEE80211_FC0_SUBTYPE_NODATA) == 0 &&
!(rxi->rxi_flags & IEEE80211_RXI_AMPDU_DONE)) {
int ba_state = ni->ni_rx_ba[tid].ba_state;
frames but also QoS "no data" frames. Such frames do not contain sequence
numbers and our reordering buffer logic will treat them as if they did.
QoS frame subtype fields (in binary) and their corresponding names are:
1000 QoS Data
1001 QoS Data + CF-Ack
1010 QoS Data + CF-Poll
1011 QoS Data + CF-Ack + CF-Poll
1100 QoS Null (no data)
1101 Reserved
1110 QoS CF-Poll (no data)
1111 QoS CF-Ack + CF-Poll (no data)
^
|
+--- the "no data" bit
This diff prevents us from trying to reorder QoS "no data" frames, which
end up confusing the reordering logic and causing traffic to stall.
The only APs I have which send such frames are Broadcom-based, so I had
been blaming this bug on their APs. Apologies, Broadcom :)
These debug messages I put in place a few days ago disappear:
ieee80211_input_ba: received frame with bad sequence number 1478, expecting 228:291
Because ieee80211_has_qos() only looks at the most signicant subtype bit
checking just the 'hasqos' variable by itself is not sufficient.
We need to check the "no data" bit, too.
Index: ieee80211_input.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.162
diff -u -p -r1.162 ieee80211_input.c
--- ieee80211_input.c 9 Feb 2016 13:48:31 -0000 1.162
+++ ieee80211_input.c 11 Feb 2016 14:48:04 -0000
@@ -275,6 +275,7 @@ ieee80211_input(struct ifnet *ifp, struc
}
if (type == IEEE80211_FC0_TYPE_DATA && hasqos &&
+ (subtype & IEEE80211_FC0_SUBTYPE_NODATA) == 0 &&
!(rxi->rxi_flags & IEEE80211_RXI_AMPDU_DONE)) {
int ba_state = ni->ni_rx_ba[tid].ba_state;