Page MenuHomeFreeBSD

net80211: convert ni_txrate to a struct, with extra rate information
Needs ReviewPublic

Authored by adrian on Thu, Jan 23, 2:36 AM.
Referenced Files
Unknown Object (File)
Wed, Jan 29, 1:30 PM
Unknown Object (File)
Tue, Jan 28, 3:51 PM
Unknown Object (File)
Tue, Jan 28, 3:20 PM
Unknown Object (File)
Tue, Jan 28, 2:38 PM
Unknown Object (File)
Tue, Jan 28, 7:51 AM
Unknown Object (File)
Fri, Jan 24, 12:49 AM
Unknown Object (File)
Thu, Jan 23, 9:13 PM
Unknown Object (File)
Thu, Jan 23, 5:38 PM

Details

Reviewers
bz
Group Reviewers
wireless
Summary
  • create struct ieee80211_node_txrate, which represents both the existing legacy / HT rates, and makes space for VHT rates
  • convert the ieee80211_node_* routines that manipulate the txrate field to use the new format
  • return OFDM6 for now if a VHT rate is set but the driver calls ieee80211_node_get_txrate_dot11rate(). It SHOULD be the lowest available rate - which for 11b will be a CCK rate, for turbo/half/quarter will be different rates! - but this is just for development.

This should be a no-op for existing drivers and rate control, as
everything now uses the accessor functions instead of directly
accessing ni->ni_txrate.

Locally tested:

  • RTL8821AU, STA mode (5GHz VHT/40)

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 62126
Build 59010: arc lint + arc unit

Event Timeline

bz requested changes to this revision.Mon, Jan 27, 12:21 AM
bz added a subscriber: bz.

Actually not requesting changes yet but more requesting discussion ...

sys/net80211/ieee80211_node.h
142

Make this 0.

143

I'd suffix that with _MCS

146

I'd call them flags; I am not sure what you envision there to be defined beyond that but sooner or later ... I think you may want to have SGI in there too as a flag.

147

Remove VHT from comments. It's NSS and MCS.

In the end what you are modelling here seems to be struct rate_info in sys/compat/linuxkpi/common/include/net/cfg80211.h minus (the "bw" and the the MCS case [unless you cleanup HT]).

That struct already has fields for HE/EHT.

it is funny we want to rewrite the Linux drivers but then still have own data structures rather than closing the rapture. It's funny how I can see where all these things are coming from ...
grep -r 'struct rate_info' sys/contrib/dev | grep txrate # ?

So here's the question: do you want to cleanup HT to use mcs and a flag and we can move things over for this and save LinuxKPI the conversions or do we keep inventing our own names?
XXX-BZ "legacy" is in 100kbit/s ... D48601

This revision now requires changes to proceed.Mon, Jan 27, 12:21 AM
sys/net80211/ieee80211_node.h
142

It's 0 so "uninitialised" (which is hopefully 0) gets caught.

147

Well, they come from "if i needed to represent stuff how would i", I'm not basing it on the linux side of things. The only time I've looked at the linux side of things is when I hit ieee80211_* namespace clashes. :-)

It's named VHT-NSS / MCS right now because it's explicitly /not/ representing 11n just yet. 11n MCS is still represented in the "legacy" way. I'd like to eventually clean that up, and it shouldn't be hard to do with the cleanups going on in here. When that happens, I'll remove the VHT bits.

Also, linux /does/ change stuff - eg look at the stuff that mechanically changed when they implemented the dual-sta support. I'd prefer we stick with translations in linuxkpi - even if they're simple mechanical translations - so if/when they do change other stuff, we've limited how far the changes end up spreading in our codebase.

(And yeah I do owe you and others an email on porting linux drivers to freebsd with shims rather than a linuxkpi; I've had a bunch of experience with "drivers that run on any wifi stack" <-> "driver shim layer" "stack shim layer" <-> "wifi stack". It's .. dirty. ;-)

sys/net80211/ieee80211_node.h
142

Ok, if you prefer to stay with something ..

Can we make this
(a) an enum
(b) remove the "TX" from the name as you'll need the same information in the RX path (and yes that's another construction site).
(c) reserve a value for _MCS (or HT as you like) as compared to VHT (or VHT_MCS)
?

updated; now it actually does do _HT and _VHT as separate rates, although both LEGACY and HT set dot11rate. That simplifies everything else.