ath / net80211: check node flags rather than ic/vap/ni htcaps for SGI

[bz]

Summary

JUST UP FOR DISUSSION NOT FOR THE REVIEW

The code here (and possibly in mwl?) have a builtin assumption that
ni_chw is not set to anything before we go through
ieee80211_ht_updateparams() / ieee80211_ht_updatehtcap() or its final
version.

As a result with ni_chw no initialized to BW_20 which matches the default
malloc 0 compared to the past where ni_chw was 0 was not valid at all as
20 meant BW_20 so the checks are matching even before (RE)ASSOC requests
in the STA or AP case.

with this it is questionable if the checks in
ieee80211_ht_check_tx_shortgi_[24]0() are correct either as things
work either by very detailed knowledge or accident in the past.

[bz]

This likely matches the comment which came along this code being enabled in 6246be6e58d86770e7db34dd8d30cc453a19eab8

[adrian]

Maybe we should considering adding IEEE80211_STA_RX_BW_UNDEFINED = 0 in the set, and an accessor method or two?

I know now that we're just plainly not doing it right for 80/160/320 either in the VHT code I added. :(

[bz]

In D49198#1122064, @adrian wrote:

Maybe we should considering adding IEEE80211_STA_RX_BW_UNDEFINED = 0 in the set, and an accessor method or two?

You know the comment from the review above the enum from the 2nd commit. I think I'll pull that enum back into LinuxKPI.

I know now that we're just plainly not doing it right for 80/160/320 either in the VHT code I added. :(

That said, your original commit to if_ath_tx_ht.c said exactly that you did not have the negotiated set as a flag and that is what the above is if I read the code correctly for both STA and AP.

The assumption that ni_chw was 0 and not 20 right from the start has hidden this problem for 14(?) years?

[adrian]

In D49198#1122070, @bz wrote:

In D49198#1122064, @adrian wrote:

Maybe we should considering adding IEEE80211_STA_RX_BW_UNDEFINED = 0 in the set, and an accessor method or two?

You know the comment from the review above the enum from the 2nd commit. I think I'll pull that enum back into LinuxKPI.

*nod*

I know now that we're just plainly not doing it right for 80/160/320 either in the VHT code I added. :(

That said, your original commit to if_ath_tx_ht.c said exactly that you did not have the negotiated set as a flag and that is what the above is if I read the code correctly for both STA and AP.

The assumption that ni_chw was 0 and not 20 right from the start has hidden this problem for 14(?) years?

I /think/ so but I need to understand exactly why this is happening.

Oh wait I think I know, I /think/ short-GI isn't supported in 20MHz, only in 40MHz. Aha! Yes, in if_ath.c:

/*
 * Enable short-GI for HT20 only if the hardware
 * advertises support.
 * Notably, anything earlier than the AR9287 doesn't.
 */

That's likely it!

[adrian]

ok, let's ask nathan to test with this, and if it works, I'll go put in a diff to fix it in ieee80211_ht_check_tx_shortgi20 / ieee80211_ht_check_tx_shortgi40.

[bz]

I /think/ so but I need to understand exactly why this is happening.

Oh wait I think I know, I /think/ short-GI isn't supported in 20MHz, only in 40MHz. Aha! Yes, in if_ath.c:

/*
 * Enable short-GI for HT20 only if the hardware
 * advertises support.
 * Notably, anything earlier than the AR9287 doesn't.
 */

That's likely it!

That makes no sense, then it shouldn't be advertised in ic->ic_htcaps & IEEE80211_HTCAP_SHORTGI20 and the check should fail. Unless there are more inconsistencies.

[adrian]

In D49198#1122092, @bz wrote:

I /think/ so but I need to understand exactly why this is happening.

Oh wait I think I know, I /think/ short-GI isn't supported in 20MHz, only in 40MHz. Aha! Yes, in if_ath.c:

/*
 * Enable short-GI for HT20 only if the hardware
 * advertises support.
 * Notably, anything earlier than the AR9287 doesn't.
 */

That's likely it!

That makes no sense, then it shouldn't be advertised in ic->ic_htcaps & IEEE80211_HTCAP_SHORTGI20 and the check should fail. Unless there are more inconsistencies.

That wouldn't surprise me. I'll go double/triple check.

I can whack an AR5418 and AR9280 (where it's not supported) in and fire things up and take a look.