[RFCv2] respect channels in iface combinations

[RFCv2] respect channels in iface combinations

[Michal Kazior]

Hi,

Here goes an updated patchset with multi-channel work on
cfg80211. Thanks go to Johannes and Eliad for review and
pointing out my mistakes.

I've addressed the issue with non-fixed channel IBSS.
Channels are now cosidered either shared or exclusive. An
exclusive channel always takes a channel resource (when
accouting interface combinations and
num_different_channels). Exclusive channel could also be
called hopping but I thought the first one fits better as it
explicitly states code logic decisions.

I've also refactored and fixed interface combination
checking. There are now functions like
cfg80211_can_use_chan, cfg80211_can_use_iftype_chan.

Monitor channel is now always defined. When there are
only monitors running we set a channel (first we can find in
supported bands structures). This way the corner case
(of a monitor not having a channel and .get_channel
returning NULL) mentioned by Johannes is fixed.

This is based on wireless-next tree.


-- Pozdrawiam / Best regards, Michal Kazior.

[RFCv2 01/13] cfg80211: introduce cfg80211_stop_ap

[Michal Kazior]

This functionality will be reused when interface
is going down. Avoids code duplication. Also adds
missing wdev locking.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/Makefile  |    2 +-
 net/wireless/ap.c      |   44 ++++++++++++++++++++++++++++++++++++++++++++
 net/wireless/core.h    |    4 ++++
 net/wireless/nl80211.c |   17 +----------------
 4 files changed, 50 insertions(+), 17 deletions(-)
 create mode 100644 net/wireless/ap.c

diff --git a/net/wireless/Makefile b/net/wireless/Makefile
index 55a28ab..0f7e0d6 100644
--- a/net/wireless/Makefile
+++ b/net/wireless/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_WEXT_SPY) += wext-spy.o
 obj-$(CONFIG_WEXT_PRIV) += wext-priv.o
 
 cfg80211-y += core.o sysfs.o radiotap.o util.o reg.o scan.o nl80211.o
-cfg80211-y += mlme.o ibss.o sme.o chan.o ethtool.o mesh.o
+cfg80211-y += mlme.o ibss.o sme.o chan.o ethtool.o mesh.o ap.o
 cfg80211-$(CONFIG_CFG80211_DEBUGFS) += debugfs.o
 cfg80211-$(CONFIG_CFG80211_WEXT) += wext-compat.o wext-sme.o
 cfg80211-$(CONFIG_CFG80211_INTERNAL_REGDB) += regdb.o
diff --git a/net/wireless/ap.c b/net/wireless/ap.c
new file mode 100644
index 0000000..45199cc
--- /dev/null
+++ b/net/wireless/ap.c
@@ -0,0 +1,44 @@
+#include <linux/ieee80211.h>
+#include <linux/export.h>
+#include <net/cfg80211.h>
+#include "nl80211.h"
+#include "core.h"
+
+
+static int __cfg80211_stop_ap(struct cfg80211_registered_device *rdev,
+			      struct net_device *dev)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	int err;
+
+	ASSERT_WDEV_LOCK(wdev);
+
+	if (!rdev->ops->stop_ap)
+		return -EOPNOTSUPP;
+
+	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
+	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
+		return -EOPNOTSUPP;
+
+	if (!wdev->beacon_interval)
+		return -ENOENT;
+
+	err = rdev->ops->stop_ap(&rdev->wiphy, dev);
+	if (!err)
+		wdev->beacon_interval = 0;
+
+	return err;
+}
+
+int cfg80211_stop_ap(struct cfg80211_registered_device *rdev,
+		     struct net_device *dev)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	int err;
+
+	wdev_lock(wdev);
+	err = __cfg80211_stop_ap(rdev, dev);
+	wdev_unlock(wdev);
+
+	return err;
+}
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 9348a47..a366ffd 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -315,6 +315,10 @@ int cfg80211_set_mesh_freq(struct cfg80211_registered_device *rdev,
 			   struct wireless_dev *wdev, int freq,
 			   enum nl80211_channel_type channel_type);
 
+/* AP */
+int cfg80211_stop_ap(struct cfg80211_registered_device *rdev,
+		     struct net_device *dev);
+
 /* MLME */
 int __cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
 			 struct net_device *dev,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 7ae54b8..c17b57b 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2390,23 +2390,8 @@ static int nl80211_stop_ap(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
 	struct net_device *dev = info->user_ptr[1];
-	struct wireless_dev *wdev = dev->ieee80211_ptr;
-	int err;
-
-	if (!rdev->ops->stop_ap)
-		return -EOPNOTSUPP;
 
-	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
-	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
-		return -EOPNOTSUPP;
-
-	if (!wdev->beacon_interval)
-		return -ENOENT;
-
-	err = rdev->ops->stop_ap(&rdev->wiphy, dev);
-	if (!err)
-		wdev->beacon_interval = 0;
-	return err;
+	return cfg80211_stop_ap(rdev, dev);
 }
 
 static const struct nla_policy sta_flags_policy[NL80211_STA_FLAG_MAX + 1] = {
-- 
1.7.0.4

[RFCv2 02/13] cfg80211: .stop_ap when interface is going down

[Michal Kazior]

We'll need this for proper channel tracking (which
is going to be needed for channel context
accounting and finding matching/active interface
combination).

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/core.c |    3 +++
 net/wireless/util.c |    3 +++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index a87d435..46a33ea 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -869,6 +869,9 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		case NL80211_IFTYPE_MESH_POINT:
 			cfg80211_leave_mesh(rdev, dev);
 			break;
