Page MenuHomeFreeBSD

[net80211] address seqno allocation for group addressed frames
ClosedPublic

Authored by adrian on Jan 27 2017, 6:39 AM.

Details

Summary

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.

Test Plan
  • 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.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

adrian retitled this revision from to [net80211] address seqno allocation for group addressed frames.
adrian updated this object.
adrian edited the test plan for this revision. (Show Details)

Grr, turns out that 802.11-2012 states in 8.3.2.1 that we shouldn't even be sending multicast/broadcast data frames as QoS frames unless all members of the BSS are QoS. Fun times.

Guess I'll go add that one too.

sys/net80211/ieee80211_output.c
781 ↗(On Diff #24498)

is there any limitations for aggregating QoS Null frames?

I don't think we are allowed to - QoS NULL frames dont' have a sequence number from the normal TID space, so they aren't aggregated.

sys/net80211/ieee80211_input.h
191 ↗(On Diff #24498)

ieee80211_gettid() does not check MCAST address bit; so, this check should be omitted too (by moving IS_MULTICAST() check upper); otherwise, ni->ni_rxseqs[tid] may be updated in the 'ok' path.

Shift IEEE80211_IS_MULTICAST() above so ni_rxseqs[] isn't updated.

I'll create a new group addressed frame sequence number category later
on in a separate patch so we can do proper duplicate detection.

adrian added inline comments.
sys/net80211/ieee80211_output.c
781 ↗(On Diff #24498)

They don't get sequence numbers, so we shouldn't be aggregating them.

We can fix that in a separate follow-up commit.

avos edited edge metadata.
This revision is now accepted and ready to land.Jan 29 2017, 10:27 AM
This revision was automatically updated to reflect the committed changes.
adrian marked an inline comment as done.