net80211: add macros for testing if a channel is defined / channel is "any channel" * A channel being defined is currently "channel is not null". APIs can return a NULL channel if there's no channel available to match some criteria. * A channel being defined as "any channel" is NOT NULL and a magic value (0xffff). Deferencing this is obviously bad. * Two channels being equivalent is right now defined as the pointers being equivalent but it should eventually be the a subset of the channel parameters that match. We should be hiding the inner workings of the channel represent / contents from consumers, and this will let us lay the groundwork for further detailed checks in the channel macros (ie, avoid NULL / ANYC dereferencing.) * create a macros that returns true if the channel is defined (IEEE80211_IS_CHAN_DEFINED()) * create a macro that return true if the channel is the "any channel" (IEEE80211_IS_CHAN_ANYC()) * Create a macro that tests for channel equivalence (IEEE80211_IS_CHAN_EQUIV()) * Convert a handful of places over to use the new macros as as atest. This is mostly a test and request for feedback before the rest of the net80211 stack / drivers are converted.
Details
Details
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 61299 Build 58183: arc lint + arc unit
Event Timeline
Comment Actions
This looks reasonable.
I cannot judge if it is technically correct though, as I lack background knowledge.
Comment Actions
I replied to wireless@ for this. Also it really doesn't belong into this stack of changes.
Comment Actions
Ok, so where do you want to take this? Find all the places in the tree and replace them, just do net80211 and leave the drivers? Just add the macros and let someone else do them all in a go?
Also the IEEE80211_IS_CHAN_ANY() according to https://lists.freebsd.org/archives/freebsd-wireless/2025-January/002700.html
Comment Actions
So after some digging into it, I think I want to do the following:
- Converting stuff to IEEE80211_IS_CHAN_ANY(c) is pretty easy, I did what I could find
- Define "a channel that hasn't been initialised will result in IEEE80211_IS_CHAN_DEFINED(c) returning false", so functions like the ieee82011_find_channel*() family can be defined as "return a channel, or ! IEEE80211_IS_CHAN_DEFINED(returned channel)"
I have a bunch of other things I'd like to do. I think a big part of sorting out the "stop the channel needing to be a pointer to a global array of channels" is initially "stop assuming channels are pointers" and make them increasingly opaque.
I think the eventual outcome is that instead of the channels being a pointer to the channel array, the channel representation in the node/vap, drivers, etc become an actual "ieee80211_channel" which actually gets modified.
Here's my shortlist thus far:
- accessing a channel goes via inlines/macros, not eg chan->ic_freq (we already do this for IEEE80211_IS_CHAN_xxx() but we don't do it for everything);
- methods to get and set the channel information are provided, rather than just pointer stuff - eg setting the bss channel / node channel is done via a method, not by pointer assignment;
- testing for channel equivalence isn't done via pointer comparison - introduce a macro or inline that tests that they are equivalent freq, width, op modes;
- functions returning a channel or NULL take a "struct ieee80211_channel **" and return true/false, and either set the pointer to the channel or to NULL (ie, the IEEE80211_IS_CHAN_DEFINED() behaviour mentioned above).
I'm pretty sure we can do this in an incremental fashion where the driver use of channels become opaque, and then slowly migrating the
net80211 stack away from the channel array and by explicitly setting a channel struct and testing for equivalence / getting information via
the above kind of refactored methods/macros/etc.
Eventually we should be left with the global channel array, the ieee80211_find_channel*() style API calls and the regulatory domain code
being the only "array" of channels and how they're found/iterated through, and then we can replace that with equivalent representation
and logic.
(For example there's plenty of "iterate through channels and only return those with specific b, bg, bgn, a, an, bgn/vht, an/vht, etc
flags" and in a world with no global list of those, we'll have to basically create channel search iterators.)
Comment Actions
IEEE80211_IS_CHAN_DEFINED is not really used as such in drivers though there are plenty of tests.
There's still drivers missing that are using ANYC and not converted to macros. Do the all cleanly in one go.
PS: as a side note: I wish OpenBSD/..whoever ported them to FreeBSD would have copy&pasted them the same way in each driver and not spelt things three different ways.
| sys/compat/linuxkpi/common/src/linux_80211.c | ||
|---|---|---|
| 1341 ↗ | (On Diff #178537) | IS_DEFINED? |
| 2580 ↗ | (On Diff #178537) | These are all !IS_DEFINED cases |
| 2620 ↗ | (On Diff #178537) | This is a IS_DEFINED |
| 5206 ↗ | (On Diff #178537) | !DEFINED ... and so on, I just stop adding comments. |
Comment Actions
- add IEEE80211_IS_CHAN_EQUIV() as part of this test
- add a few more conversions
This still isn't complete, but I think it's a good enough exploration of
the migration / refactoring strategy.
Comment Actions
- migrate to NET80211_CHANNEL_* macro naming, to avoid adding more IEEE80211 pollution for things that aren't in the 802.11 spec
- .. and they're now clearly pointer versions of the macros
- Add some missing linuxkpi conversions, thanks bz
See https://wiki.freebsd.org/AdrianChadd/Net80211ChannelRepresentation2026 for more
information about this design.
| sys/net80211/ieee80211_vht.c | ||
|---|---|---|
| 887 | nope, this is wrong - iv_bss is a node, not a channel | |