+		case NL80211_IFTYPE_AP:
+			cfg80211_stop_ap(rdev, dev);
+			break;
 		default:
 			break;
 		}
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 316cfd0..fc948d0 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -814,6 +814,9 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
 		dev->ieee80211_ptr->mesh_id_up_len = 0;
 
 		switch (otype) {
+		case NL80211_IFTYPE_AP:
+			cfg80211_stop_ap(rdev, dev);
+			break;
 		case NL80211_IFTYPE_ADHOC:
 			cfg80211_leave_ibss(rdev, dev, false);
 			break;
-- 
1.7.0.4

[RFCv2 03/13] cfg80211: add channel tracking for AP and mesh

[Michal Kazior]

We need to know which channel is used by a running
AP and mesh for channel context accounting and
finding matching/active interface combination.

STA/IBSS have current_bss already which allows us
to check which channel a vif is tuned to.
Non-fixed channel IBSS can be handled with
additional changes.

Monitor mode is going to be handled differently.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 include/net/cfg80211.h |    3 +++
 net/wireless/ap.c      |    4 +++-
 net/wireless/mesh.c    |    6 +++++-
 net/wireless/nl80211.c |    5 ++++-
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 7319f25..0f9d7b4 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2335,6 +2335,9 @@ struct wireless_dev {
 	struct ieee80211_channel *preset_chan;
 	enum nl80211_channel_type preset_chantype;
 
+	/* for AP and mesh channel tracking */
+	struct ieee80211_channel *channel;
+
 	bool ps;
 	int ps_timeout;
 
diff --git a/net/wireless/ap.c b/net/wireless/ap.c
index 45199cc..fcc60d8 100644
--- a/net/wireless/ap.c
+++ b/net/wireless/ap.c
@@ -24,8 +24,10 @@ static int __cfg80211_stop_ap(struct cfg80211_registered_device *rdev,
 		return -ENOENT;
 
 	err = rdev->ops->stop_ap(&rdev->wiphy, dev);
-	if (!err)
+	if (!err) {
 		wdev->beacon_interval = 0;
+		wdev->channel = NULL;
+	}
 
 	return err;
 }
diff --git a/net/wireless/mesh.c b/net/wireless/mesh.c
index b44c736..c9c38e3 100644
--- a/net/wireless/mesh.c
+++ b/net/wireless/mesh.c
@@ -153,6 +153,7 @@ int __cfg80211_join_mesh(struct cfg80211_registered_device *rdev,
 	if (!err) {
 		memcpy(wdev->ssid, setup->mesh_id, setup->mesh_id_len);
 		wdev->mesh_id_len = setup->mesh_id_len;
+		wdev->channel = setup->channel;
 	}
 
 	return err;
@@ -243,8 +244,11 @@ static int __cfg80211_leave_mesh(struct cfg80211_registered_device *rdev,
 		return -ENOTCONN;
 
 	err = rdev->ops->leave_mesh(&rdev->wiphy, dev);
-	if (!err)
+	if (!err) {
 		wdev->mesh_id_len = 0;
+		wdev->channel = NULL;
+	}
+
 	return err;
 }
 
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index c17b57b..5d36124 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2356,8 +2356,11 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 		return -EINVAL;
 
 	err = rdev->ops->start_ap(&rdev->wiphy, dev, &params);
-	if (!err)
+	if (!err) {
 		wdev->beacon_interval = params.beacon_interval;
+		wdev->channel = params.channel;
+	}
+
 	return err;
 }
 
-- 
1.7.0.4

Re: [RFCv2 03/13] cfg80211: add channel tracking for AP and mesh

[Johannes Berg]

On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
> We need to know which channel is used by a running
> AP and mesh for channel context accounting and
> finding matching/active interface combination.

I think there's the AP channel change function now that full-MAC drivers
can call -- need to update there?

johannes

Re: [RFCv2 03/13] cfg80211: add channel tracking for AP and mesh

[Michal Kazior]

Johannes Berg wrote:
> On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
>> We need to know which channel is used by a running
>> AP and mesh for channel context accounting and
>> finding matching/active interface combination.
>
> I think there's the AP channel change function now that full-MAC drivers
> can call -- need to update there?

I'm not aware of that. I thought it's impossible to change a channel 
while AP is running now. Am I missing something?

But actually there's a bug. I forgot about the libertas set_channel for 
mesh workaround. I'll address that later.


-- 
Pozdrawiam / Best regards, Michal Kazior.

Re: [RFCv2 03/13] cfg80211: add channel tracking for AP and mesh

[Johannes Berg]

On Wed, 2012-06-20 at 11:03 +0200, Michal Kazior wrote:
> Johannes Berg wrote:
> > On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
> >> We need to know which channel is used by a running
> >> AP and mesh for channel context accounting and
> >> finding matching/active interface combination.
> >
> > I think there's the AP channel change function now that full-MAC drivers
> > can call -- need to update there?
> 
> I'm not aware of that. I thought it's impossible to change a channel 
> while AP is running now. Am I missing something?

Yeah ... I think the ath6kl driver (only user?) disassociates all
stations or something ... I don't really remember what they told me
about how it worked, but it should be on the list somewhere in the
discussion(s) about the patches that added the feature, see

commit 5314526b1743e8e8614293db7d86e480b4fe9824
Author: Thomas Pedersen <c_tpeder@qca.qualcomm.com>
Date:   Fri Apr 6 13:35:47 2012 -0700

    cfg80211: add channel switch notify event


johannes

[RFCv2 04/13] cfg80211: track ibss fixed channel

[Michal Kazior]

IBSS may hop between channels. It is necessary to
account this special case when considering
interface combinations.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 include/net/cfg80211.h |    2 ++
 net/wireless/ibss.c    |    1 +
 2 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0f9d7b4..92fdd02 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2338,6 +2338,8 @@ struct wireless_dev {
 	/* for AP and mesh channel tracking */
 	struct ieee80211_channel *channel;
 
+	bool ibss_fixed;
+
 	bool ps;
 	int ps_timeout;
 
diff --git a/net/wireless/ibss.c b/net/wireless/ibss.c
index 89baa33..b90fd86 100644
--- a/net/wireless/ibss.c
+++ b/net/wireless/ibss.c
@@ -113,6 +113,7 @@ int __cfg80211_join_ibss(struct cfg80211_registered_device *rdev,
 		kfree(wdev->connect_keys);
 	wdev->connect_keys = connkeys;
 
+	wdev->ibss_fixed = params->channel_fixed;
 #ifdef CONFIG_CFG80211_WEXT
 	wdev->wext.ibss.channel = params->channel;
 #endif
-- 
1.7.0.4

[RFCv2 05/13] cfg80211: introduce cfg80211_get_chan_state

[Michal Kazior]

Helper function for finding out which channel is
used by a given interface.

An exclusive channel can be used only by a single
interface. This is mainly for non-fixed channel
IBSS handling.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/chan.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
 net/wireless/core.h |   12 ++++++++++++
 2 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index c1999e4..167e7cb 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -92,3 +92,54 @@ int cfg80211_set_monitor_channel(struct cfg80211_registered_device *rdev,
 
 	return rdev->ops->set_monitor_channel(&rdev->wiphy, chan, chantype);
 }
+
+void
+cfg80211_get_chan_state(struct cfg80211_registered_device *rdev,
+		        struct wireless_dev *wdev,
+		        struct ieee80211_channel **chan,
+		        enum cfg80211_chan_mode *chanmode)
+{
+	*chan = NULL;
+	*chanmode = CHAN_MODE_UNDEFINED;
+
+	ASSERT_RDEV_LOCK(rdev);
+	ASSERT_WDEV_LOCK(wdev);
+
+	if (!netif_running(wdev->netdev))
+		return;
+
+	switch (wdev->iftype) {
+	case NL80211_IFTYPE_ADHOC:
+		if (wdev->current_bss) {
+			*chan = wdev->current_bss->pub.channel;
+			*chanmode = wdev->ibss_fixed
+				  ? CHAN_MODE_SHARED
+				  : CHAN_MODE_EXCLUSIVE;
+			return;
+		}
+	case NL80211_IFTYPE_STATION:
+	case NL80211_IFTYPE_P2P_CLIENT:
+		if (wdev->current_bss) {
+			*chan = wdev->current_bss->pub.channel;
+			*chanmode = CHAN_MODE_SHARED;
+			return;
+		}
+		break;
+	case NL80211_IFTYPE_AP:
+	case NL80211_IFTYPE_P2P_GO:
+	case NL80211_IFTYPE_MESH_POINT:
+		*chan = wdev->channel;
+		*chanmode = CHAN_MODE_SHARED;
+		return;
+	case NL80211_IFTYPE_MONITOR:
+	case NL80211_IFTYPE_AP_VLAN:
+	case NL80211_IFTYPE_WDS:
+		/* these interface types don't really have a channel */
+		return;
+	case NL80211_IFTYPE_UNSPECIFIED:
+	case NUM_NL80211_IFTYPES:
+		WARN_ON(1);
+	}
+
+	return;
+}
diff --git a/net/wireless/core.h b/net/wireless/core.h
index a366ffd..bbf2f75 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -267,6 +267,12 @@ struct cfg80211_cached_keys {
 	int def, defmgmt;
 };
 
+enum cfg80211_chan_mode {
+	CHAN_MODE_UNDEFINED,
+	CHAN_MODE_SHARED,
+	CHAN_MODE_EXCLUSIVE,
+};
+
 
 /* free object */
 extern void cfg80211_dev_free(struct cfg80211_registered_device *rdev);
@@ -445,6 +451,12 @@ cfg80211_can_add_interface(struct cfg80211_registered_device *rdev,
 	return cfg80211_can_change_interface(rdev, NULL, iftype);
 }
 
+void
+cfg80211_get_chan_state(struct cfg80211_registered_device *rdev,
+		        struct wireless_dev *wdev,
+		        struct ieee80211_channel **chan,
+		        enum cfg80211_chan_mode *chanmode);
+
 struct ieee80211_channel *
 rdev_freq_to_chan(struct cfg80211_registered_device *rdev,
 		  int freq, enum nl80211_channel_type channel_type);
-- 
1.7.0.4

Re: [RFCv2 05/13] cfg80211: introduce cfg80211_get_chan_state

[Johannes Berg]

On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
> Helper function for finding out which channel is
> used by a given interface.
> 
> An exclusive channel can be used only by a single
> interface. This is mainly for non-fixed channel
> IBSS handling.
> 
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  net/wireless/chan.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  net/wireless/core.h |   12 ++++++++++++
>  2 files changed, 63 insertions(+), 0 deletions(-)
> 
> diff --git a/net/wireless/chan.c b/net/wireless/chan.c
> index c1999e4..167e7cb 100644
> --- a/net/wireless/chan.c
> +++ b/net/wireless/chan.c
> @@ -92,3 +92,54 @@ int cfg80211_set_monitor_channel(struct cfg80211_registered_device *rdev,
>  
>  	return rdev->ops->set_monitor_channel(&rdev->wiphy, chan, chantype);
>  }
> +
> +void
> +cfg80211_get_chan_state(struct cfg80211_registered_device *rdev,
> +		        struct wireless_dev *wdev,
> +		        struct ieee80211_channel **chan,
> +		        enum cfg80211_chan_mode *chanmode)

Why not return the mode rather than void & pointer?

johannes

Re: [RFCv2 05/13] cfg80211: introduce cfg80211_get_chan_state

[Michal Kazior]

Johannes Berg wrote:
> On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
>> Helper function for finding out which channel is
>> used by a given interface.
>>
>> An exclusive channel can be used only by a single
>> interface. This is mainly for non-fixed channel
>> IBSS handling.
>>
>> Signed-off-by: Michal Kazior<michal.kazior@tieto.com>
>> ---
>>   net/wireless/chan.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>   net/wireless/core.h |   12 ++++++++++++
>>   2 files changed, 63 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/wireless/chan.c b/net/wireless/chan.c
>> index c1999e4..167e7cb 100644
>> --- a/net/wireless/chan.c
>> +++ b/net/wireless/chan.c
>> @@ -92,3 +92,54 @@ int cfg80211_set_monitor_channel(struct cfg80211_registered_device *rdev,
>>
>>   	return rdev->ops->set_monitor_channel(&rdev->wiphy, chan, chantype);
>>   }
>> +
>> +void
>> +cfg80211_get_chan_state(struct cfg80211_registered_device *rdev,
>> +		        struct wireless_dev *wdev,
>> +		        struct ieee80211_channel **chan,
>> +		        enum cfg80211_chan_mode *chanmode)
>
> Why not return the mode rather than void&  pointer?

I think returning the mode would be ambiguous - the function name isn't 
"get_chan_mode". chan_mode isn't a result on its own. But then again, 
it's just a matter of preference.


-- 
Pozdrawiam / Best regards, Michal Kazior.

Re: [RFCv2 05/13] cfg80211: introduce cfg80211_get_chan_state

[Johannes Berg]

On Wed, 2012-06-20 at 11:11 +0200, Michal Kazior wrote:

> >> +void
> >> +cfg80211_get_chan_state(struct cfg80211_registered_device *rdev,
> >> +		        struct wireless_dev *wdev,
> >> +		        struct ieee80211_channel **chan,
> >> +		        enum cfg80211_chan_mode *chanmode)
> >
> > Why not return the mode rather than void&  pointer?
> 
> I think returning the mode would be ambiguous - the function name isn't 
> "get_chan_mode". chan_mode isn't a result on its own. But then again, 
> it's just a matter of preference.

No, ok, you're right -- I missed that it was returning both the channel
& the mode.

johannes

[RFCv2 06/13] cfg80211: track monitor interfaces count

[Michal Kazior]

Implements .set_monitor_enabled(wiphy, enabled).

Notifies driver upon change of interface layout.

If only monitor interfaces become present it is
called with 2nd argument being true. If
non-monitor interface appears then 2nd argument
is false. Driver is notified only upon change.

This makes it more obvious about the fact that
cfg80211 supports single monitor channel. Once we
implement multi-channel we don't want to allow
setting monitor channel while other interface
types are running. Otherwise it would be ambiguous
once we start considering num_different_channels.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 include/net/cfg80211.h |    4 ++++
 net/wireless/core.c    |   24 ++++++++++++++++++++++++
 net/wireless/core.h    |   14 ++++++++++++++
 net/wireless/util.c    |    5 +++++
 4 files changed, 47 insertions(+), 0 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 92fdd02..120eae0 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1428,6 +1428,8 @@ struct cfg80211_gtk_rekey_data {
  *	interfaces are active this callback should reject the configuration.
  *	If no interfaces are active or the device is down, the channel should
  *	be stored for when a monitor interface becomes active.
+ * @set_monitor_enabled: Notify driver that there are only monitor
+ *	interfaces running.
  * @get_channel: Get the current operating channel, should return %NULL if
  *	there's no single defined operating channel if for example the
  *	device implements channel hopping for multi-channel virtual interfaces.
@@ -1746,6 +1748,8 @@ struct cfg80211_ops {
 				struct ethtool_stats *stats, u64 *data);
 	void	(*get_et_strings)(struct wiphy *wiphy, struct net_device *dev,
 				  u32 sset, u8 *data);
+
+	void (*set_monitor_enabled)(struct wiphy *wiphy, bool enabled);
 };
 
 /*
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 46a33ea..9097d6c 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -776,6 +776,24 @@ static struct device_type wiphy_type = {
 	.name	= "wlan",
 };
 
+void cfg80211_update_iface_num(struct cfg80211_registered_device *rdev,
+			       enum nl80211_iftype iftype, int num)
+{
+	bool has_monitors_only_old = cfg80211_has_monitors_only(rdev);
+	bool has_monitors_only_new;
+
+	ASSERT_RDEV_LOCK(rdev);
+
+	rdev->num_running_ifaces += num;
+	if (iftype == NL80211_IFTYPE_MONITOR)
+		rdev->num_running_monitor_ifaces += num;
+
+	has_monitors_only_new = cfg80211_has_monitors_only(rdev);
+	if (has_monitors_only_new != has_monitors_only_old)
+		rdev->ops->set_monitor_enabled(&rdev->wiphy,
+					       has_monitors_only_new);
+}
+
 static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 					 unsigned long state,
 					 void *ndev)
@@ -879,6 +897,9 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		break;
 	case NETDEV_DOWN:
 		dev_hold(dev);
+		cfg80211_lock_rdev(rdev);
+		cfg80211_update_iface_num(rdev, wdev->iftype, -1);
+		cfg80211_unlock_rdev(rdev);
 		queue_work(cfg80211_wq, &wdev->cleanup_work);
 		break;
 	case NETDEV_UP:
@@ -986,6 +1007,9 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		ret = cfg80211_can_add_interface(rdev, wdev->iftype);
 		if (ret)
 			return notifier_from_errno(ret);
+		cfg80211_lock_rdev(rdev);
+		cfg80211_update_iface_num(rdev, wdev->iftype, 1);
+		cfg80211_unlock_rdev(rdev);
 		break;
 	}
 
diff --git a/net/wireless/core.h b/net/wireless/core.h
index bbf2f75..b27fa3b 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -56,6 +56,9 @@ struct cfg80211_registered_device {
 
 	u32 ap_beacons_nlpid;
 
+	int num_running_ifaces;
+	int num_running_monitor_ifaces;
+
 	/* BSSes/scanning */
 	spinlock_t bss_lock;
 	struct list_head bss_list;
@@ -223,6 +226,14 @@ static inline void wdev_unlock(struct wireless_dev *wdev)
 #define ASSERT_RDEV_LOCK(rdev) lockdep_assert_held(&(rdev)->mtx)
 #define ASSERT_WDEV_LOCK(wdev) lockdep_assert_held(&(wdev)->mtx)
 
+static inline bool cfg80211_has_monitors_only(struct cfg80211_registered_device *rdev)
+{
+	ASSERT_RDEV_LOCK(rdev);
+
+	return rdev->num_running_ifaces == rdev->num_running_monitor_ifaces &&
+	       rdev->num_running_ifaces > 0;
+}
+
 enum cfg80211_event_type {
 	EVENT_CONNECT_RESULT,
 	EVENT_ROAMED,
@@ -470,6 +481,9 @@ int ieee80211_get_ratemask(struct ieee80211_supported_band *sband,
 int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev,
 				 u32 beacon_int);
 
+void cfg80211_update_iface_num(struct cfg80211_registered_device *rdev,
+			       enum nl80211_iftype iftype, int num);
+
 #ifdef CONFIG_CFG80211_DEVELOPER_WARNINGS
 #define CFG80211_DEV_WARN_ON(cond)	WARN_ON(cond)
 #else
diff --git a/net/wireless/util.c b/net/wireless/util.c
index fc948d0..9b92ec5 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -871,6 +871,11 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
 		}
 	}
 
+	if (!err && ntype != otype && netif_running(dev)) {
+		cfg80211_update_iface_num(rdev, ntype, 1);
+		cfg80211_update_iface_num(rdev, otype, -1);
+	}
+
 	return err;
 }
 
-- 
1.7.0.4

Re: [RFCv2 06/13] cfg80211: track monitor interfaces count

[Johannes Berg]

On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
> Implements .set_monitor_enabled(wiphy, enabled).
> 
> Notifies driver upon change of interface layout.
> 
> If only monitor interfaces become present it is
> called with 2nd argument being true. If
> non-monitor interface appears then 2nd argument
> is false. Driver is notified only upon change.
> 
> This makes it more obvious about the fact that
> cfg80211 supports single monitor channel. Once we
> implement multi-channel we don't want to allow
> setting monitor channel while other interface
> types are running. Otherwise it would be ambiguous
> once we start considering num_different_channels.

This makes sense :)

> + * @set_monitor_enabled: Notify driver that there are only monitor
> + *	interfaces running.

Maybe rename to "set_monitor_only()"? But I don't really care much.

Ok totally makes sense, not sure why I thought yesterday I should take a
closer look.

Btw, I need to commend you on the quality of your code in general, I
like all the locking assertions etc. :-)

