[RFC][PATCH] cfg80211: report monitor interface channel via wext when possible

[RFC][PATCH] cfg80211: report monitor interface channel via wext when possible

[Paul Fertser]

This makes it possible to retrieve the channel for the monitor interface
in cases when it can be determined unambigously. Tested with ath5k.

Signed-off-by: Paul Fertser <fercerpav@gmail.com>
---
 net/wireless/chan.c        |    5 +++--
 net/wireless/wext-compat.c |   12 ++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 17cd0c0..869e764 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -81,6 +81,7 @@ int cfg80211_set_freq(struct cfg80211_registered_device *rdev,
 		      enum nl80211_channel_type channel_type)
 {
 	struct ieee80211_channel *chan;
+	struct wireless_dev *passed_wdev = wdev;
 	int result;
 
 	if (wdev && wdev->iftype == NL80211_IFTYPE_MONITOR)
@@ -128,8 +129,8 @@ int cfg80211_set_freq(struct cfg80211_registered_device *rdev,
 	if (result)
 		return result;
 
-	if (wdev)
-		wdev->channel = chan;
+	if (passed_wdev)
+		passed_wdev->channel = chan;
 
 	return 0;
 }
diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index 3e5dbd4..aa637e6 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -819,6 +819,8 @@ int cfg80211_wext_giwfreq(struct net_device *dev,
 			  struct iw_freq *freq, char *extra)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
+	int result;
 
 	switch (wdev->iftype) {
 	case NL80211_IFTYPE_STATION:
@@ -828,6 +830,16 @@ int cfg80211_wext_giwfreq(struct net_device *dev,
 	default:
 		if (!wdev->channel)
 			return -EINVAL;
+		/* The actual working channel might have been changed, verify it
+		 * by re-setting pretending we want to set channel for a monitor
+		 * interface */
+		result = rdev->ops->set_channel(&rdev->wiphy, NULL,
+						wdev->channel,
+						NL80211_CHAN_NO_HT);
+		if (result) {
+			wdev->channel = NULL;
+			return -EINVAL;
+		}
 		freq->m = wdev->channel->center_freq;
 		freq->e = 6;
 		return 0;
-- 
1.7.2.2

Re: [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible

[Johannes Berg]

On Sat, 2011-01-22 at 03:11 +0300, Paul Fertser wrote:
> This makes it possible to retrieve the channel for the monitor interface
> in cases when it can be determined unambigously. Tested with ath5k.

NAK.

http://article.gmane.org/gmane.linux.kernel.wireless.general/61860

johannes

Re: [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible

[Paul Fertser]

On Sun, Jan 23, 2011 at 10:06:39AM +0100, Johannes Berg wrote:
> On Sat, 2011-01-22 at 03:11 +0300, Paul Fertser wrote:
> > This makes it possible to retrieve the channel for the monitor interface
> > in cases when it can be determined unambigously. Tested with ath5k.
> 
> NAK.
> 
> http://article.gmane.org/gmane.linux.kernel.wireless.general/61860

If i understood your correctly, "somehow query oper_channel" means one
needs to extend cfg80211 API a little and add necessary code to
mac80211. And the result would be higher-level API depending on
mac80211 implementation details (which are going to be soon changed
anyway after introduction of the real multi-channel feature).

What i propose should work with any cfg80211 driver where
set_channel() is properly implemented for monitor interfaces, and
mac80211 is certainly one of them. When in the first place one
requests set_channel() for a monitor that's not compatible with the
current configuration, it'll return an error, and so wdev->channel
would not be set at all. If it succeeded and then someone later wants
to verify the monitor channel, set_channel() is still the best
opportunity because it is this function that has all the checks to
verify the channel is still valid.

To sum up, i see no drawbacks in using set_channel() for that, it
doesn't add any implicit requirements, doesn't change semantics,
non-intrusive, doesn't break anything and works correctly every time.

-- 
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav@gmail.com

Re: [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible

[Johannes Berg]

On Sun, 2011-01-23 at 12:41 +0300, Paul Fertser wrote:
> On Sun, Jan 23, 2011 at 10:06:39AM +0100, Johannes Berg wrote:
> > On Sat, 2011-01-22 at 03:11 +0300, Paul Fertser wrote:
> > > This makes it possible to retrieve the channel for the monitor interface
> > > in cases when it can be determined unambigously. Tested with ath5k.
> > 
> > NAK.
> > 
> > http://article.gmane.org/gmane.linux.kernel.wireless.general/61860
> 
> If i understood your correctly, "somehow query oper_channel" means one
> needs to extend cfg80211 API a little and add necessary code to
> mac80211. And the result would be higher-level API depending on
> mac80211 implementation details (which are going to be soon changed
> anyway after introduction of the real multi-channel feature).

Not necessarily -- that's exactly why you'd want such a callback: in the
case that mac80211 does get a multi-channel feature (and the feature is
currently in use) this callback would either return both/all channels
(which probably isn't very useful) or NULL to indicate that it doesn't
have a single fixed channel. In that case, the monitor interface would
still report an unknown channel -- which is really what's happening
anyway since other interfaces will be hopping around.

> What i propose should work with any cfg80211 driver where
> set_channel() is properly implemented for monitor interfaces, and
> mac80211 is certainly one of them. When in the first place one
> requests set_channel() for a monitor that's not compatible with the
> current configuration, it'll return an error, and so wdev->channel
> would not be set at all. If it succeeded and then someone later wants
> to verify the monitor channel, set_channel() is still the best
> opportunity because it is this function that has all the checks to
> verify the channel is still valid.
> 
> To sum up, i see no drawbacks in using set_channel() for that, it
> doesn't add any implicit requirements, doesn't change semantics,
> non-intrusive, doesn't break anything and works correctly every time.

Yes, in some ways this makes sense, but I'm not convinced that
semantically we really want it. It really relies on returning an error
when the given channel cannot be assigned, which I guess we could, but
it just feels wrong. I can't really give a good reason for it other than
that.

However, also consider this case: If you have a monitor and IBSS
interface, the IBSS might -- rather infrequently -- hop channels. In
this case, it might make sense to still return the channel when the
monitor is queried, but you wouldn't be able to set it?

Also, say you did this:
 - add monitor, set to channel 5
 - add IBSS, create network on channel 1
 - observe packets on monitor iface received on channel 1
 - remove IBSS interface
 - query monitor channel -- now suddenly with your approach
   the channel is reset to 5, when it was quite obviously 1
   in the time between

I think that'd be kinda confusing too.

johannes

Re: [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible

[Bruno Randolf]

On Sun January 23 2011 18:06:39 Johannes Berg wrote:
> On Sat, 2011-01-22 at 03:11 +0300, Paul Fertser wrote:
> > This makes it possible to retrieve the channel for the monitor interface
> > in cases when it can be determined unambigously. Tested with ath5k.
> 
> NAK.
> 
> http://article.gmane.org/gmane.linux.kernel.wireless.general/61860

> I don't think so -- I don't recall anyone ever asking before ;-)

I definetly want this functionality for my WLAN monitoring tool ("horst" - 
http://br1.einfach.org/horst). A similar patch has also been discussed for 
aircrack-ng in June 2010 (https://patchwork.kernel.org/patch/103589/).

I understand your point of not wanting to report wrong information, esp when 
we have multiple interfaces on different channels - but in most situations for 
sniffers etc this is not the case, we just have one monitor interface or one 
interface + one monitor interface, in which cases I believe we should be able 
to report a channel.

So just for the record: I want it, pretty please! :)

bruno

Re: [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible

[Paul Fertser]

On Mon, Jan 24, 2011 at 10:10:38AM +0900, Bruno Randolf wrote:
> On Sun January 23 2011 18:06:39 Johannes Berg wrote:
> > On Sat, 2011-01-22 at 03:11 +0300, Paul Fertser wrote:
> > > This makes it possible to retrieve the channel for the monitor interface
> > > in cases when it can be determined unambigously. Tested with ath5k.
> > 
> > NAK.
> > 
> > http://article.gmane.org/gmane.linux.kernel.wireless.general/61860
> 
> > I don't think so -- I don't recall anyone ever asking before ;-)
> 
> I definetly want this functionality for my WLAN monitoring tool ("horst" - 
> http://br1.einfach.org/horst). 

You took the quote out of the context :), Johannes meant that nobody
has asked him how exactly he would like to see this functionality
implemented, and the in the linked mail he outlines his proposal.

Johannes, i know you're a busy man (no kidding, i understand you're
doing serious work) and users should trust maintainers on their
technical decisions, but i hope you can still spare a minute to
explain me why my proposed way to use set_channel is wrong (as i'm yet
to see any drawbacks). Working with real professionals like you is a
great opportunity for me to learn and i appreciate it a lot.

-- 
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav@gmail.com