[PATCH] cfg80211: fix WEXT ioctl GIWFREQ for monitor interfaces

[PATCH] cfg80211: fix WEXT ioctl GIWFREQ for monitor interfaces

[David Gnedt]

The cfg80211_set_freq function now also saves the channel for monitor
interfaces.
This fixes a issue were the WEXT ioctl GIWFREQ doesn't report the correct
channel when the channel was switched in monitor mode.

Signed-off-by: David Gnedt <david.gnedt@davizone.at>
---
 net/wireless/chan.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index d0c92dd..fd05938 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -51,9 +51,6 @@ int cfg80211_set_freq(struct cfg80211_registered_device *rdev,
 	struct ieee80211_channel *chan;
 	int result;

-	if (wdev && wdev->iftype == NL80211_IFTYPE_MONITOR)
-		wdev = NULL;
-
 	if (wdev) {
 		ASSERT_WDEV_LOCK(wdev);

@@ -69,7 +66,9 @@ int cfg80211_set_freq(struct cfg80211_registered_device *rdev,
 		return -EINVAL;

 	result = rdev->ops->set_channel(&rdev->wiphy,
-					wdev ? wdev->netdev : NULL,
+					wdev && wdev->iftype !=
+						NL80211_IFTYPE_MONITOR ?
+							wdev->netdev : NULL,
 					chan, channel_type);
 	if (result)
 		return result;
-- 
1.6.3.3

Re: [PATCH] cfg80211: fix WEXT ioctl GIWFREQ for monitor interfaces

[Gábor Stefanik]

On Mon, Jul 19, 2010 at 8:41 PM, David Gnedt <david.gnedt@davizone.at> wrote:
> The cfg80211_set_freq function now also saves the channel for monitor
> interfaces.
> This fixes a issue were the WEXT ioctl GIWFREQ doesn't report the correct
> channel when the channel was switched in monitor mode.
>
> Signed-off-by: David Gnedt <david.gnedt@davizone.at>
> ---
>  net/wireless/chan.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/net/wireless/chan.c b/net/wireless/chan.c
> index d0c92dd..fd05938 100644
> --- a/net/wireless/chan.c
> +++ b/net/wireless/chan.c
> @@ -51,9 +51,6 @@ int cfg80211_set_freq(struct cfg80211_registered_device *rdev,
>        struct ieee80211_channel *chan;
>        int result;
>
> -       if (wdev && wdev->iftype == NL80211_IFTYPE_MONITOR)
> -               wdev = NULL;
> -
>        if (wdev) {
>                ASSERT_WDEV_LOCK(wdev);
>
> @@ -69,7 +66,9 @@ int cfg80211_set_freq(struct cfg80211_registered_device *rdev,
>                return -EINVAL;
>
>        result = rdev->ops->set_channel(&rdev->wiphy,
> -                                       wdev ? wdev->netdev : NULL,
> +                                       wdev && wdev->iftype !=
> +                                               NL80211_IFTYPE_MONITOR ?
> +                                                       wdev->netdev : NULL,

Just out of curiosity: why is it important to pass NULL as netdev for
monitor interfaces?

>                                        chan, channel_type);
>        if (result)
>                return result;
> --
> 1.6.3.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)

Re: [PATCH] cfg80211: fix WEXT ioctl GIWFREQ for monitor interfaces

[Johannes Berg]

On Mon, 2010-07-19 at 20:41 +0200, David Gnedt wrote:
> The cfg80211_set_freq function now also saves the channel for monitor
> interfaces.
> This fixes a issue were the WEXT ioctl GIWFREQ doesn't report the correct
> channel when the channel was switched in monitor mode.

This is still wrong, as discussed previously on this list.

johannes

Re: [PATCH] cfg80211: fix WEXT ioctl GIWFREQ for monitor interfaces

[Gábor Stefanik]

On Mon, Jul 19, 2010 at 10:27 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Mon, 2010-07-19 at 20:41 +0200, David Gnedt wrote:
>> The cfg80211_set_freq function now also saves the channel for monitor
>> interfaces.
>> This fixes a issue were the WEXT ioctl GIWFREQ doesn't report the correct
>> channel when the channel was switched in monitor mode.
>
> This is still wrong, as discussed previously on this list.
>
> johannes
>

And the conclusion was... ?


(BTW, I say that a GIWFREQ on a monitor interface should always return
the channel the PHY is tuned to at the moment when it is issued. Most
tools seem to expect this behavior.)

Re: [PATCH] cfg80211: fix WEXT ioctl GIWFREQ for monitor interfaces

[David Gnedt]

Am 2010-07-19 23:06, schrieb Gábor Stefanik:
> (BTW, I say that a GIWFREQ on a monitor interface should always return
> the channel the PHY is tuned to at the moment when it is issued. Most
> tools seem to expect this behavior.)

I agree, that would be the expected behaviour.

I am not very familar with the entire wireless subsystem yet, but wouldn't that
imply a interface change in cfg80211 and mac80211 to add an "get_channel" function?

Because if the card is hopping channels (e.g. because of 2 station interfaces on
different channels), only the driver itself can tell what's really the current
channel.

Nevertheless a default implementation for this new "get_channel" can be written
at mac80211 level (or even cfg80211?), which tries to find the current channel
by looking at all virtual interfaces, so only mac80211 drivers which allow
multiple channels (and non-mac80211 drivers) need to implement it.

Regards,
David

Re: [PATCH] cfg80211: fix WEXT ioctl GIWFREQ for monitor interfaces

[Johannes Berg]

On Tue, 2010-07-20 at 01:28 +0200, David Gnedt wrote:
> Am 2010-07-19 23:06, schrieb Gábor Stefanik:
> > (BTW, I say that a GIWFREQ on a monitor interface should always return
> > the channel the PHY is tuned to at the moment when it is issued. Most
> > tools seem to expect this behavior.)
> 
> I agree, that would be the expected behaviour.
> 
> I am not very familar with the entire wireless subsystem yet, but wouldn't that
> imply a interface change in cfg80211 and mac80211 to add an "get_channel" function?

Yes, I think so.

> Because if the card is hopping channels (e.g. because of 2 station interfaces on
> different channels), only the driver itself can tell what's really the current
> channel.

Right. Although in that case I'm not sure we should be telling userspace
what channel the monitor interface is on, since there's no single
channel it is on, and I certainly hope userspace won't be requesting the
channel many times per second!

> Nevertheless a default implementation for this new "get_channel" can be written
> at mac80211 level (or even cfg80211?), which tries to find the current channel
> by looking at all virtual interfaces, so only mac80211 drivers which allow
> multiple channels (and non-mac80211 drivers) need to implement it.

Indeed, but I think mac80211 would be more appropriate than cfg80211
since the latter won't really have all the information unless it makes a
whole bunch of assumptions that we'll eventually have to reconsider.

johannes

Re: [PATCH] cfg80211: fix WEXT ioctl GIWFREQ for monitor interfaces

[Gábor Stefanik]

On Tue, Jul 20, 2010 at 3:04 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> On Tue, 2010-07-20 at 01:28 +0200, David Gnedt wrote:
>> Am 2010-07-19 23:06, schrieb Gábor Stefanik:
>> > (BTW, I say that a GIWFREQ on a monitor interface should always return
>> > the channel the PHY is tuned to at the moment when it is issued. Most
>> > tools seem to expect this behavior.)
>>
>> I agree, that would be the expected behaviour.
>>
>> I am not very familar with the entire wireless subsystem yet, but wouldn't that
>> imply a interface change in cfg80211 and mac80211 to add an "get_channel" function?
>
> Yes, I think so.
>
>> Because if the card is hopping channels (e.g. because of 2 station interfaces on
>> different channels), only the driver itself can tell what's really the current
>> channel.
>
> Right. Although in that case I'm not sure we should be telling userspace
> what channel the monitor interface is on, since there's no single
> channel it is on, and I certainly hope userspace won't be requesting the
> channel many times per second!

Well, there is no reason why channel-hopping due to multiple virtual
PHYs and channel-hopping by userspace control (see Kismet) should
behave differently GIWFREQ-wise. Also, userspace (apart from maybe
hostapd - I haven't looked into that at all) seems to expect GIWFREQ
on a monitor interface to unconditionally return the channel the radio
is tuned to at the moment.

>
>> Nevertheless a default implementation for this new "get_channel" can be written
>> at mac80211 level (or even cfg80211?), which tries to find the current channel
>> by looking at all virtual interfaces, so only mac80211 drivers which allow
>> multiple channels (and non-mac80211 drivers) need to implement it.
>
> Indeed, but I think mac80211 would be more appropriate than cfg80211
> since the latter won't really have all the information unless it makes a
> whole bunch of assumptions that we'll eventually have to reconsider.
>
> johannes
>
>



-- 
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)