johannes

[RFCv2 07/13] mac80211: refactor virtual monitor code

[Michal Kazior]

Use cfg80211 the new .set_monitor_enabled instead
of tracking it inside mac80211.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/cfg.c         |   11 +++++++++++
 net/mac80211/ieee80211_i.h |    4 ++++
 net/mac80211/iface.c       |   16 ++--------------
 3 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 85ac364..df7332e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2943,6 +2943,16 @@ ieee80211_wiphy_get_channel(struct wiphy *wiphy,
 	return local->oper_channel;
 }
 
+static void ieee80211_set_monitor_enabled(struct wiphy *wiphy, bool enabled)
+{
+	struct ieee80211_local *local = wiphy_priv(wiphy);
+
+	if (enabled)
+		WARN_ON(ieee80211_add_virtual_monitor(local));
+	else
+		ieee80211_del_virtual_monitor(local);
+}
+
 #ifdef CONFIG_PM
 static void ieee80211_set_wakeup(struct wiphy *wiphy, bool enabled)
 {
@@ -3018,6 +3028,7 @@ struct cfg80211_ops mac80211_config_ops = {
 	.probe_client = ieee80211_probe_client,
 	.get_channel = ieee80211_wiphy_get_channel,
 	.set_noack_map = ieee80211_set_noack_map,
+	.set_monitor_enabled = ieee80211_set_monitor_enabled,
 #ifdef CONFIG_PM
 	.set_wakeup = ieee80211_set_wakeup,
 #endif
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index e6cbf5b..9eb3b8b 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1472,6 +1472,10 @@ u8 *ieee80211_ie_build_ht_oper(u8 *pos, struct ieee80211_sta_ht_cap *ht_cap,
 			       enum nl80211_channel_type channel_type,
 			       u16 prot_mode);
 
+/* virtual monitor */
+int ieee80211_add_virtual_monitor(struct ieee80211_local *local);
+void ieee80211_del_virtual_monitor(struct ieee80211_local *local);
+
 /* channel management */
 enum ieee80211_chan_mode {
 	CHAN_MODE_UNDEFINED,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 87aeb4f..47f46fd 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -214,7 +214,7 @@ static void ieee80211_set_default_queues(struct ieee80211_sub_if_data *sdata)
 	sdata->vif.cab_queue = IEEE80211_INVAL_HW_QUEUE;
 }
 
-static int ieee80211_add_virtual_monitor(struct ieee80211_local *local)
+int ieee80211_add_virtual_monitor(struct ieee80211_local *local)
 {
 	struct ieee80211_sub_if_data *sdata;
 	int ret;
@@ -255,7 +255,7 @@ static int ieee80211_add_virtual_monitor(struct ieee80211_local *local)
 	return 0;
 }
 
-static void ieee80211_del_virtual_monitor(struct ieee80211_local *local)
+void ieee80211_del_virtual_monitor(struct ieee80211_local *local)
 {
 	struct ieee80211_sub_if_data *sdata;
 
@@ -371,12 +371,6 @@ static int ieee80211_do_open(struct net_device *dev, bool coming_up)
 			break;
 		}
 
-		if (local->monitors == 0 && local->open_count == 0) {
-			res = ieee80211_add_virtual_monitor(local);
-			if (res)
-				goto err_stop;
-		}
-
 		/* must be before the call to ieee80211_configure_filter */
 		local->monitors++;
 		if (local->monitors == 1) {
@@ -391,8 +385,6 @@ static int ieee80211_do_open(struct net_device *dev, bool coming_up)
 		break;
 	default:
 		if (coming_up) {
-			ieee80211_del_virtual_monitor(local);
-
 			res = drv_add_interface(local, sdata);
 			if (res)
 				goto err_stop;
@@ -627,7 +619,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 		if (local->monitors == 0) {
 			local->hw.conf.flags &= ~IEEE80211_CONF_MONITOR;
 			hw_reconf_flags |= IEEE80211_CONF_CHANGE_MONITOR;
-			ieee80211_del_virtual_monitor(local);
 		}
 
 		ieee80211_adjust_monitor_flags(sdata, -1);
@@ -701,9 +692,6 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 		}
 	}
 	spin_unlock_irqrestore(&local->queue_stop_reason_lock, flags);
-
-	if (local->monitors == local->open_count && local->monitors > 0)
-		ieee80211_add_virtual_monitor(local);
 }
 
 static int ieee80211_stop(struct net_device *dev)
-- 
1.7.0.4

[RFCv2 08/13] cfg80211: refuse to .set_monitor_channel when non-monitors are present

[Michal Kazior]

Having .set_monitor_channel work with non-monitor
interfaces running would make interface
combinations accounting ambiguous.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/chan.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 167e7cb..019401b 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -85,6 +85,8 @@ int cfg80211_set_monitor_channel(struct cfg80211_registered_device *rdev,
 
 	if (!rdev->ops->set_monitor_channel)
 		return -EOPNOTSUPP;
+	if (!cfg80211_has_monitors_only(rdev))
+		return -EBUSY;
 
 	chan = rdev_freq_to_chan(rdev, freq, chantype);
 	if (!chan)
-- 
1.7.0.4

[RFCv2 09/13] cfg80211: track monitor channel

[Michal Kazior]

Make it even more obvious we support single
monitor channel. This will allow us to remove
.get_channel.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/chan.c |    9 ++++++++-
 net/wireless/core.c |    8 +++++++-
 net/wireless/core.h |    3 +++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 019401b..434c56b 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -82,6 +82,7 @@ int cfg80211_set_monitor_channel(struct cfg80211_registered_device *rdev,
 				 int freq, enum nl80211_channel_type chantype)
 {
 	struct ieee80211_channel *chan;
+	int err;
 
 	if (!rdev->ops->set_monitor_channel)
 		return -EOPNOTSUPP;
@@ -92,7 +93,13 @@ int cfg80211_set_monitor_channel(struct cfg80211_registered_device *rdev,
 	if (!chan)
 		return -EINVAL;
 
-	return rdev->ops->set_monitor_channel(&rdev->wiphy, chan, chantype);
+	err = rdev->ops->set_monitor_channel(&rdev->wiphy, chan, chantype);
+	if (!err) {
+		rdev->monitor_channel = chan;
+		rdev->monitor_channel_type = chantype;
+	}
+
+	return err;
 }
 
 void
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 9097d6c..8315c2a 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -789,9 +789,15 @@ void cfg80211_update_iface_num(struct cfg80211_registered_device *rdev,
 		rdev->num_running_monitor_ifaces += num;
 
 	has_monitors_only_new = cfg80211_has_monitors_only(rdev);
-	if (has_monitors_only_new != has_monitors_only_old)
+	if (has_monitors_only_new != has_monitors_only_old) {
 		rdev->ops->set_monitor_enabled(&rdev->wiphy,
 					       has_monitors_only_new);
+
+		if (!has_monitors_only_new) {
+			rdev->monitor_channel = NULL;
+			rdev->monitor_channel_type = NL80211_CHAN_NO_HT;
+		}
+	}
 }
 
 static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
diff --git a/net/wireless/core.h b/net/wireless/core.h
index b27fa3b..9d9441d 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -59,6 +59,9 @@ struct cfg80211_registered_device {
 	int num_running_ifaces;
 	int num_running_monitor_ifaces;
 
+	struct ieee80211_channel *monitor_channel;
+	enum nl80211_channel_type monitor_channel_type;
+
 	/* BSSes/scanning */
 	spinlock_t bss_lock;
 	struct list_head bss_list;
-- 
1.7.0.4

[RFCv2 10/13] cfg80211: set initial monitor channel

[Michal Kazior]

Implements behaviour seen in mac80211. A running
monitor always has a channel - even before
.set_channel. This way we won't break current
behaviour.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/core.c |   31 +++++++++++++++++++++++++++++++
 1 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 8315c2a..5b3f74e 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -776,6 +776,35 @@ static struct device_type wiphy_type = {
 	.name	= "wlan",
 };
 
+static struct ieee80211_channel *
+cfg80211_get_any_chan(struct cfg80211_registered_device *rdev)
+{
+	struct ieee80211_supported_band *sband;
+	int i;
+
+	for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
+		sband = rdev->wiphy.bands[i];
+		if (sband && sband->n_channels > 0)
+			return &sband->channels[0];
+	}
+
+	return NULL;
+}
+
+static void cfg80211_init_mon_chan(struct cfg80211_registered_device *rdev)
+{
+	struct ieee80211_channel *chan;
+
+	chan = cfg80211_get_any_chan(rdev);
+	if (WARN_ON(!chan))
+		return;
+
+	mutex_lock(&rdev->devlist_mtx);
+	WARN_ON(cfg80211_set_monitor_channel(rdev, chan->center_freq,
+					     NL80211_CHAN_NO_HT));
+	mutex_unlock(&rdev->devlist_mtx);
+}
+
 void cfg80211_update_iface_num(struct cfg80211_registered_device *rdev,
 			       enum nl80211_iftype iftype, int num)
 {
@@ -796,6 +825,8 @@ void cfg80211_update_iface_num(struct cfg80211_registered_device *rdev,
 		if (!has_monitors_only_new) {
 			rdev->monitor_channel = NULL;
 			rdev->monitor_channel_type = NL80211_CHAN_NO_HT;
+		} else {
+			cfg80211_init_mon_chan(rdev);
 		}
 	}
 }
-- 
1.7.0.4

Re: [RFCv2 10/13] cfg80211: set initial monitor channel

[Johannes Berg]

On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
> Implements behaviour seen in mac80211. A running
> monitor always has a channel - even before
> .set_channel. This way we won't break current
> behaviour.

I'm a little doubtful about this. Currently, mac80211 will keep the
channel if you set it on say wlan0, your monitor iface gets the same
channel. Then if you remove wlan0, you still have the same channel, but
it seems here this is different now? I'm not sure it matters much, but
it's worth thinking about? Or am I totally misunderstanding this now?

johannes

Re: [RFCv2 10/13] cfg80211: set initial monitor channel

[Michal Kazior]

Johannes Berg wrote:
> On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
>> Implements behaviour seen in mac80211. A running
>> monitor always has a channel - even before
>> .set_channel. This way we won't break current
>> behaviour.
>
> I'm a little doubtful about this. Currently, mac80211 will keep the
> channel if you set it on say wlan0, your monitor iface gets the same
> channel. Then if you remove wlan0, you still have the same channel, but
> it seems here this is different now? I'm not sure it matters much, but
> it's worth thinking about? Or am I totally misunderstanding this now?

Right. It's not the exact same behaviour. It only guarantees a channel 
is always set in monitor mode. We could maybe set the last 
monitor-mode-channel seen when starting up monitor-mode again. Would 
that be okay?


-- 
Pozdrawiam / Best regards, Michal Kazior.

Re: [RFCv2 10/13] cfg80211: set initial monitor channel

[Johannes Berg]

On Mon, 2012-06-25 at 08:57 +0200, Michal Kazior wrote:
> Johannes Berg wrote:
> > On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
> >> Implements behaviour seen in mac80211. A running
> >> monitor always has a channel - even before
> >> .set_channel. This way we won't break current
> >> behaviour.
> >
> > I'm a little doubtful about this. Currently, mac80211 will keep the
> > channel if you set it on say wlan0, your monitor iface gets the same
> > channel. Then if you remove wlan0, you still have the same channel, but
> > it seems here this is different now? I'm not sure it matters much, but
> > it's worth thinking about? Or am I totally misunderstanding this now?
> 
> Right. It's not the exact same behaviour. It only guarantees a channel 
> is always set in monitor mode. We could maybe set the last 
> monitor-mode-channel seen when starting up monitor-mode again. Would 
> that be okay?

Yeah I think that's probably fine. Come to think of it, without that
change you always have a channel pointer, but it's not guaranteed to be
programmed into the device? Maybe I'm confused.

johannes

Re: [RFCv2 10/13] cfg80211: set initial monitor channel

[Michal Kazior]

Johannes Berg wrote:
> On Mon, 2012-06-25 at 08:57 +0200, Michal Kazior wrote:
>> Johannes Berg wrote:
>>> On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
>>>> Implements behaviour seen in mac80211. A running
>>>> monitor always has a channel - even before
>>>> .set_channel. This way we won't break current
>>>> behaviour.
>>>
>>> I'm a little doubtful about this. Currently, mac80211 will keep the
>>> channel if you set it on say wlan0, your monitor iface gets the same
>>> channel. Then if you remove wlan0, you still have the same channel, but
>>> it seems here this is different now? I'm not sure it matters much, but
>>> it's worth thinking about? Or am I totally misunderstanding this now?
>>
>> Right. It's not the exact same behaviour. It only guarantees a channel
>> is always set in monitor mode. We could maybe set the last
>> monitor-mode-channel seen when starting up monitor-mode again. Would
>> that be okay?
>
> Yeah I think that's probably fine. Come to think of it, without that
> change you always have a channel pointer, but it's not guaranteed to be
> programmed into the device? Maybe I'm confused.

The rdev->monitor_channel is set to NULL when monitor mode is disabled 
(see `cfg80211: track monitor channel`).

When monitor mode is started the hardware is programmed (and pointer 
set) to the first channel found in supported bands (see `cfg80211: set 
initial monitor channel`).


-- 
Pozdrawiam / Best regards, Michal Kazior.

Re: [RFCv2 10/13] cfg80211: set initial monitor channel

[Johannes Berg]

On Mon, 2012-06-25 at 09:40 +0200, Michal Kazior wrote:
> Johannes Berg wrote:
> > On Mon, 2012-06-25 at 08:57 +0200, Michal Kazior wrote:
> >> Johannes Berg wrote:
> >>> On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
> >>>> Implements behaviour seen in mac80211. A running
> >>>> monitor always has a channel - even before
> >>>> .set_channel. This way we won't break current
> >>>> behaviour.
> >>>
> >>> I'm a little doubtful about this. Currently, mac80211 will keep the
> >>> channel if you set it on say wlan0, your monitor iface gets the same
> >>> channel. Then if you remove wlan0, you still have the same channel, but
> >>> it seems here this is different now? I'm not sure it matters much, but
> >>> it's worth thinking about? Or am I totally misunderstanding this now?
> >>
> >> Right. It's not the exact same behaviour. It only guarantees a channel
> >> is always set in monitor mode. We could maybe set the last
> >> monitor-mode-channel seen when starting up monitor-mode again. Would
> >> that be okay?
> >
> > Yeah I think that's probably fine. Come to think of it, without that
> > change you always have a channel pointer, but it's not guaranteed to be
> > programmed into the device? Maybe I'm confused.
> 
> The rdev->monitor_channel is set to NULL when monitor mode is disabled 
> (see `cfg80211: track monitor channel`).
> 
> When monitor mode is started the hardware is programmed (and pointer 
> set) to the first channel found in supported bands (see `cfg80211: set 
> initial monitor channel`).

Ah ok, fair enough. I don't mind this, let's see if anyone complains? :)

johannes

[RFCv2 11/13] cfg80211/mac80211: remove .get_channel

[Michal Kazior]

We do not need it anymore since cfg80211 tracks
monitor channel and monitor channel type.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 include/net/cfg80211.h     |    6 ------
 net/mac80211/cfg.c         |   11 -----------
 net/wireless/nl80211.c     |   16 +++++-----------
 net/wireless/wext-compat.c |    9 ++-------
 4 files changed, 7 insertions(+), 35 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 120eae0..7672552 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1430,9 +1430,6 @@ struct cfg80211_gtk_rekey_data {
  *	be stored for when a monitor interface becomes active.
  * @set_monitor_enabled: Notify driver that there are only monitor
  *	interfaces running.
- * @get_channel: Get the current operating channel, should return %NULL if
- *	there's no single defined operating channel if for example the
- *	device implements channel hopping for multi-channel virtual interfaces.
  *
  * @scan: Request to do a scan. If returning zero, the scan request is given
  *	the driver, and will be valid until passed to cfg80211_scan_done().
@@ -1739,9 +1736,6 @@ struct cfg80211_ops {
 				  struct net_device *dev,
 				  u16 noack_map);
 
-	struct ieee80211_channel *(*get_channel)(struct wiphy *wiphy,
-					       enum nl80211_channel_type *type);
-
 	int	(*get_et_sset_count)(struct wiphy *wiphy,
 				     struct net_device *dev, int sset);
 	void	(*get_et_stats)(struct wiphy *wiphy, struct net_device *dev,
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index df7332e..4efc1ff 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2933,16 +2933,6 @@ static int ieee80211_probe_client(struct wiphy *wiphy, struct net_device *dev,
 	return 0;
 }
 
-static struct ieee80211_channel *
-ieee80211_wiphy_get_channel(struct wiphy *wiphy,
-			    enum nl80211_channel_type *type)
-{
-	struct ieee80211_local *local = wiphy_priv(wiphy);
-
-	*type = local->_oper_channel_type;
-	return local->oper_channel;
-}
-
 static void ieee80211_set_monitor_enabled(struct wiphy *wiphy, bool enabled)
 {
 	struct ieee80211_local *local = wiphy_priv(wiphy);
@@ -3026,7 +3016,6 @@ struct cfg80211_ops mac80211_config_ops = {
 	.tdls_oper = ieee80211_tdls_oper,
 	.tdls_mgmt = ieee80211_tdls_mgmt,
 	.probe_client = ieee80211_probe_client,
-	.get_channel = ieee80211_wiphy_get_channel,
 	.set_noack_map = ieee80211_set_noack_map,
 	.set_monitor_enabled = ieee80211_set_monitor_enabled,
 #ifdef CONFIG_PM
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 5d36124..819ebc0 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1588,17 +1588,11 @@ static int nl80211_send_iface(struct sk_buff *msg, u32 pid, u32 seq, int flags,
 			(cfg80211_rdev_list_generation << 2)))
 		goto nla_put_failure;
 
-	if (rdev->ops->get_channel) {
-		struct ieee80211_channel *chan;
-		enum nl80211_channel_type channel_type;
-
-		chan = rdev->ops->get_channel(&rdev->wiphy, &channel_type);
-		if (chan &&
-		    (nla_put_u32(msg, NL80211_ATTR_WIPHY_FREQ,
-				    chan->center_freq) ||
-		     nla_put_u32(msg, NL80211_ATTR_WIPHY_CHANNEL_TYPE,
-				    channel_type)))
-			goto nla_put_failure;
+	if (rdev->monitor_channel) {
+		nla_put_u32(msg, NL80211_ATTR_WIPHY_FREQ,
+			    rdev->monitor_channel->center_freq);
+		nla_put_u32(msg, NL80211_ATTR_WIPHY_CHANNEL_TYPE,
+			    rdev->monitor_channel_type);
 	}
 
 	return genlmsg_end(msg, hdr);
diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index bc87983..7df42f5 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -827,8 +827,6 @@ static int cfg80211_wext_giwfreq(struct net_device *dev,
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_registered_device *rdev = wiphy_to_dev(wdev->wiphy);
-	struct ieee80211_channel *chan;
-	enum nl80211_channel_type channel_type;
 
 	switch (wdev->iftype) {
 	case NL80211_IFTYPE_STATION:
@@ -836,13 +834,10 @@ static int cfg80211_wext_giwfreq(struct net_device *dev,
 	case NL80211_IFTYPE_ADHOC:
 		return cfg80211_ibss_wext_giwfreq(dev, info, freq, extra);
 	case NL80211_IFTYPE_MONITOR:
-		if (!rdev->ops->get_channel)
+		if (!rdev->monitor_channel)
 			return -EINVAL;
 
-		chan = rdev->ops->get_channel(wdev->wiphy, &channel_type);
-		if (!chan)
-			return -EINVAL;
-		freq->m = chan->center_freq;
+		freq->m = rdev->monitor_channel->center_freq;
 		freq->e = 6;
 		return 0;
 	default:
-- 
1.7.0.4

[RFCv2 12/13] cfg80211: add channel checking for iface combinations

[Michal Kazior]

.connect cannot be handled since the driver scans
and connects on its own. It is up to the driver
then to refuse a connection (with -EBUSY for
example).

Non-fixed channel IBSSes always take a single
channel resource. For example two non-fixed
channel IBSSes always take up 2
num_different_channels, even if they operate on
the same channel at a given point of time.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/core.h |   27 ++++++++++++++++--
 net/wireless/util.c |   73 +++++++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 92 insertions(+), 8 deletions(-)

diff --git a/net/wireless/core.h b/net/wireless/core.h
index 9d9441d..17da8f0 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -454,9 +454,20 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
 			  u32 *flags, struct vif_params *params);
 void cfg80211_process_rdev_events(struct cfg80211_registered_device *rdev);
 
-int cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
-				  struct wireless_dev *wdev,
-				  enum nl80211_iftype iftype);
+int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
+				 struct wireless_dev *wdev,
+				 enum nl80211_iftype iftype,
+				 struct ieee80211_channel *chan,
+				 enum cfg80211_chan_mode chanmode);
+
+static inline int
+cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
+			      struct wireless_dev *wdev,
+			      enum nl80211_iftype iftype)
+{
+	return cfg80211_can_use_iftype_chan(rdev, wdev, iftype, NULL,
+					    CHAN_MODE_UNDEFINED);
+}
 
 static inline int
 cfg80211_can_add_interface(struct cfg80211_registered_device *rdev,
@@ -465,6 +476,16 @@ cfg80211_can_add_interface(struct cfg80211_registered_device *rdev,
 	return cfg80211_can_change_interface(rdev, NULL, iftype);
 }
 
+static inline int
+cfg80211_can_use_chan(struct cfg80211_registered_device *rdev,
+		      struct wireless_dev *wdev,
+		      struct ieee80211_channel *chan,
+		      enum cfg80211_chan_mode chanmode)
+{
+	return cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
+					    chan, chanmode);
+}
+
 void
 cfg80211_get_chan_state(struct cfg80211_registered_device *rdev,
 		        struct wireless_dev *wdev,
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 9b92ec5..d40f324 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -938,13 +938,20 @@ int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev,
 	return res;
 }
 
-int cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
-				  struct wireless_dev *wdev,
-				  enum nl80211_iftype iftype)
+int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
+				 struct wireless_dev *wdev,
+				 enum nl80211_iftype iftype,
+				 struct ieee80211_channel *chan,
+				 enum cfg80211_chan_mode chanmode)
 {
 	struct wireless_dev *wdev_iter;
 	u32 used_iftypes = BIT(iftype);
 	int num[NUM_NL80211_IFTYPES];
+	struct ieee80211_channel **used_channels;
+	struct ieee80211_channel *ch;
+	enum cfg80211_chan_mode chmode;
+	int num_different_channels = 0;
+	int num_max_channels = 0;
 	int total = 1;
 	int i, j;
 
@@ -954,6 +961,33 @@ int cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
 	if (rdev->wiphy.software_iftypes & BIT(iftype))
 		return 0;
 
+	for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
+		if (!rdev->wiphy.bands[i])
+			continue;
+
+		num_max_channels += rdev->wiphy.bands[i]->n_channels;
+	}
+	if (!num_max_channels)
+		return -EINVAL;
+
+	used_channels = kzalloc(num_max_channels * sizeof(*used_channels),
+				GFP_KERNEL);
+	if (!used_channels)
+		return -ENOMEM;
+
+	switch (chanmode) {
+	case CHAN_MODE_UNDEFINED:
+		break;
+	case CHAN_MODE_SHARED:
+		WARN_ON(!chan);
+		used_channels[0] = chan;
+		num_different_channels++;
+		break;
+	case CHAN_MODE_EXCLUSIVE:
+		num_different_channels++;
+		break;
+	}
+
 	memset(num, 0, sizeof(num));
 
 	num[iftype] = 1;
@@ -968,11 +1002,37 @@ int cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
 		if (rdev->wiphy.software_iftypes & BIT(wdev_iter->iftype))
 			continue;
 
+		cfg80211_get_chan_state(rdev, wdev_iter, &ch, &chmode);
+
+		switch (chmode) {
+		case CHAN_MODE_UNDEFINED:
+			break;
+		case CHAN_MODE_SHARED:
+			for (i = 0; i < num_max_channels; i++)
+				if (!used_channels[i] || used_channels[i] == ch)
+					break;
+
+			if (i == num_max_channels) {
+				kfree(used_channels);
+				return -ENOMEM;
+			}
+
+			if (used_channels[i] == NULL) {
+				used_channels[i] = ch;
+				num_different_channels++;
+			}
+			break;
+		case CHAN_MODE_EXCLUSIVE:
+			num_different_channels++;
+			break;
+		}
+
 		num[wdev_iter->iftype]++;
 		total++;
 		used_iftypes |= BIT(wdev_iter->iftype);
 	}
 	mutex_unlock(&rdev->devlist_mtx);
+	kfree(used_channels);
 
 	if (total == 1)
 		return 0;
@@ -984,12 +1044,15 @@ int cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
 
 		c = &rdev->wiphy.iface_combinations[i];
 
+		if (total > c->max_interfaces)
+			continue;
+		if (num_different_channels > c->num_different_channels)
+			continue;
+
 		limits = kmemdup(c->limits, sizeof(limits[0]) * c->n_limits,
 				 GFP_KERNEL);
 		if (!limits)
 			return -ENOMEM;
-		if (total > c->max_interfaces)
-			goto cont;
 
 		for (iftype = 0; iftype < NUM_NL80211_IFTYPES; iftype++) {
 			if (rdev->wiphy.software_iftypes & BIT(iftype))
-- 
1.7.0.4

Re: [RFCv2 12/13] cfg80211: add channel checking for iface combinations

[Johannes Berg]

On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:

> +		case CHAN_MODE_SHARED:
> +			for (i = 0; i < num_max_channels; i++)
> +				if (!used_channels[i] || used_channels[i] == ch)
> +					break;
> +
> +			if (i == num_max_channels) {
> +				kfree(used_channels);
> +				return -ENOMEM;
> +			}

Maybe use -EBUSY? -ENOMEM doesn't seem to make much sense? Or maybe I'm
misunderstanding that? You allocated a pretty large number before (one
for each possible channel) so we can't really run into it anyway I
think?

Or ... maybe we should just do something like

 * force num_different_channels to be less than a constant we define,
   say #define CFG80211_MAX_NUM_DIFFERENT_CHANNELS 10 by checking in
   wiphy registration -- if somebody needs more .... they can change
   the constant but that seems very unlikely
 * use a channel array with that size on the stack here, then we don't
   even need to alloc memory

johannes

Re: [RFCv2 12/13] cfg80211: add channel checking for iface combinations

[Michal Kazior]

Johannes Berg wrote:
> On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
>
>> +		case CHAN_MODE_SHARED:
>> +			for (i = 0; i<  num_max_channels; i++)
>> +				if (!used_channels[i] || used_channels[i] == ch)
>> +					break;
>> +
>> +			if (i == num_max_channels) {
>> +				kfree(used_channels);
>> +				return -ENOMEM;
>> +			}
>
> Maybe use -EBUSY? -ENOMEM doesn't seem to make much sense? Or maybe I'm
> misunderstanding that? You allocated a pretty large number before (one
> for each possible channel) so we can't really run into it anyway I
> think?
>
> Or ... maybe we should just do something like
>
>   * force num_different_channels to be less than a constant we define,
>     say #define CFG80211_MAX_NUM_DIFFERENT_CHANNELS 10 by checking in
>     wiphy registration -- if somebody needs more .... they can change
>     the constant but that seems very unlikely
>   * use a channel array with that size on the stack here, then we don't
>     even need to alloc memory

I just get too generic sometimes. Sounds good to me.


-- 
Pozdrawiam / Best regards, Michal Kazior.

[RFCv2 13/13] cfg80211: respect iface combinations when starting operation

[Michal Kazior]

devlist_mtx locking is changed to accomodate changes.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/core.c    |    2 ++
 net/wireless/ibss.c    |   10 ++++++++++
 net/wireless/mesh.c    |    7 +++++++
 net/wireless/mlme.c    |    8 ++++++++
 net/wireless/nl80211.c |    8 ++++++++
 net/wireless/util.c    |    5 +++--
 6 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 5b3f74e..3098f1e 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1041,7 +1041,9 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 			return notifier_from_errno(-EOPNOTSUPP);
 		if (rfkill_blocked(rdev->rfkill))
 			return notifier_from_errno(-ERFKILL);
+		mutex_lock(&rdev->devlist_mtx);
 		ret = cfg80211_can_add_interface(rdev, wdev->iftype);
+		mutex_unlock(&rdev->devlist_mtx);
 		if (ret)
 			return notifier_from_errno(ret);
 		cfg80211_lock_rdev(rdev);
diff --git a/net/wireless/ibss.c b/net/wireless/ibss.c
index b90fd86..ca5672f 100644
--- a/net/wireless/ibss.c
+++ b/net/wireless/ibss.c
@@ -118,6 +118,16 @@ int __cfg80211_join_ibss(struct cfg80211_registered_device *rdev,
 	wdev->wext.ibss.channel = params->channel;
 #endif
 	wdev->sme_state = CFG80211_SME_CONNECTING;
+
+	err = cfg80211_can_use_chan(rdev, wdev, params->channel,
+				    params->channel_fixed
+				    ? CHAN_MODE_SHARED
+				    : CHAN_MODE_EXCLUSIVE);
+	if (err) {
+		wdev->connect_keys = NULL;
+		return err;
+	}
+
 	err = rdev->ops->join_ibss(&rdev->wiphy, dev, params);
 	if (err) {
 		wdev->connect_keys = NULL;
diff --git a/net/wireless/mesh.c b/net/wireless/mesh.c
index c9c38e3..a1e648a 100644
--- a/net/wireless/mesh.c
+++ b/net/wireless/mesh.c
@@ -149,6 +149,11 @@ int __cfg80211_join_mesh(struct cfg80211_registered_device *rdev,
 					  setup->channel_type))
 		return -EINVAL;
 
+	err = cfg80211_can_use_chan(rdev, wdev, setup->channel,
+				    CHAN_MODE_SHARED);
+	if (err)
+		return err;
+
 	err = rdev->ops->join_mesh(&rdev->wiphy, dev, conf, setup);
 	if (!err) {
 		memcpy(wdev->ssid, setup->mesh_id, setup->mesh_id_len);
@@ -167,9 +172,11 @@ int cfg80211_join_mesh(struct cfg80211_registered_device *rdev,
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	int err;
 
+	mutex_lock(&rdev->devlist_mtx);
 	wdev_lock(wdev);
 	err = __cfg80211_join_mesh(rdev, dev, setup, conf);
 	wdev_unlock(wdev);
+	mutex_unlock(&rdev->devlist_mtx);
 
 	return err;
 }
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index da4406f..3692aa9 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -302,8 +302,14 @@ int __cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
 	if (!req.bss)
 		return -ENOENT;
 
+	err = cfg80211_can_use_chan(rdev, wdev, req.bss->channel,
+				    CHAN_MODE_SHARED);
+	if (err)
+		goto end;
+
 	err = rdev->ops->auth(&rdev->wiphy, dev, &req);
 
+end:
 	cfg80211_put_bss(req.bss);
 	return err;
 }
@@ -317,11 +323,13 @@ int cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
 {
 	int err;
 
+	mutex_lock(&rdev->devlist_mtx);
 	wdev_lock(dev->ieee80211_ptr);
 	err = __cfg80211_mlme_auth(rdev, dev, chan, auth_type, bssid,
 				   ssid, ssid_len, ie, ie_len,
 				   key, key_len, key_idx);
 	wdev_unlock(dev->ieee80211_ptr);
+	mutex_unlock(&rdev->devlist_mtx);
 
 	return err;
 }
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 819ebc0..85ddb3e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2349,6 +2349,14 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 					  params.channel_type))
 		return -EINVAL;
 
+	mutex_lock(&rdev->devlist_mtx);
+	err = cfg80211_can_use_chan(rdev, wdev, params.channel,
+				    CHAN_MODE_SHARED);
+	mutex_unlock(&rdev->devlist_mtx);
+
+	if (err)
+		return err;
+
 	err = rdev->ops->start_ap(&rdev->wiphy, dev, &params);
 	if (!err) {
 		wdev->beacon_interval = params.beacon_interval;
diff --git a/net/wireless/util.c b/net/wireless/util.c
index d40f324..51bda53 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -805,8 +805,10 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
 		return -EBUSY;
 
 	if (ntype != otype && netif_running(dev)) {
+		mutex_lock(&rdev->devlist_mtx);
 		err = cfg80211_can_change_interface(rdev, dev->ieee80211_ptr,
 						    ntype);
+		mutex_unlock(&rdev->devlist_mtx);
 		if (err)
 			return err;
 
@@ -956,6 +958,7 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
 	int i, j;
 
 	ASSERT_RTNL();
+	lockdep_assert_held(&rdev->devlist_mtx);
 
 	/* Always allow software iftypes */
 	if (rdev->wiphy.software_iftypes & BIT(iftype))
@@ -992,7 +995,6 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
 
 	num[iftype] = 1;
 
-	mutex_lock(&rdev->devlist_mtx);
 	list_for_each_entry(wdev_iter, &rdev->netdev_list, list) {
 		if (wdev_iter == wdev)
 			continue;
@@ -1031,7 +1033,6 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
 		total++;
 		used_iftypes |= BIT(wdev_iter->iftype);
 	}
-	mutex_unlock(&rdev->devlist_mtx);
 	kfree(used_channels);
 
 	if (total == 1)
-- 
1.7.0.4

Re: [RFCv2 13/13] cfg80211: respect iface combinations when starting operation

[Johannes Berg]

On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:

> +++ b/net/wireless/mlme.c
> @@ -302,8 +302,14 @@ int __cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
>  	if (!req.bss)
>  		return -ENOENT;
>  
> +	err = cfg80211_can_use_chan(rdev, wdev, req.bss->channel,
> +				    CHAN_MODE_SHARED);
> +	if (err)
> +		goto end;

I think you need it in assoc too?


looks good!

johannes

Re: [RFCv2 13/13] cfg80211: respect iface combinations when starting operation

[Michal Kazior]

Johannes Berg wrote:
> On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
>
>> +++ b/net/wireless/mlme.c
>> @@ -302,8 +302,14 @@ int __cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
>>   	if (!req.bss)
>>   		return -ENOENT;
>>
>> +	err = cfg80211_can_use_chan(rdev, wdev, req.bss->channel,
>> +				    CHAN_MODE_SHARED);
>> +	if (err)
>> +		goto end;
>
> I think you need it in assoc too?

Hmm.. I didn't think it's possible for bss to change channels in between 
auth and assoc. I'll add the check for assoc too then.


-- 
Pozdrawiam / Best regards, Michal Kazior.

Re: [RFCv2 13/13] cfg80211: respect iface combinations when starting operation

[Johannes Berg]

On Mon, 2012-06-25 at 09:05 +0200, Michal Kazior wrote:
> Johannes Berg wrote:
> > On Wed, 2012-06-20 at 08:14 +0200, Michal Kazior wrote:
> >
> >> +++ b/net/wireless/mlme.c
> >> @@ -302,8 +302,14 @@ int __cfg80211_mlme_auth(struct cfg80211_registered_device *rdev,
> >>   	if (!req.bss)
> >>   		return -ENOENT;
> >>
> >> +	err = cfg80211_can_use_chan(rdev, wdev, req.bss->channel,
> >> +				    CHAN_MODE_SHARED);
> >> +	if (err)
> >> +		goto end;
> >
> > I think you need it in assoc too?
> 
> Hmm.. I didn't think it's possible for bss to change channels in between 
> auth and assoc. I'll add the check for assoc too then.

There's no guarantee we ever go through auth, since we could be doing
FT-over-the-DS, in which case we only come through assoc.

johannes

Re: [RFCv2] respect channels in iface combinations

[Johannes Berg]

Hi Michal,

> I've addressed the issue with non-fixed channel IBSS.
> Channels are now cosidered either shared or exclusive. An
> exclusive channel always takes a channel resource (when
> accouting interface combinations and
> num_different_channels). Exclusive channel could also be
> called hopping but I thought the first one fits better as it
> explicitly states code logic decisions.
> 
> I've also refactored and fixed interface combination
> checking. There are now functions like
> cfg80211_can_use_chan, cfg80211_can_use_iftype_chan.
> 
> Monitor channel is now always defined. When there are
> only monitors running we set a channel (first we can find in
> supported bands structures). This way the corner case
> (of a monitor not having a channel and .get_channel
> returning NULL) mentioned by Johannes is fixed.

Thanks!

I've looked over it briefly and sent a few comments, but I still need to
look at patches 6, 10, 12 and 13 in more detail.

johannes