Re: SIOCSIWFREQ while in NL80211_IFTYPE_STATION

Re: SIOCSIWFREQ while in NL80211_IFTYPE_STATION

[Johannes Berg]

+linux-wireless, where this should've gone

> I am running a single wlan0 interface in managed mode (no aliases,
> no  other wireless interfaces).
> The association with the AP still hasn't happened.
> 
> I noticed that if trying to change the frequency to one of the valid 
> values, the driver returns EBUSY.
> 
> The call stack is
> cfg80211_wext_siwfreq
> -->cfg80211_mgd_wext_siwfreq
> --->cfg80211_set_monitor_channel (notice call to set 'monitor'
> channel 
> in managed mode)
> ----> fails with EBUSY
> 
> Is therefore the expected behavior to fail under the above
> circumstances 
> (managed mode && single wlan0 interface && no association)?
> And if it is, please could you clarify when would it be valid to
> change the frequency in managed mode?

Frankly, I don't remember - all of this is plastered all over with
backward compatibility hooks etc.

How are you running into this? Why are you even trying to do this? You
really shouldn't use wireless extensions any more.

Also, there shouldn't be much reason to be setting the channel anyway,
unless you want to trigger a connection specifically on that channel,
but then when you use nl80211 you get that included in the CONNECT
command there.

Finally, I suspect that this particular backward compatibility hook
can't really work anyway and could be removed, but I'm not sure that
would have the effect you want either.

johannes

Re: SIOCSIWFREQ while in NL80211_IFTYPE_STATION

[Jorge Ramirez]

On 01/05/2017 12:38 PM, Johannes Berg wrote:
> +linux-wireless, where this should've gone

nice. thanks and apologies.

>
>> I am running a single wlan0 interface in managed mode (no aliases,
>> no  other wireless interfaces).
>> The association with the AP still hasn't happened.
>>
>> I noticed that if trying to change the frequency to one of the valid
>> values, the driver returns EBUSY.
>>
>> The call stack is
>> cfg80211_wext_siwfreq
>> -->cfg80211_mgd_wext_siwfreq
>> --->cfg80211_set_monitor_channel (notice call to set 'monitor'
>> channel
>> in managed mode)
>> ----> fails with EBUSY
>>
>> Is therefore the expected behavior to fail under the above
>> circumstances
>> (managed mode && single wlan0 interface && no association)?
>> And if it is, please could you clarify when would it be valid to
>> change the frequency in managed mode?
> Frankly, I don't remember - all of this is plastered all over with
> backward compatibility hooks etc.

yes, that is also what it seems to me: plenty of sedimentation in that 
code path.

>
> How are you running into this? Why are you even trying to do this? You
> really shouldn't use wireless extensions any more.

I am not sure it matters - if I understood your reply, there is no valid 
use case to change the frequency in that mode (and all that code should 
be removed); it seems to me that it is also your view that userspace 
(iwconfig) should be fixed accordingly to not call the extensions?

please let me know if I misunderstood your answer.

I am just trying to understand how the current code is supposed to work 
by exercising widely available user-space tools while debugging the kernel.

I noticed the error (EBUSY) just using the wireless tools package as per 
its man pages.
$ iwconfig wlan0 freq <frequency>

So, what other tool should we use instead of iwconfig (if the use case 
is valid)?

>
> Also, there shouldn't be much reason to be setting the channel anyway,
> unless you want to trigger a connection specifically on that channel,
> but then when you use nl80211 you get that included in the CONNECT
> command there.

> Finally, I suspect that this particular backward compatibility hook
> can't really work anyway and could be removed, but I'm not sure that
> would have the effect you want either.

Actually the frequency gets programmed without errors when reverting 
your commit http://tinyurl.com/ho4urp8 (which comes as a surprise as well).
It seems there is some more users doing the same for research purposes.

https://github.com/seemoo-lab/bcm-rpi3/blob/master/kernel/net/wireless/chan.c#L855



>
> johannes

Re: SIOCSIWFREQ while in NL80211_IFTYPE_STATION

[Johannes Berg]

> I am not sure it matters - if I understood your reply, there is no
> valid use case to change the frequency in that mode (and all that
> code should be removed); 

All of wext *is* being removed - slowly :)
It's not longer default in the kernel configuration now.

IIRC, there actually was a valid use case here - to set the frequency
before you set the SSID - to be able to force a connection on that
channel with wext.

> it seems to me that it is also your view that userspace (iwconfig)
> should be fixed accordingly to not call the extensions?

iwconfig should just be deleted and iw be used :-)

> I am just trying to understand how the current code is supposed to
> work  by exercising widely available user-space tools while debugging
> the kernel.

Heh, ok.

> Actually the frequency gets programmed without errors when reverting 
> your commit http://tinyurl.com/ho4urp8 (which comes as a surprise as
> well).

Well, it's not my commit, but that makes sense. I suppose we should
treat this as a regression, but reverting that doesn't seem like the
right fix. Actually, I'm not convinced we should do monitor channel
change anyway when you set the frequency with wext, so we can probably
just remove that call there - want to send a patch to that effect?

johannes

Re: SIOCSIWFREQ while in NL80211_IFTYPE_STATION

[Jorge Ramirez]

