| Message ID | 20190701130504.27413-1-sven@narfation.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2] nl80211: Don't force VHT channel definition with HE | expand |
On 01/07/2019 15:05, Sven Eckelmann wrote: > From: Sven Eckelmann <seckelmann@datto.com> > > HE (802.11ax) is also supported on 2.4GHz. And the 2.4GHz band isn't > supposed to use VHT opers. Some codepaths in wpa_supplicant will therefore > not initialize the freq->bandwidth or the freq->center_freq1/2 members. As > a result, the nl80211_put_freq_params will directly return an error (-1) or > the kernel will return an error due to the invalid channel definition. > > Instead, the channel definitions should be created based on the actual > HT/VHT/none information. > > Fixes: ad9a1bfe788e ("nl80211: Share VHT channel configuration for HE") > Signed-off-by: Sven Eckelmann <seckelmann@datto.com> > --- > v2: Fixed Signed-off-by: > > Cc: John Crispin <john@phrozen.org> > > @John, please inform me when there is some kind of dependency to > NL80211_ATTR_CHANNEL_WIDTH (in contrast to NL80211_ATTR_WIPHY_CHANNEL_TYPE) > which is required to get HE working. Or if I missed some other behavior > which requires VHT and HE to use the same codepath here. > > src/drivers/driver_nl80211.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c > index 45835a21b..1c445fae9 100644 > --- a/src/drivers/driver_nl80211.c > +++ b/src/drivers/driver_nl80211.c > @@ -4352,7 +4352,7 @@ static int nl80211_put_freq_params(struct nl_msg *msg, > wpa_printf(MSG_DEBUG, " * vht_enabled=%d", freq->vht_enabled); > wpa_printf(MSG_DEBUG, " * ht_enabled=%d", freq->ht_enabled); > > - if (freq->vht_enabled || freq->he_enabled) { > + if (freq->vht_enabled) { > enum nl80211_chan_width cw; > > wpa_printf(MSG_DEBUG, " * bandwidth=%d", freq->bandwidth); this will make HE mode not work. would simply adding a check for !2.4ghz not be better ? John
On 07/01/2019 06:15 AM, John Crispin wrote: > > On 01/07/2019 15:05, Sven Eckelmann wrote: >> From: Sven Eckelmann <seckelmann@datto.com> >> >> HE (802.11ax) is also supported on 2.4GHz. And the 2.4GHz band isn't >> supposed to use VHT opers. Some codepaths in wpa_supplicant will therefore >> not initialize the freq->bandwidth or the freq->center_freq1/2 members. As >> a result, the nl80211_put_freq_params will directly return an error (-1) or >> the kernel will return an error due to the invalid channel definition. >> >> Instead, the channel definitions should be created based on the actual >> HT/VHT/none information. >> >> Fixes: ad9a1bfe788e ("nl80211: Share VHT channel configuration for HE") >> Signed-off-by: Sven Eckelmann <seckelmann@datto.com> >> --- >> v2: Fixed Signed-off-by: >> >> Cc: John Crispin <john@phrozen.org> >> >> @John, please inform me when there is some kind of dependency to >> NL80211_ATTR_CHANNEL_WIDTH (in contrast to NL80211_ATTR_WIPHY_CHANNEL_TYPE) >> which is required to get HE working. Or if I missed some other behavior >> which requires VHT and HE to use the same codepath here. >> >> src/drivers/driver_nl80211.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c >> index 45835a21b..1c445fae9 100644 >> --- a/src/drivers/driver_nl80211.c >> +++ b/src/drivers/driver_nl80211.c >> @@ -4352,7 +4352,7 @@ static int nl80211_put_freq_params(struct nl_msg *msg, >> wpa_printf(MSG_DEBUG, " * vht_enabled=%d", freq->vht_enabled); >> wpa_printf(MSG_DEBUG, " * ht_enabled=%d", freq->ht_enabled); >> - if (freq->vht_enabled || freq->he_enabled) { >> + if (freq->vht_enabled) { >> enum nl80211_chan_width cw; >> wpa_printf(MSG_DEBUG, " * bandwidth=%d", freq->bandwidth); > > this will make HE mode not work. would simply adding a check for !2.4ghz not be better ? > > John /AC *can* work fine on 2.4Ghz, with ath10k and a small tweak to the kernel to revert the patch that disabled /AC on 2.4Ghz. So, please let supplicant still do /AC on 2.4 if the user configures it to do so. Thanks, Ben
On 01/07/2019 15:48, Ben Greear wrote: > > > On 07/01/2019 06:15 AM, John Crispin wrote: >> >> On 01/07/2019 15:05, Sven Eckelmann wrote: >>> From: Sven Eckelmann <seckelmann@datto.com> >>> >>> HE (802.11ax) is also supported on 2.4GHz. And the 2.4GHz band isn't >>> supposed to use VHT opers. Some codepaths in wpa_supplicant will >>> therefore >>> not initialize the freq->bandwidth or the freq->center_freq1/2 >>> members. As >>> a result, the nl80211_put_freq_params will directly return an error >>> (-1) or >>> the kernel will return an error due to the invalid channel definition. >>> >>> Instead, the channel definitions should be created based on the actual >>> HT/VHT/none information. >>> >>> Fixes: ad9a1bfe788e ("nl80211: Share VHT channel configuration for HE") >>> Signed-off-by: Sven Eckelmann <seckelmann@datto.com> >>> --- >>> v2: Fixed Signed-off-by: >>> >>> Cc: John Crispin <john@phrozen.org> >>> >>> @John, please inform me when there is some kind of dependency to >>> NL80211_ATTR_CHANNEL_WIDTH (in contrast to >>> NL80211_ATTR_WIPHY_CHANNEL_TYPE) >>> which is required to get HE working. Or if I missed some other behavior >>> which requires VHT and HE to use the same codepath here. >>> >>> src/drivers/driver_nl80211.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/src/drivers/driver_nl80211.c >>> b/src/drivers/driver_nl80211.c >>> index 45835a21b..1c445fae9 100644 >>> --- a/src/drivers/driver_nl80211.c >>> +++ b/src/drivers/driver_nl80211.c >>> @@ -4352,7 +4352,7 @@ static int nl80211_put_freq_params(struct >>> nl_msg *msg, >>> wpa_printf(MSG_DEBUG, " * vht_enabled=%d", freq->vht_enabled); >>> wpa_printf(MSG_DEBUG, " * ht_enabled=%d", freq->ht_enabled); >>> - if (freq->vht_enabled || freq->he_enabled) { >>> + if (freq->vht_enabled) { >>> enum nl80211_chan_width cw; >>> wpa_printf(MSG_DEBUG, " * bandwidth=%d", freq->bandwidth); >> >> this will make HE mode not work. would simply adding a check for >> !2.4ghz not be better ? >> >> John > > /AC *can* work fine on 2.4Ghz, with ath10k and a small tweak to the > kernel to revert > the patch that disabled /AC on 2.4Ghz. So, please let supplicant > still do /AC on > 2.4 if the user configures it to do so. > > Thanks, > Ben > which, if you look at Svens V3 patch, is what the patch does. John
diff --git a/src/drivers/driver_nl80211.c b/src/drivers/driver_nl80211.c index 45835a21b..1c445fae9 100644 --- a/src/drivers/driver_nl80211.c +++ b/src/drivers/driver_nl80211.c @@ -4352,7 +4352,7 @@ static int nl80211_put_freq_params(struct nl_msg *msg, wpa_printf(MSG_DEBUG, " * vht_enabled=%d", freq->vht_enabled); wpa_printf(MSG_DEBUG, " * ht_enabled=%d", freq->ht_enabled); - if (freq->vht_enabled || freq->he_enabled) { + if (freq->vht_enabled) { enum nl80211_chan_width cw; wpa_printf(MSG_DEBUG, " * bandwidth=%d", freq->bandwidth);