by Michal Kazior

[permalink] [raw]

Subject: [PATCH 5/7] cfg80211: extend get_channel to take wdev argument

Prepares for moving oper_channel to sdata.

Change-Id: I1d66c4506a463c6b673457127a4bd595fa6018ce
Signed-off-by: Michal Kazior <[email protected]>
---
include/net/cfg80211.h | 3 ++-
net/mac80211/cfg.c | 1 +
net/wireless/nl80211.c | 7 ++++++-
net/wireless/wext-compat.c | 2 +-
4 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index d17ad5f..55b3c4b 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1711,7 +1711,8 @@ struct cfg80211_ops {
u16 noack_map);

struct ieee80211_channel *(*get_channel)(struct wiphy *wiphy,
- enum nl80211_channel_type *type);
+ struct wireless_dev *wdev,
+ enum nl80211_channel_type *type);
};

/*
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 9fa5044..8b71e90 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2693,6 +2693,7 @@ static int ieee80211_probe_client(struct wiphy *wiphy, struct net_device *dev,

static struct ieee80211_channel *
ieee80211_wiphy_get_channel(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
enum nl80211_channel_type *type)
{
struct ieee80211_local *local = wiphy_priv(wiphy);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index bcf6f70..2854a4f 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1502,10 +1502,15 @@ static int nl80211_send_iface(struct sk_buff *msg, u32 pid, u32 seq, int flags,
(cfg80211_rdev_list_generation << 2));

if (rdev->ops->get_channel) {
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
struct ieee80211_channel *chan;
enum nl80211_channel_type channel_type;

- chan = rdev->ops->get_channel(&rdev->wiphy, &channel_type);
+ wdev_lock(wdev);
+ chan = rdev->ops->get_channel(&rdev->wiphy, wdev,
+ &channel_type);
+ wdev_unlock(wdev);
+
if (chan) {
NLA_PUT_U32(msg, NL80211_ATTR_WIPHY_FREQ,
chan->center_freq);
diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index 6a6181a..d545afe 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -832,7 +832,7 @@ static int cfg80211_wext_giwfreq(struct net_device *dev,
if (!rdev->ops->get_channel)
return -EINVAL;

- chan = rdev->ops->get_channel(wdev->wiphy, &channel_type);
+ chan = rdev->ops->get_channel(wdev->wiphy, wdev, &channel_type);
if (!chan)
return -EINVAL;
freq->m = chan->center_freq;
--
1.7.0.4

2012-05-11 12:11:34

by Johannes Berg

[permalink] [raw]

Subject: Re: [RFC/PATCH] multi-channel preparation work

Hi Michal,

> Here's some work on multi-channel operation. It refactors channel
> switching logic with regard to ieee80211_hw_config and moves
> oper_channel stuff to sdata.

This could be useful, but it's hard to tell really. For example, if you
have a channel context, why would you need to store the channel/channel
type per sdata? Each sdata would just point to the channel context, I
think?

> I plan on doing channel contexts next. Monitor interface stuff probably
> needs to be addressed as well since it's tied tightly with set_channel.

Maybe we need to take a more radical approach in introducing
multi-channel and channel contexts :-)

You've started with very small changes like this patchset, but I can't
get a good feeling for how it'll be in the end with channel contexts.
For example, see the question above.

I think we should think about the end result / design more.

You saw my patches for managing the AP channel differently. I think we
should do the same for mesh, Thomas might actually be working on it but
if not it's pretty easy to do really. There are some questions about how
it would work though that Thomas and I may have to discuss [1].

The reason for all this is to get rid of the set_channel call. It's not
really how things should look like with multi-channel, since there we
need a clear indication of when we start and stop using a certain
channel. With the AP mode changes, we'll start using it in start_ap()
(and obviously stop using it in stop_ap()), which simplifies things.

Felix says WDS is completely broken today, so I don't think I care about
it, we should probably just remove the ability to set the channel on WDS
interfaces. Maybe even force them to be slaved to AP mode interfaces.
Felix, any thoughts? I had a patch to introduce explicit start-wds and
stop-wds operations but there were issues with error handling since I'd
have to call start-wds in NETDEV_UP for compatibility... and that can't
return errors any more [2].

That leaves monitor mode, and setting the channel when there's no
interface at all. Monitor mode is special, but we have a nice way out:
when a driver supports multi-channel it will probably need to use the
new queue scheme. Then, if _injection_ is desired, the driver needs to
implement support for explicit monitor mode interfaces anyway. So I
think we can force multi-channel capable drivers to implement explicit
monitor mode, if they want to support a real monitor mode at all.
Depending on what mesh needs, we may be able to equate "phy channel" to
"monitor mode channel", the cfg80211 callbacks pretty much make that
assumption today but inside cfg80211 it may not be true.

In any case, the result of all of this is that the cfg80211 set_channel
operation is no longer used by anything but monitor mode, we could
rename it to set_monitor_channel. This operation would only be available
when only monitor interfaces, if any interfaces at all, are active.
Internally in mac80211, the channel context for monitor interfaces would
be attached to the explicit monitor interface.

Now, if we have a driver that supports only a single channel and maybe
doesn't even support the explicit monitor interface API, then obviously
we have to translate all this to the explicit channel setting with
hw_config. However, that's not terribly difficult I think? We need to
have a limit on channel contexts anyway, so we don't create too many, so
here the limit is 1 and when we change that context we program the
hardware.

The only other difficult thing is software scanning/remain-on-channel,
i.e. the temporary channel. But this is only supported for drivers that
don't have multi-channel, so we can essentially keep it the same with
the tmp_channel variable, it just overrides the context.

I'm probably glossing over a lot of details, but at least I now have a
clearer picture of how we can do this. :-) I hope it helps explain my
thinking.

johannes

[1] The question is whether or not setting the channel on the *phy* is
needed for mesh, and if it has to support bringing up mesh without
having any channel set at all. These are backward compatibility
considerations.

[2] Alternatively, we could change the userspace API and require that
you set the WDS peer *after* bringing up the interface. Today, you have
to set it *before*. Then setting the peer could actually be the signal
to establish the "link". Given that it all doesn't work, maybe that's an
alternative? I'd have patches for this almost ready.