d/freebsd-dev
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[net80211] address seqno allocation for group addressed frames

Browse Source 
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
This commit is contained in:
Adrian Chadd 2017-01-30 01:11:30 +00:00
parent 301c7e4c49
commit 9764ef21c4
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=312972
3 changed files with 69 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
sys/net80211/ieee80211_ht.c
View File

@ -827,6 +827,16 @@ ieee80211_ampdu_reorder(struct ieee80211_node *ni, struct mbuf *m)
*/
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

13
sys/net80211/ieee80211_input.h
View File

@ -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_node *ni, struct ieee80211_frame *wh,
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);
/*

57
sys/net80211/ieee80211_output.c
View File

@ -122,9 +122,7 @@ ieee80211_vap_pkt_send_dest(struct ieee80211vap *vap, struct mbuf *m,
{
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 ieee80211vap *vap, struct mbuf *m,
* 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 ieee80211vap *vap, struct mbuf *m,
* 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_node *ni,
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, struct ieee80211_node *ni,
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, struct ieee80211_node *ni,
* 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
@ -1559,6 +1578,22 @@ ieee80211_encap(struct ieee80211vap *vap, struct ieee80211_node *ni,
* and we don't need the TX lock held.
*/
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