svn commit: r312972 - head/sys/net80211
Adrian Chadd
adrian at FreeBSD.org
Mon Jan 30 01:11:32 UTC 2017
Author: adrian
Date: Mon Jan 30 01:11:30 2017
New Revision: 312972
URL: https://svnweb.freebsd.org/changeset/base/312972
Log:
[net80211] address seqno allocation for group addressed frames
After some digging and looking at packet traces, it looks like the
sequence number allocation being done by net80211 doesn't meet
802.11-2012.
Specifically, group addressed frames (broadcast, multicast) have
sequence numbers allocated from a separate pool, even if they're
QoS frames.
This patch starts to try and address this, both on transmit and
receive.
* When receiving, don't throw away multicast frames for now.
It's sub-optimal, but until we correctly track group addressed
frames via another TID counter, this is the best we can do.
* When doing A-MPDU checks, don't include group addressed frames
in the sequence number checks.
* When transmitting, don't allocate group frame sequence numbers
from the TID, instead use the NONQOS TID for allocation.
This may fix iwn(4) 11n because I /think/ this was one of the
handful of places where ni_txseqs[] was being assigned /outside/
of the driver itself.
This however doesn't completely fix things - notably the way that
TID assignment versus WME assignment for driver hardware queues
will mess up multicast ordering. For example, if all multicast
QoS frames come from one sequence number space but they're
expected to obey the QoS value assigned, they'll end up in
different queues in the hardware and go out in different
orders.
I can't fix that right now and indeed fixing it will require some
pretty heavy lifting of both the WME<->TID QoS assignment, as well
as figuring out what the correct way for drivers to behave.
For example, both iwn(4) and ath(4) shouldn't put QoS multicast
traffic into the same output queue as aggregate traffic, because
the sequence numbers are all wrong. So perhaps the correct thing
to do there is ignore the WME/TID for QoS traffic and map it all
to the best effort queue or something, and ensure it doesn't
muck up the TID/blockack window tracking. However, I'm /pretty/
sure that is still going to happen.
.. maybe I should disable multicast QoS frames in general as well,
but I don't know what that'll do for whatever the current state
of 802.11s mesh support is.
Tested:
* STA mode, ath10k NIC
* AP mode, AR9344/AR9580 AP
* iperf tcp/udp tests with concurrent multicast QoS traffic.
Before this, iperfs would fail pretty quickly because the sending
AP would start sending out QoS multicast frames that would be
out of order from the rest of the TID traffic, causing the blockack
window to get way, way out of sync.
This now doesn't occur.
TODO:
* verify which QoS frames SHOULD be tagged as M_AMPDU_MPDU.
For example, QoS NULL frames shouldn't be tagged!
Reviewed by: avos
Differential Revision: https://reviews.freebsd.org/D9357
Modified:
head/sys/net80211/ieee80211_ht.c
head/sys/net80211/ieee80211_input.h
head/sys/net80211/ieee80211_output.c
Modified: head/sys/net80211/ieee80211_ht.c
==============================================================================
--- head/sys/net80211/ieee80211_ht.c Sun Jan 29 22:38:13 2017 (r312971)
+++ head/sys/net80211/ieee80211_ht.c Mon Jan 30 01:11:30 2017 (r312972)
@@ -827,6 +827,16 @@ ieee80211_ampdu_reorder(struct ieee80211
*/
return PROCESS;
}
+
+ /*
+ * 802.11-2012 9.3.2.10 - Duplicate detection and recovery.
+ *
+ * Multicast QoS data frames are checked against a different
+ * counter, not the per-TID counter.
+ */
+ if (IEEE80211_IS_MULTICAST(wh->i_addr1))
+ return PROCESS;
+
if (IEEE80211_IS_DSTODS(wh))
tid = ((struct ieee80211_qosframe_addr4 *)wh)->i_qos[0];
else
Modified: head/sys/net80211/ieee80211_input.h
==============================================================================
--- head/sys/net80211/ieee80211_input.h Sun Jan 29 22:38:13 2017 (r312971)
+++ head/sys/net80211/ieee80211_input.h Mon Jan 30 01:11:30 2017 (r312972)
@@ -149,6 +149,12 @@ ishtinfooui(const uint8_t *frm)
* (as the seqnum wraps), handle that special case so packets aren't
* incorrectly dropped - ie, if the next packet is sequence number 0
* but a retransmit since the initial packet didn't make it.
+ *
+ * XXX TODO: handle sequence number space wrapping with dropped frames;
+ * especially in high interference conditions under high traffic load
+ * The RX AMPDU reorder code also needs it.
+ *
+ * XXX TODO: update for 802.11-2012 9.3.2.10 Duplicate Detection and Recovery.
*/
static __inline int
ieee80211_check_rxseq(struct ieee80211_node *ni, struct ieee80211_frame *wh,
@@ -175,6 +181,13 @@ ieee80211_check_rxseq(struct ieee80211_n
if (! IEEE80211_HAS_SEQ(type, subtype))
return 1;
+ /*
+ * Always allow multicast frames for now - QoS (any TID)
+ * or not.
+ */
+ if (IEEE80211_IS_MULTICAST(wh->i_addr1))
+ return 1;
+
tid = ieee80211_gettid(wh);
/*
Modified: head/sys/net80211/ieee80211_output.c
==============================================================================
--- head/sys/net80211/ieee80211_output.c Sun Jan 29 22:38:13 2017 (r312971)
+++ head/sys/net80211/ieee80211_output.c Mon Jan 30 01:11:30 2017 (r312972)
@@ -122,9 +122,7 @@ ieee80211_vap_pkt_send_dest(struct ieee8
{
struct ieee80211com *ic = vap->iv_ic;
struct ifnet *ifp = vap->iv_ifp;
-#ifdef IEEE80211_SUPPORT_SUPERG
int mcast;
-#endif
if ((ni->ni_flags & IEEE80211_NODE_PWR_MGT) &&
(m->m_flags & M_PWR_SAV) == 0) {
@@ -164,9 +162,7 @@ ieee80211_vap_pkt_send_dest(struct ieee8
* interface it (might have been) received on.
*/
m->m_pkthdr.rcvif = (void *)ni;
-#ifdef IEEE80211_SUPPORT_SUPERG
mcast = (m->m_flags & (M_MCAST | M_BCAST)) ? 1: 0;
-#endif
BPF_MTAP(ifp, m); /* 802.3 tx */
@@ -181,10 +177,15 @@ ieee80211_vap_pkt_send_dest(struct ieee8
* The default ic_ampdu_enable routine handles staggering
* ADDBA requests in case the receiver NAK's us or we are
* otherwise unable to establish a BA stream.
+ *
+ * Don't treat group-addressed frames as candidates for aggregation;
+ * net80211 doesn't support 802.11aa-2012 and so group addressed
+ * frames will always have sequence numbers allocated from the NON_QOS
+ * TID.
*/
if ((ni->ni_flags & IEEE80211_NODE_AMPDU_TX) &&
(vap->iv_flags_ht & IEEE80211_FHT_AMPDU_TX)) {
- if ((m->m_flags & M_EAPOL) == 0) {
+ if ((m->m_flags & M_EAPOL) == 0 && (! mcast)) {
int tid = WME_AC_TO_TID(M_WME_GETAC(m));
struct ieee80211_tx_ampdu *tap = &ni->ni_tx_ampdu[tid];
@@ -776,12 +777,20 @@ ieee80211_send_setup(
* requiring the TX lock.
*/
tap = &ni->ni_tx_ampdu[tid];
- if (tid != IEEE80211_NONQOS_TID && IEEE80211_AMPDU_RUNNING(tap))
+ if (tid != IEEE80211_NONQOS_TID && IEEE80211_AMPDU_RUNNING(tap)) {
m->m_flags |= M_AMPDU_MPDU;
- else {
+ } else {
if (IEEE80211_HAS_SEQ(type & IEEE80211_FC0_TYPE_MASK,
type & IEEE80211_FC0_SUBTYPE_MASK))
- seqno = ni->ni_txseqs[tid]++;
+ /*
+ * 802.11-2012 9.3.2.10 - QoS multicast frames
+ * come out of a different seqno space.
+ */
+ if (IEEE80211_IS_MULTICAST(wh->i_addr1)) {
+ seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++;
+ } else {
+ seqno = ni->ni_txseqs[tid]++;
+ }
else
seqno = 0;
@@ -1239,7 +1248,7 @@ ieee80211_encap(struct ieee80211vap *vap
struct ieee80211_frame *wh;
struct ieee80211_key *key;
struct llc *llc;
- int hdrsize, hdrspace, datalen, addqos, txfrag, is4addr;
+ int hdrsize, hdrspace, datalen, addqos, txfrag, is4addr, is_mcast;
ieee80211_seq seqno;
int meshhdrsize, meshae;
uint8_t *qos;
@@ -1247,6 +1256,8 @@ ieee80211_encap(struct ieee80211vap *vap
IEEE80211_TX_LOCK_ASSERT(ic);
+ is_mcast = !! (m->m_flags & (M_MCAST | M_BCAST));
+
/*
* Copy existing Ethernet header to a safe place. The
* rest of the code assumes it's ok to strip it when
@@ -1291,11 +1302,19 @@ ieee80211_encap(struct ieee80211vap *vap
* ap's require all data frames to be QoS-encapsulated
* once negotiated in which case we'll need to make this
* configurable.
- * NB: mesh data frames are QoS.
+ *
+ * Don't send multicast QoS frames.
+ * Technically multicast frames can be QoS if all stations in the
+ * BSS are also QoS.
+ *
+ * NB: mesh data frames are QoS, including multicast frames.
*/
- addqos = ((ni->ni_flags & (IEEE80211_NODE_QOS|IEEE80211_NODE_HT)) ||
+ addqos =
+ (((is_mcast == 0) && (ni->ni_flags &
+ (IEEE80211_NODE_QOS|IEEE80211_NODE_HT))) ||
(vap->iv_opmode == IEEE80211_M_MBSS)) &&
(m->m_flags & M_EAPOL) == 0;
+
if (addqos)
hdrsize = sizeof(struct ieee80211_qosframe);
else
@@ -1560,6 +1579,22 @@ ieee80211_encap(struct ieee80211vap *vap
*/
if ((m->m_flags & M_AMPDU_MPDU) == 0) {
/*
+ * 802.11-2012 9.3.2.10 -
+ *
+ * If this is a multicast frame then we need
+ * to ensure that the sequence number comes from
+ * a separate seqno space and not the TID space.
+ *
+ * Otherwise multicast frames may actually cause
+ * holes in the TX blockack window space and
+ * upset various things.
+ */
+ if (IEEE80211_IS_MULTICAST(wh->i_addr1))
+ seqno = ni->ni_txseqs[IEEE80211_NONQOS_TID]++;
+ else
+ seqno = ni->ni_txseqs[tid]++;
+
+ /*
* NB: don't assign a sequence # to potential
* aggregates; we expect this happens at the
* point the frame comes off any aggregation q
More information about the svn-src-head
mailing list