net80211: set_vht_extchan() reverse order to always return best
ClosedPublic
Actions

Authored by bz on Aug 17 2020, 5:11 PM.

Details

Reviewers

adrian
gnn

Group Reviewers

wireless

Commits

rS364551: net80211: set_vht_extchan() reverse order to always return best

Summary

In set_vht_extchan() the checks are performed in the order of VHT20/40/80.
That means if a channel is has a lower and higheer VHT flag set we would
return the lower first.
We normally do not set more than one VHT flag so this change is supposed
to be a NOP but follows the logical thinking order of returning the best
first. Also we nowhere assert a single VHT flag so make sure we'll not
be stuck with VHT20 when we could do more.

While here add the debugging printfs for VHT160 and VHT80P80 which still
need to do once we deal with a driver at that level.

MFC After: 2 weeks
Reviewed by:
Sponsored by: Rubicon Communications, LLC (d/b/a "Netgate")

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bz created this revision.Aug 17 2020, 5:11 PM

Herald added a subscriber: melifaro. · View Herald TranscriptAug 17 2020, 5:11 PM

bz requested review of this revision.Aug 17 2020, 5:11 PM

Harbormaster completed remote builds in B33031: Diff 75898.Aug 17 2020, 5:12 PM

bz edited the summary of this revision. (Show Details)Aug 17 2020, 5:13 PM

Can you #define the magic number?

sys/net80211/ieee80211.c
1207 ↗	(On Diff #75898)	What is the meaning of the magic number 2? Can that be #defined to something?

This revision now requires changes to proceed.Aug 18 2020, 9:14 PM

bz added inline comments.Aug 19 2020, 2:56 PM

sys/net80211/ieee80211.c
1207 ↗	(On Diff #75898)	As you may notice, it comes from above as the block was moved down. This has to do with the base channel we are using and the center frequency for the 40Mhz channel. This is something the way it is in FreeBSD which also confused me initially. In short we operate on either channels or frequencies but to the user the the center frequency channels are currently not really exposed. So let me give you an example: you are on channel 44 (20Mhz) and you want to "upgrade" to 40 Mhz so it'll be 44+2 for the 46 which is the center for the HT40 channel. You are on channel 48 (20Mhz) and you want to "upgrade" to 40 Mhz so it'll we 48-2 for the 46 .. (which is below). You may notice that above for VHT80 we do a + 40 [Mhz] before converting Mhz to channel number. And there it's even more confusing. For 80+80 it'll be even worse with freq2; It's basically always grabbing an extra or 3 extra 20Mhz channels in the right places. I don't think "#defining" this to something is a good idea. Maybe adding a bit of commentary will do in the future.

Any further comments? I'd love to commit this Monday morning UTC.

gnn accepted this revision.Aug 20 2020, 11:14 PM

This revision is now accepted and ready to land.Aug 20 2020, 11:14 PM

adrian accepted this revision.Aug 23 2020, 4:43 PM

adrian added inline comments.

sys/net80211/ieee80211.c
1207 ↗	(On Diff #75898)	Yeah, it's part of the spec for 5GHz channels in 802.11. I think it's a bit different for 2GHz VHT support though (but honestly if we limited ourselves to 2GHz VHT20 only we'd likely be Just Fine.)