* [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible
@ 2011-01-22 0:11 Paul Fertser
2011-01-23 9:06 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Paul Fertser @ 2011-01-22 0:11 UTC (permalink / raw)
To: linux-wireless; +Cc: Thomas d'Otreppe, Richard Farina, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible
2011-01-22 0:11 [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible Paul Fertser
@ 2011-01-23 9:06 ` Johannes Berg
2011-01-23 9:41 ` Paul Fertser
2011-01-24 1:10 ` Bruno Randolf
0 siblings, 2 replies; 6+ messages in thread
From: Johannes Berg @ 2011-01-23 9:06 UTC (permalink / raw)
To: Paul Fertser; +Cc: linux-wireless, Thomas d'Otreppe, Richard Farina
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible
2011-01-23 9:06 ` Johannes Berg
@ 2011-01-23 9:41 ` Paul Fertser
2011-01-24 11:47 ` Johannes Berg
2011-01-24 1:10 ` Bruno Randolf
1 sibling, 1 reply; 6+ messages in thread
From: Paul Fertser @ 2011-01-23 9:41 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, Thomas d'Otreppe, Richard Farina
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible
2011-01-23 9:06 ` Johannes Berg
2011-01-23 9:41 ` Paul Fertser
@ 2011-01-24 1:10 ` Bruno Randolf
2011-01-24 7:46 ` Paul Fertser
1 sibling, 1 reply; 6+ messages in thread
From: Bruno Randolf @ 2011-01-24 1:10 UTC (permalink / raw)
To: Johannes Berg
Cc: Paul Fertser, linux-wireless, Thomas d'Otreppe,
Richard Farina, Maxim Levitsky
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible
2011-01-24 1:10 ` Bruno Randolf
@ 2011-01-24 7:46 ` Paul Fertser
0 siblings, 0 replies; 6+ messages in thread
From: Paul Fertser @ 2011-01-24 7:46 UTC (permalink / raw)
To: Bruno Randolf
Cc: Johannes Berg, linux-wireless, Thomas d'Otreppe,
Richard Farina, Maxim Levitsky
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible
2011-01-23 9:41 ` Paul Fertser
@ 2011-01-24 11:47 ` Johannes Berg
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2011-01-24 11:47 UTC (permalink / raw)
To: Paul Fertser; +Cc: linux-wireless, Thomas d'Otreppe, Richard Farina
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-24 11:46 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-01-22 0:11 [RFC][PATCH] cfg80211: report monitor interface channel via wext when possible Paul Fertser
2011-01-23 9:06 ` Johannes Berg
2011-01-23 9:41 ` Paul Fertser
2011-01-24 11:47 ` Johannes Berg
2011-01-24 1:10 ` Bruno Randolf
2011-01-24 7:46 ` Paul Fertser
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).