Page MenuHomeFreeBSD

net80211: remove direct use of ni->ni_txrate, add indirection methods
Needs RevisionPublic

Authored by adrian on Thu, Jan 23, 2:34 AM.

Details

Reviewers
thj
Group Reviewers
wireless
Summary

The summary:

  • Refactor ni_txrate access into ieee80211_node_get_txrate_dot11rate() and ieee80211_node_set_txrate_dot11rate(). These wrap the ni->ni_txrate access and will eventually be able to do runtime sanity checks and fallback where necessary.
  • Refactor ieee80211_node_get_txrate_mbit() from the ioctl code which sets isi_txmbps (which is in 0.5mbit units.)
  • Also use ieee80211_node_get_txrate_mbit() in various places in the code where the dot11rate was turned into a Mb value, which was very wrong for HT (but also only used for logging, so it didn't have an effect on normal runtime.)

The long version:

The current ni->ni_txrate value is what net80211's phy code
calls a 'dot11rate'. Inside the ieee80211_phy.c tables you'll
find a bunch of tables which represent:

  • for legacy rates its in 1/2 mbit units.
  • for turbo (Atheros 40MHz OFDM) it's the non-turbo rates, but the turbo rate speed in kbit/sec.
  • for 802.11n rates its the MCS, starting at 0x80.

However there are a couple of catches with this:

  • Basic rates are represented in the pre-11n rates using the high bit (IEEE80211_RATE_BASIC)
  • 11n rates are also represented using the high bit (IEEE80211_RATE_MCS)

Now, ni->ni_txrate will clear the IEEE80211_RATE_BASIC flag before
storing it, so if the high bit exists it must be an 802.11n rate.
However, there's still a bunch of code everywhere that purposefully
filters that out.

The goals of this commit:

  • Provide an easy API to migrate existing drivers and other consumers to - ieee80211_node_get_txrate_dot11rate() is defined as "will return the normal legacy or HT rate" so all the existing code can work.
  • Lay the ground work for extending ni_txrate (and a rate representation in general) that can represent legacy, HT, VHT, EHT, HE, etc rates.
  • Create a central place where ni_txrate is updated from rate control, drivers that will update ni_txrate itself, and consumers, so we can provide some basic runtime checks / logging as VHT, EHT, HE, etc rates are eventually added.

For example, a VHT driver will eventually receive VHT rates, but an
existing HT driver will not, so the API should log and return a
sensible default when something like a VHT rate shows up on a HT only
device.

The rate control code currently returns a rix, and sets ni_txrate to the
dot11rate. Drivers can choose either. However, choosing the rix is
risky because you need to know if it's the ni_rates or ni_htrates, which
requires a lot of duplicate work that lines up consistently at all
layers (see the AMRR code for an example.)

Diff Detail

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

Event Timeline

thj requested changes to this revision.Thu, Jan 23, 4:23 PM
thj added a subscriber: thj.
thj added inline comments.
sys/net80211/ieee80211_node.c
3171

This is asking for trouble, we should either return Mbit or call this accessor something else

ieee80211_node_get_txrate_500kbit is terrible, but won't lead to mistakes.

I'd suggest that the mbps variable here is also misleading. If I wasn't reading the review I would be very unsure if the comment at the top is incorrect or if the code is

This revision now requires changes to proceed.Thu, Jan 23, 4:23 PM