net80211: add IEEE80211_IS_QOS_NULL()
ClosedPublic
Actions

Authored by adrian on Sun, Nov 17, 6:09 AM.

Details

Reviewers

None

Group Reviewers

wireless

Commits

rG1375790a15b1: net80211: add IEEE80211_IS_QOS_NULL()

Summary

This will be useful when fixing up the sequence number generation
and checks, as the rules around how sequence numbers are generated
have been clarified in 802.11-2016 and later. QoS-NULL frames are
explicitly marked as "any sequence number".

But for now, just create a macro and use it in the one place
it's currently being used as a check - ath(4).

Add IEEE80211_IS_QOS_NULL()
Change the "will this frame go into the TX block-ack window" check in the ath(4) transmit path.

Locally tested:

ath(4), AR9380, STA mode w/ AMPDU TX/RX enabled and negotiated

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 60640
Build 57524: arc lint + arc unit

Event Timeline

adrian created this revision.Sun, Nov 17, 6:09 AM

Herald added subscribers: glebius, melifaro, imp. · View Herald TranscriptSun, Nov 17, 6:09 AM

adrian requested review of this revision.Sun, Nov 17, 6:09 AM

Harbormaster completed remote builds in B60639: Diff 146609.Sun, Nov 17, 6:09 AM

add url

Harbormaster completed remote builds in B60640: Diff 146610.Sun, Nov 17, 6:10 AM

adrian added a parent revision: D47522: rtwn: change the USB TX transfers to only do one pending transfer per endpoint.Sun, Nov 17, 6:34 AM

adrian added a reviewer: wireless.

adrian added a project: wireless.

The commit message sounds like this is a NFC but IEEE80211_FC0_VERSION_MASK and `IEEE80211_FC0_TYPE_MASK are now being checked where they were not before? I think this should be clarified in the commit message.

In D47645#1086332, @emaste wrote:

The commit message sounds like this is a NFC but IEEE80211_FC0_VERSION_MASK and `IEEE80211_FC0_TYPE_MASK are now being checked where they were not before? I think this should be clarified in the commit message.

good call, lemme go edit that.

bz added a subscriber: bz.Sun, Nov 17, 6:00 PM

bz added inline comments.

sys/dev/ath/if_ath_tx.c
2993	Can we move this into the DPRINTF at the end and remove the local variable "subtype" entirely? If I am not wrong we are not adjusting tha part of wh so should be possible?
sys/net80211/ieee80211.h
278	Funny enough I missed this. It does not actually return anything given it's not a function. "Evaluate to true". You also don't really need those comments given the macros are IS QOS_NULL is more or less self-explanatory. Could at least condense them to a single line comment. But you could also just ignore me on this one ;-)

adrian added inline comments.Sun, Nov 17, 9:53 PM

sys/dev/ath/if_ath_tx.c
2993	Yeah, lemme look at how else I can clean this up. I think we'd also benefit from a couple of macros to fetch the type and subtype, and also set the type and subtype. What do you think?
sys/net80211/ieee80211.h
278	ah good call on condensing to one line. And yeah, "returns true" is a stretch, I know. :-) Lemme go clean that up.