On 01/05/2017 03:06 PM, Johannes Berg wrote:
>> I am not sure it matters - if I understood your reply, there is no
>> valid use case to change the frequency in that mode (and all that
>> code should be removed);
> All of wext *is* being removed - slowly :)
> It's not longer default in the kernel configuration now.

ah thanks for the info. I should have checked!

>
> IIRC, there actually was a valid use case here - to set the frequency
> before you set the SSID - to be able to force a connection on that
> channel with wext.

ok, yes I thought this would make sense.

>
>> it seems to me that it is also your view that userspace (iwconfig)
>> should be fixed accordingly to not call the extensions?
> iwconfig should just be deleted and iw be used :-)

understood.

>
>> I am just trying to understand how the current code is supposed to
>> work  by exercising widely available user-space tools while debugging
>> the kernel.
> Heh, ok.
>
>> Actually the frequency gets programmed without errors when reverting
>> your commit http://tinyurl.com/ho4urp8 (which comes as a surprise as
>> well).
> Well, it's not my commit, but that makes sense. I suppose we should
> treat this as a regression, but reverting that doesn't seem like the
> right fix. Actually, I'm not convinced we should do monitor channel
> change anyway when you set the frequency with wext, so we can probably
> just remove that call there - want to send a patch to that effect?

do you mean this?

[jramirez@igloo ~ (debian-qcom-dragonboard410c-16.09-local $)]$ git diff
diff --git a/net/wireless/wext-sme.c b/net/wireless/wext-sme.c
index a4e8af3..c56bac5 100644
--- a/net/wireless/wext-sme.c
+++ b/net/wireless/wext-sme.c
@@ -106,30 +106,7 @@ int cfg80211_mgd_wext_siwfreq(struct net_device *dev,
                         goto out;
         }

-
         wdev->wext.connect.channel = chan;
-
-   /*
-    * SSID is not set, we just want to switch monitor channel,
-    * this is really just backward compatibility, if the SSID
-    * is set then we use the channel to select the BSS to use
-    * to connect to instead. If we were connected on another
-    * channel we disconnected above and reconnect below.
-    */
-   if (chan && !wdev->wext.connect.ssid_len) {
-           struct cfg80211_chan_def chandef = {
-                   .width = NL80211_CHAN_WIDTH_20_NOHT,
-                   .center_freq1 = freq,
-           };
-
-           chandef.chan = ieee80211_get_channel(&rdev->wiphy, freq);
-           if (chandef.chan)
-                   err = cfg80211_set_monitor_channel(rdev, &chandef);
-           else
-                   err = -EINVAL;
-           goto out;
-   }
-
         err = cfg80211_mgd_wext_connect(rdev, wdev);
   out:
         wdev_unlock(wdev);



I tested the change above: we can now modify the channel/frequency when 
the SSID is not set in managed mode.
When the SSID is set however iwconfig does not report any error but 
channel/frequency doesn't change.

if you think this is acceptable I can submit a patch


> johannes

Re: SIOCSIWFREQ while in NL80211_IFTYPE_STATION

[Johannes Berg]

On Thu, 2017-01-05 at 15:38 +0100, Jorge Ramirez wrote:

> do you mean this?
> 
> [jramirez@igloo ~ (debian-qcom-dragonboard410c-16.09-local $)]$ git
> diff
> diff --git a/net/wireless/wext-sme.c b/net/wireless/wext-sme.c
> index a4e8af3..c56bac5 100644
> --- a/net/wireless/wext-sme.c
> +++ b/net/wireless/wext-sme.c
> @@ -106,30 +106,7 @@ int cfg80211_mgd_wext_siwfreq(struct net_device
> *dev,
>                          goto out;
>          }
> 
> -
>          wdev->wext.connect.channel = chan;
> -
> -   /*
> -    * SSID is not set, we just want to switch monitor channel,
> -    * this is really just backward compatibility, if the SSID
> -    * is set then we use the channel to select the BSS to use
> -    * to connect to instead. If we were connected on another
> -    * channel we disconnected above and reconnect below.
> -    */
> -   if (chan && !wdev->wext.connect.ssid_len) {
> -           struct cfg80211_chan_def chandef = {
> -                   .width = NL80211_CHAN_WIDTH_20_NOHT,
> -                   .center_freq1 = freq,
> -           };
> -
> -           chandef.chan = ieee80211_get_channel(&rdev->wiphy, freq);
> -           if (chandef.chan)
> -                   err = cfg80211_set_monitor_channel(rdev,
> &chandef);
> -           else
> -                   err = -EINVAL;
> -           goto out;
> -   }
> -
>          err = cfg80211_mgd_wext_connect(rdev, wdev);
>    out:
>          wdev_unlock(wdev);

Yeah. Frankly, I don't even understand that comment anymore - if the
interface is in managed mode, why set the monitor channel, it's not
monitoring? And if it's not in managed mode we don't get here.

> 
> 
> I tested the change above: we can now modify the channel/frequency
> when 
> the SSID is not set in managed mode.
> When the SSID is set however iwconfig does not report any error but 
> channel/frequency doesn't change.
> 
> if you think this is acceptable I can submit a patch

I think it looks fine, though writing the commit message may be tricky
:)

johannes