Page MenuHomeFreeBSD

ifconfig: remove debug printfs from set80211vhtconf()
AcceptedPublic

Authored by bz on Sat, Jan 4, 1:56 PM.

Details

Reviewers
adrian
emaste
Group Reviewers
wireless
Summary

Anyone testing VHT options would wonder about these extra two printfs
by now. Remove them from the tree before I have to do locally again
in another branch.

Sponsored by: The FreeBSD Foundation
Fixes: e9bb7f9aa1b4f
MFC after: 1 week

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 61501
Build 58385: arc lint + arc unit

Event Timeline

bz requested review of this revision.Sat, Jan 4, 1:56 PM
This revision is now accepted and ready to land.Sat, Jan 4, 3:55 PM

That /does/ remind me, we should figure out why "ifconfig wlan0 -vht" doesn't work :-)

(ifconfig wlan0 -ht and ifconfig wlan0 ht works though!)

That /does/ remind me, we should figure out why "ifconfig wlan0 -vht" doesn't work :-)

(ifconfig wlan0 -ht and ifconfig wlan0 ht works though!)

shot in the dark?

diff --git sbin/ifconfig/ifieee80211.c sbin/ifconfig/ifieee80211.c
index e9337cf37917..24a6d9180a64 100644
--- sbin/ifconfig/ifieee80211.c
+++ sbin/ifconfig/ifieee80211.c
@@ -6031,7 +6031,7 @@ static struct cmd ieee80211_cmds[] = {
        DEF_CMD("ht",           3,      set80211htconf),        /* NB: 20+40 */
        DEF_CMD("-ht",          0,      set80211htconf),
        DEF_CMD("vht",          IEEE80211_FVHT_VHT,             set80211vhtconf),
-       DEF_CMD("-vht",         0,                              set80211vhtconf),
+       DEF_CMD("-vht",         -IEEE80211_FVHT_VHT,            set80211vhtconf),
        DEF_CMD("vht40",        IEEE80211_FVHT_USEVHT40,        set80211vhtconf),
        DEF_CMD("-vht40",       -IEEE80211_FVHT_USEVHT40,       set80211vhtconf),
        DEF_CMD("vht80",        IEEE80211_FVHT_USEVHT80,        set80211vhtconf),

rofl nice catch, wanna land that too? :-)

rofl nice catch, wanna land that too? :-)

The question is, should -vht also clear -vht40 -vht80 -vht160 -cht80p80?

In D48319#1102005, @bz wrote:

rofl nice catch, wanna land that too? :-)

The question is, should -vht also clear -vht40 -vht80 -vht160 -cht80p80?

It should just disable vht entirely, but it shouldn't need to clear ALL the flags.
The net80211 code should be checking if vht is enabled before checking the channel bandwidths avaliable.

I'm /pretty/ sure setting -ht doesn't also clear -ht40 greenfield, etc, it just disables being able to negotiate HT to begin with.

In D48319#1102005, @bz wrote:

rofl nice catch, wanna land that too? :-)

The question is, should -vht also clear -vht40 -vht80 -vht160 -cht80p80?

It should just disable vht entirely, but it shouldn't need to clear ALL the flags.
The net80211 code should be checking if vht is enabled before checking the channel bandwidths avaliable.

I'm /pretty/ sure setting -ht doesn't also clear -ht40 greenfield, etc, it just disables being able to negotiate HT to begin with.

Well, tricky answer given we do not show [-]ht40 in ifconfig -v; but ifconfig -ht40 does a -ht I thinkn all the 20/40/ht is together if you look at the 6 lines at
https://cgit.freebsd.org/src/tree/sbin/ifconfig/ifieee80211.c#n6027
That's things I never quite understood why they are the way they are. And that's why I am putting so much emphasis on "cleanup" needs to be done and with you being around helps a lot as we can figure out what it should have been...

hm, look at that, those numbers are hilarious.

IEEE80211_IOC_HTCONF uses numbers, not flag bits, because of course why not.

For ioctl get:

  • if FHT_HT is 0, then it returns 0, regardless of FHT_USEHT40
  • if FHT_HT is set, it will set 1 for FHT_HT, and |= 2 for USEHT40

for ioctl set:

  • it treats bit 0 as FHT_HT, and bit 1 as FHT_USEHT40

So its PRINTING and SETTING in the net80211 ioctl code are different. That's pretty funny.

I'm sure it's going to bite us if we do what 11n does - ie, "+vht" enables everything appropriate for the interface, and "-vht" disables it all.

As long as -vht clears FHT_VHT and if that's not set, we just don't return / print the other vht flags, we'll be OK. having it disable and re-enable ALL of the flags is going to get very old very quickly during testing. :-)

So, yeah, let's land this diff, then go fix +vht / -vht to clear/set FVHT_VHT and verify that does the right thing, without having to set/clear all the flags.

I think a good mini project for someone is to turn the HTCONF ioctl bits to actually use defines, not magic numbers. :-)

So, yeah, let's land this diff, then go fix +vht / -vht to clear/set FVHT_VHT and verify that does the right thing, without having to set/clear all the flags.

I think a good mini project for someone is to turn the HTCONF ioctl bits to actually use defines, not magic numbers. :-)

ACK; I'll put the -vht patch up for review -- likely tomorrow at some point as it is 1AM -- and then also look at your reviews.