net80211: add helper functions for VHT transmit

[adrian]

Summary

Add helper functions for VHT TX for 20MHz, 40MHz and 80MHz.

[fuz]

Nitpick: ! is not followed by a space as per style(9).

[gavin]

Other than the style nit, looks good to me

[bz]

Given we'll need VHT160 as well at least I wonder if here we can make the 40/80/160 macrofized because all that changes is simply the number. 80P80 is going to be more tricky but we should add that one as well?

[adrian]

i think maybe we should do both - I can add a check_tx_vht160 and check_tx_vht_80p80 that will check each of the channel configs appropriately, and then a top-level function that does it based on a switch. That way each function is clean, and we can make them all inlines.

How's that sound?

[adrian]

make them all static***

[bz]

You mean ieee80211_vht_check_tx_vht(ni, enum ieee80211_vht_chanwidth) for example (not sure if it's the best enum to use but..)? I think that would be great.

[adrian]

yup, that.

[adrian]

ah crap, hm, I can't use ieee80211_vht_chanwidth as it doesn't have a 40/80 bandwidth option.

The closest is ieee80211_sta_rx_bw .

[adrian]

Oh, this leads to another fun question, especially in hostap mode - what do we do when the bss vap bandwidth is much higher than the node bandwidth?

For STA mode the node / bss node is the same, so the logic makes sense. The driver can check 160, 80, 40, 20 in order and choose.

For AP / IBSS modes however, the node bandwidth may be smaller than the bss node, and the current width can change to be smaller than that too.

Eg:

• for an 80MHz AP with a 40MHz client with a 40MHz ni_chw, the bss channel will be VHT80, the node channel with be VHT40, so I believe that check will fail
• for an 80MHz AP with a 40MHz client and a 20MHz ni_chw, the bss channel will be VHT80, the node channel will be VHT40, and ni_chw will be RX_BW_20, so the check will definitely fail

I do like the API change though, so I'm going to do the API change, make it work in my patch set, and THEN we can figure out how to clean all of this up a bit more.

[adrian]

update after talking with bz@

[adrian]

ok this looks like it'll be fine for now, VHT40 is set for VHT80 channels, VHT80 is set for VHT80+80 and VHT160 channels, so I think we're OK.

Let's get this into the tree and start using it in the rtwn TX path and see how it all plays out in production.

[adrian]

rebase

[emaste]

sys/net80211/ieee80211_vht.c

886

Are these NULL conditions (like ni == NULL) plausible -- should they be asserts instead?

911

style(9) has no space after the !

934

style(9)

957

style(9)

992

these functions are all very similar and the latter ones all call ieee80211_vht_check_tx_vht anway, would it be clearer to just do

bool
ieee80211_vht_check_tx_bw(const struct ieee80211_node *ni,
    enum ieee80211_sta_rx_bw bw)
{
        <code from ieee80211_vht_check_tx_vht, return false if not>

        switch (bw) {
        case IEEE80211_STA_RX_BW_20:
                return (true);
        case IEEE80211_STA_RX_BW_40:
                return (IEEE80211_IS_CHAN_VHT40(bss_chan) &&
                    IEEE80211_IS_CHAN_VHT40(ni->ni_chan) &&
                    (ni->ni_chw == IEEE80211_STA_RX_BW_40));
...

[adrian]

feedback from emaste, clarify the comments, add headerdoc bits

[adrian]

sys/net80211/ieee80211_vht.c

886

I'm a big fan of handling cases without asserting, so the non-debug kernel will not deref crazy stuff. But I can add some KASSERTs() in a later commit!

992

these functions are all very similar and the latter ones all call ieee80211_vht_check_tx_vht anway, would it be clearer to just do

bool
ieee80211_vht_check_tx_bw(const struct ieee80211_node *ni,
    enum ieee80211_sta_rx_bw bw)
{
        <code from ieee80211_vht_check_tx_vht, return false if not>

        switch (bw) {
        case IEEE80211_STA_RX_BW_20:
                return (true);
        case IEEE80211_STA_RX_BW_40:
                return (IEEE80211_IS_CHAN_VHT40(bss_chan) &&
                    IEEE80211_IS_CHAN_VHT40(ni->ni_chan) &&
                    (ni->ni_chw == IEEE80211_STA_RX_BW_40));
...

I'd like to leave them refactored out for now until we know there aren't any other weird corner cases we want to handle. They're static methods so noone should be using them directly anyway.