[RFC] multi-channel work

[RFC] multi-channel work

[Michal Kazior]

Hi Johannes,

I've done another patchset aimed at multi-channel operation. This one
is mostly cfg80211 related.

The patchset clarifies channel handling by cfg80211. As I understand
we want to treat monitor interface as a special case. Monitor
interface can be used as a passive way to sniff data on other
interfaces. Monitor may have a channel set - this should be only
possible when there are no other interfaces running. Otherwise we
can't easily do channels accounting (with regard to interface
combinations). In other words the virtual monitor interface concept is
copied from mac80211 to cfg80211 itself making the whole thing more
consitent in my opinion.

The patchset also introduces num_different_channel checks. It hasn't
been thoroughly tested yet.

This work is based upon your patches on channel API changes.


--
Pozdrawiam / Best regards,
Michal Kazior.

[RFC 01/14] cfg80211: respect intf combinations for 1 interface

[Michal Kazior]

Don't ignore interface combinations when only one
interface is up. Otherwise we may call a driver to
create (or change) interface type to something it
didn't report in interface combinations it
reported.

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

diff --git a/net/wireless/util.c b/net/wireless/util.c
index 177df03..147f283 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -965,7 +965,7 @@ int cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
 	}
 	mutex_unlock(&rdev->devlist_mtx);
 
-	if (total == 1)
+	if (total == 1 && rdev->wiphy.n_iface_combinations == 0)
 		return 0;
 
 	for (i = 0; i < rdev->wiphy.n_iface_combinations; i++) {
-- 
1.7.0.4

Re: [RFC 01/14] cfg80211: respect intf combinations for 1 interface

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> Don't ignore interface combinations when only one
> interface is up. Otherwise we may call a driver to
> create (or change) interface type to something it
> didn't report in interface combinations it
> reported.

We still check wiphy.interface_modes before we even go into this
function, no? Arguably we should move the check into the function, but
I'm not sure what you're fixing here.

johannes

Re: [RFC 01/14] cfg80211: respect intf combinations for 1 interface

[Michal Kazior]

Johannes Berg wrote:
> On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
>> Don't ignore interface combinations when only one
>> interface is up. Otherwise we may call a driver to
>> create (or change) interface type to something it
>> didn't report in interface combinations it
>> reported.
>
> We still check wiphy.interface_modes before we even go into this
> function, no? Arguably we should move the check into the function, but
> I'm not sure what you're fixing here.

Suppose a driver advertises support for AP and STA. Without the patch 
it's possible to bring up e.g. IBSS interface.


-- 
Pozdrawiam / Best regards, Michal Kazior.

Re: [RFC 01/14] cfg80211: respect intf combinations for 1 interface

[Johannes Berg]

On Wed, 2012-06-06 at 10:56 +0200, Michal Kazior wrote:
> Johannes Berg wrote:
> > On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> >> Don't ignore interface combinations when only one
> >> interface is up. Otherwise we may call a driver to
> >> create (or change) interface type to something it
> >> didn't report in interface combinations it
> >> reported.
> >
> > We still check wiphy.interface_modes before we even go into this
> > function, no? Arguably we should move the check into the function, but
> > I'm not sure what you're fixing here.
> 
> Suppose a driver advertises support for AP and STA. Without the patch 
> it's possible to bring up e.g. IBSS interface.

I don't think so?

main.c:1005
        case NETDEV_PRE_UP:
                if (!(wdev->wiphy->interface_modes & BIT(wdev->iftype)))
                        return notifier_from_errno(-EOPNOTSUPP);

But it does make sense to move that into the function.

johannes

[RFC 02/14] cfg80211: check iface combinations only when intf is running

[Michal Kazior]

Don't validate interface combinations on a stopped
interface. Otherwise we might end up being able to
create a new interface with a certain type, but
won't be able to change an existing interface
type.

This also skips some other functions when
interface is stopped when changing interface type.

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

diff --git a/net/wireless/util.c b/net/wireless/util.c
index 147f283..eb7ca3c 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -805,7 +805,7 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
 	     ntype == NL80211_IFTYPE_P2P_CLIENT))
 		return -EBUSY;
 
-	if (ntype != otype) {
+	if (ntype != otype && netif_running(dev)) {
 		err = cfg80211_can_change_interface(rdev, dev->ieee80211_ptr,
 						    ntype);
 		if (err)
-- 
1.7.0.4

Re: [RFC 02/14] cfg80211: check iface combinations only when intf is running

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> Don't validate interface combinations on a stopped
> interface. Otherwise we might end up being able to
> create a new interface with a certain type, but
> won't be able to change an existing interface
> type.
> 
> This also skips some other functions when
> interface is stopped when changing interface type.
> 
> Change-Id: I02af110905184c8182aa30dc9054a2c53f0238d0
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  net/wireless/util.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/wireless/util.c b/net/wireless/util.c
> index 147f283..eb7ca3c 100644
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
> @@ -805,7 +805,7 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
>  	     ntype == NL80211_IFTYPE_P2P_CLIENT))
>  		return -EBUSY;
>  
> -	if (ntype != otype) {
> +	if (ntype != otype && netif_running(dev)) {

Yeah, makes sense, can you send it separately for 3.5?

johannes

[RFC 03/14] 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.

Change-Id: I358660cdac652b56eacdd0adae76cc16aa4e4800
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 5e29bd3..e28c74a 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

Re: [RFC 03/14] cfg80211: introduce cfg80211_stop_ap

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> This functionality will be reused when interface
> is going down. Avoids code duplication. Also adds
> missing wdev locking.
> 
> Change-Id: I358660cdac652b56eacdd0adae76cc16aa4e4800
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Looks good.

johannes

[RFC 04/14] 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).

Change-Id: I7fc8eac0989ced8a81048f299a9c311e0692f850
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 eb7ca3c..252ba8d 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -815,6 +815,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

Re: [RFC 04/14] cfg80211: .stop_ap when interface is going down

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> We'll need this for proper channel tracking (which
> is going to be needed for channel context
> accounting and finding matching/active interface
> combination).

I've not quite made up my mind about full channel tracking in cfg80211,
but this totally makes sense anyway.

johannes

[RFC 05/14] 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.

Monitor mode is going to be handled differently.

Change-Id: I693f58c51aef1f814edb5451ca8b701eb6d65377
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 e28c74a..7486653 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: [RFC 05/14] cfg80211: add channel tracking for AP and mesh

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +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.
> 
> STA/IBSS have current_bss already which allows us
> to check which channel a vif is tuned to.

For IBSS this assumption is dangerous though since the channel can
change at will when it decides to merge with a different cell on a
different channel.

I don't mind tracking the channel for AP/mesh of course.

johannes

[RFC 06/14] cfg80211: introduce cfg80211_get_used_channel

[Michal Kazior]

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

Will be used for matching interface combinations.

Change-Id: Ic9e5a68d66aaa26e73901648b04a6e9b465430d0
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/chan.c |   31 +++++++++++++++++++++++++++++++
 net/wireless/core.h |    4 ++++
 2 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index c1999e4..45b28ab 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -92,3 +92,34 @@ int cfg80211_set_monitor_channel(struct cfg80211_registered_device *rdev,
 
 	return rdev->ops->set_monitor_channel(&rdev->wiphy, chan, chantype);
 }
+
+struct ieee80211_channel *
+cfg80211_get_used_channel(struct cfg80211_registered_device *rdev,
+			  struct wireless_dev *wdev)
+{
+	ASSERT_RDEV_LOCK(rdev);
+	ASSERT_WDEV_LOCK(wdev);
+
+	switch (wdev->iftype) {
+	case NL80211_IFTYPE_ADHOC:
+	case NL80211_IFTYPE_STATION:
+	case NL80211_IFTYPE_P2P_CLIENT:
+		if (wdev->current_bss)
+			return wdev->current_bss->pub.channel;
+		break;
+	case NL80211_IFTYPE_AP:
+	case NL80211_IFTYPE_P2P_GO:
+	case NL80211_IFTYPE_MESH_POINT:
+		return wdev->channel;
+	case NL80211_IFTYPE_MONITOR:
+	case NL80211_IFTYPE_AP_VLAN:
+	case NL80211_IFTYPE_WDS:
+		/* these interface types don't really have a channel */
+		return NULL;
+	case NL80211_IFTYPE_UNSPECIFIED:
+	case NUM_NL80211_IFTYPES:
+		WARN_ON(1);
+	}
+
+	return NULL;
+}
diff --git a/net/wireless/core.h b/net/wireless/core.h
index a366ffd..54c24de 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -446,6 +446,10 @@ cfg80211_can_add_interface(struct cfg80211_registered_device *rdev,
 }
 
 struct ieee80211_channel *
+cfg80211_get_used_channel(struct cfg80211_registered_device *rdev,
+			  struct wireless_dev *wdev);
+
+struct ieee80211_channel *
 rdev_freq_to_chan(struct cfg80211_registered_device *rdev,
 		  int freq, enum nl80211_channel_type channel_type);
 int cfg80211_set_monitor_channel(struct cfg80211_registered_device *rdev,
-- 
1.7.0.4

Re: [RFC 06/14] cfg80211: introduce cfg80211_get_used_channel

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> Helper function for finding out which channel is
> used by a given interface.
> 
> Will be used for matching interface combinations.
> 
> Change-Id: Ic9e5a68d66aaa26e73901648b04a6e9b465430d0
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
>  net/wireless/chan.c |   31 +++++++++++++++++++++++++++++++
>  net/wireless/core.h |    4 ++++
>  2 files changed, 35 insertions(+), 0 deletions(-)
> 
> diff --git a/net/wireless/chan.c b/net/wireless/chan.c
> index c1999e4..45b28ab 100644
> --- a/net/wireless/chan.c
> +++ b/net/wireless/chan.c
> @@ -92,3 +92,34 @@ int cfg80211_set_monitor_channel(struct cfg80211_registered_device *rdev,
>  
>  	return rdev->ops->set_monitor_channel(&rdev->wiphy, chan, chantype);
>  }
> +
> +struct ieee80211_channel *
> +cfg80211_get_used_channel(struct cfg80211_registered_device *rdev,
> +			  struct wireless_dev *wdev)
> +{
> +	ASSERT_RDEV_LOCK(rdev);
> +	ASSERT_WDEV_LOCK(wdev);
> +
> +	switch (wdev->iftype) {
> +	case NL80211_IFTYPE_ADHOC:
> +	case NL80211_IFTYPE_STATION:
> +	case NL80211_IFTYPE_P2P_CLIENT:
> +		if (wdev->current_bss)
> +			return wdev->current_bss->pub.channel;

I think the IBSS part isn't something we should really do. Maybe if we
track whether it's fixed-channel or not (to not break some use cases
like freifunk), but I don't think we should assume IBSS stays here.

In mac80211, IBSS causes it to assume "channel hopping" to handle this.
Maybe here we could return some "cookie" channel pointer to handle this.

johannes

[RFC 07/14] 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.

Change-Id: Ibd82a70c256c2de584eb541ea2c36663a59f09d4
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 include/net/cfg80211.h |    4 ++++
 net/wireless/core.c    |   18 ++++++++++++++++++
 net/wireless/core.h    |    9 +++++++++
 3 files changed, 31 insertions(+), 0 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 0f9d7b4..e2da8e4 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..5b37047 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -776,6 +776,22 @@ static struct device_type wiphy_type = {
 	.name	= "wlan",
 };
 
+static void cfg80211_update_iface_num(struct cfg80211_registered_device *rdev,
+				      struct wireless_dev *wdev, int num)
+{
+	bool has_monitors_only_old = cfg80211_has_monitors_only(rdev);
+	bool has_monitors_only_new;
+
+	rdev->num_running_ifaces += num;
+	if (wdev->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 +895,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		break;
 	case NETDEV_DOWN:
 		dev_hold(dev);
+		cfg80211_update_iface_num(rdev, wdev, -1);
 		queue_work(cfg80211_wq, &wdev->cleanup_work);
 		break;
 	case NETDEV_UP:
@@ -986,6 +1003,7 @@ 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_update_iface_num(rdev, wdev, 1);
 		break;
 	}
 
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 54c24de..79e27b1 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;
@@ -220,6 +223,12 @@ static inline void wdev_unlock(struct wireless_dev *wdev)
 	mutex_unlock(&wdev->mtx);
 }
 
+static inline bool cfg80211_has_monitors_only(struct cfg80211_registered_device *rdev)
+{
+	return rdev->num_running_ifaces == rdev->num_running_monitor_ifaces &&
+	       rdev->num_running_ifaces > 0;
+}
+
 #define ASSERT_RDEV_LOCK(rdev) lockdep_assert_held(&(rdev)->mtx)
 #define ASSERT_WDEV_LOCK(wdev) lockdep_assert_held(&(wdev)->mtx)
 
-- 
1.7.0.4

Re: [RFC 07/14] cfg80211: track monitor interfaces count

[Eliad Peller]

hi Michal,

On Mon, May 28, 2012 at 2:18 PM, Michal Kazior <michal.kazior@tieto.com> 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.
>
> Change-Id: Ibd82a70c256c2de584eb541ea2c36663a59f09d4
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
[...]

> @@ -879,6 +895,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
>                break;
>        case NETDEV_DOWN:
>                dev_hold(dev);
> +               cfg80211_update_iface_num(rdev, wdev, -1);
>                queue_work(cfg80211_wq, &wdev->cleanup_work);
>                break;
>        case NETDEV_UP:
> @@ -986,6 +1003,7 @@ 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_update_iface_num(rdev, wdev, 1);
>                break;
>        }
>

what about interface type change?

Eliad.

Re: [RFC 07/14] cfg80211: track monitor interfaces count

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +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.
> 
> Change-Id: Ibd82a70c256c2de584eb541ea2c36663a59f09d4
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

This is essentially how mac80211 behaves now, and I have no problem
imposing it on the rest of the stack as well, but I think we can
probably get rid of some more generic functionality then?

Like the software_iftypes. What if the driver actually wants us to track
the monitor channel like any other context? Doesn't make any sense for
mac80211, but before this it would have been possible.

Essentially you're saying that monitor is always a software iftype, and
that it should always behave like in mac80211 -- if it's alone then it's
a monitor, if not alone it just sees what the others see.

What do the others thing? Bing? Kalle?

The other thing we might then want to is make this more general and not
just inform the driver about the monitor/no-monitor layout change, but
also tell it which interface combination we're in right now? Might look
a bit more like

set_iface_combination(wiphy, dev, combination);

or even
set_iface_combination(wiphy, dev, combination, have_monitor);


Then pure monitor would be "combination == NULL, have_monitor=True",
etc. The only downside is that we don't have combinations advertised for
when there's a single interface only, so we'd have to point to some
internal single-interface combinations then (static in cfg80211).
Thoughts?

johannes

Re: [RFC 07/14] cfg80211: track monitor interfaces count

[Michal Kazior]

Johannes Berg wrote:
> The other thing we might then want to is make this more general and not
> just inform the driver about the monitor/no-monitor layout change, but
> also tell it which interface combination we're in right now? Might look
> a bit more like
>
> set_iface_combination(wiphy, dev, combination);
>
> or even
> set_iface_combination(wiphy, dev, combination, have_monitor);
>
>
> Then pure monitor would be "combination == NULL, have_monitor=True",
> etc. The only downside is that we don't have combinations advertised for
> when there's a single interface only, so we'd have to point to some
> internal single-interface combinations then (static in cfg80211).
> Thoughts?

I'm afraid we can't simply tell which combination we're in. I imagine 
there may be two or three combinations matching at the same time. Take 
this for example:

   comb1: 1xSTA + 1xAP (max channels = 1)
   comb2: 2xSTA (max channels = 2)

   wlan0 is running as STA @ chan=1

Which combination are we in? Which one should we report to the driver?

I don't see what passing current interface combination would be useful 
for. Do you have something particular in mind?


-- 
Pozdrawiam / Best regards, Michal Kazior.

Re: [RFC 07/14] cfg80211: track monitor interfaces count

[Johannes Berg]

On Wed, 2012-06-06 at 13:40 +0200, Michal Kazior wrote:
> Johannes Berg wrote:
> > The other thing we might then want to is make this more general and not
> > just inform the driver about the monitor/no-monitor layout change, but
> > also tell it which interface combination we're in right now? Might look
> > a bit more like
> >
> > set_iface_combination(wiphy, dev, combination);
> >
> > or even
> > set_iface_combination(wiphy, dev, combination, have_monitor);
> >
> >
> > Then pure monitor would be "combination == NULL, have_monitor=True",
> > etc. The only downside is that we don't have combinations advertised for
> > when there's a single interface only, so we'd have to point to some
> > internal single-interface combinations then (static in cfg80211).
> > Thoughts?
> 
> I'm afraid we can't simply tell which combination we're in. I imagine 
> there may be two or three combinations matching at the same time. Take 
> this for example:
> 
>    comb1: 1xSTA + 1xAP (max channels = 1)
>    comb2: 2xSTA (max channels = 2)
> 
>    wlan0 is running as STA @ chan=1
> 
> Which combination are we in? Which one should we report to the driver?

Doesn't matter, right? Pick the one with most channels or something? :-)

> I don't see what passing current interface combination would be useful 
> for. Do you have something particular in mind?

I'm thinking about # of channels in the combination mostly, in
particular in combination with IBSS. OTOH, if we always reserve one
channel for IBSS in cfg80211 (unless it's fixed & driver supports that)
then it won't matter.

But then again, what about say 2xSTA being active and drivers with
connect() API?

johannes

RE: [RFC 07/14] cfg80211: track monitor interfaces count

[Bing Zhao]

SGkgSm9oYW5uZXMsDQoNCj4gPiBJbXBsZW1lbnRzIC5zZXRfbW9uaXRvcl9lbmFibGVkKHdpcGh5
LCBlbmFibGVkKS4NCj4gPg0KPiA+IE5vdGlmaWVzIGRyaXZlciB1cG9uIGNoYW5nZSBvZiBpbnRl
cmZhY2UgbGF5b3V0Lg0KPiA+DQo+ID4gSWYgb25seSBtb25pdG9yIGludGVyZmFjZXMgYmVjb21l
IHByZXNlbnQgaXQgaXMNCj4gPiBjYWxsZWQgd2l0aCAybmQgYXJndW1lbnQgYmVpbmcgdHJ1ZS4g
SWYNCj4gPiBub24tbW9uaXRvciBpbnRlcmZhY2UgYXBwZWFycyB0aGVuIDJuZCBhcmd1bWVudA0K
PiA+IGlzIGZhbHNlLiBEcml2ZXIgaXMgbm90aWZpZWQgb25seSB1cG9uIGNoYW5nZS4NCj4gPg0K
PiA+IFRoaXMgbWFrZXMgaXQgbW9yZSBvYnZpb3VzIGFib3V0IHRoZSBmYWN0IHRoYXQNCj4gPiBj
Zmc4MDIxMSBzdXBwb3J0cyBzaW5nbGUgbW9uaXRvciBjaGFubmVsLiBPbmNlIHdlDQo+ID4gaW1w
bGVtZW50IG11bHRpLWNoYW5uZWwgd2UgZG9uJ3Qgd2FudCB0byBhbGxvdw0KPiA+IHNldHRpbmcg
bW9uaXRvciBjaGFubmVsIHdoaWxlIG90aGVyIGludGVyZmFjZQ0KPiA+IHR5cGVzIGFyZSBydW5u
aW5nLiBPdGhlcndpc2UgaXQgd291bGQgYmUgYW1iaWd1b3VzDQo+ID4gb25jZSB3ZSBzdGFydCBj
b25zaWRlcmluZyBudW1fZGlmZmVyZW50X2NoYW5uZWxzLg0KPiA+DQo+ID4gQ2hhbmdlLUlkOiBJ
YmQ4MmE3MGMyNTZjMmRlNTg0ZWI1NDFlYTJjMzY2NjNhNTlmMDlkNA0KPiA+IFNpZ25lZC1vZmYt
Ynk6IE1pY2hhbCBLYXppb3IgPG1pY2hhbC5rYXppb3JAdGlldG8uY29tPg0KPiANCj4gVGhpcyBp
cyBlc3NlbnRpYWxseSBob3cgbWFjODAyMTEgYmVoYXZlcyBub3csIGFuZCBJIGhhdmUgbm8gcHJv
YmxlbQ0KPiBpbXBvc2luZyBpdCBvbiB0aGUgcmVzdCBvZiB0aGUgc3RhY2sgYXMgd2VsbCwgYnV0
IEkgdGhpbmsgd2UgY2FuDQo+IHByb2JhYmx5IGdldCByaWQgb2Ygc29tZSBtb3JlIGdlbmVyaWMg
ZnVuY3Rpb25hbGl0eSB0aGVuPw0KPiANCj4gTGlrZSB0aGUgc29mdHdhcmVfaWZ0eXBlcy4gV2hh
dCBpZiB0aGUgZHJpdmVyIGFjdHVhbGx5IHdhbnRzIHVzIHRvIHRyYWNrDQo+IHRoZSBtb25pdG9y
IGNoYW5uZWwgbGlrZSBhbnkgb3RoZXIgY29udGV4dD8gRG9lc24ndCBtYWtlIGFueSBzZW5zZSBm
b3INCj4gbWFjODAyMTEsIGJ1dCBiZWZvcmUgdGhpcyBpdCB3b3VsZCBoYXZlIGJlZW4gcG9zc2li
bGUuDQo+IA0KPiBFc3NlbnRpYWxseSB5b3UncmUgc2F5aW5nIHRoYXQgbW9uaXRvciBpcyBhbHdh
eXMgYSBzb2Z0d2FyZSBpZnR5cGUsIGFuZA0KPiB0aGF0IGl0IHNob3VsZCBhbHdheXMgYmVoYXZl
IGxpa2UgaW4gbWFjODAyMTEgLS0gaWYgaXQncyBhbG9uZSB0aGVuIGl0J3MNCj4gYSBtb25pdG9y
LCBpZiBub3QgYWxvbmUgaXQganVzdCBzZWVzIHdoYXQgdGhlIG90aGVycyBzZWUuDQo+IA0KPiBX
aGF0IGRvIHRoZSBvdGhlcnMgdGhpbmc/IEJpbmc/IEthbGxlPw0KDQptd2lmaWV4IGRyaXZlciBk
b2Vzbid0IHN1cHBvcnQgbW9uaXRvciBpbnRlcmZhY2UgdHlwZSBjdXJyZW50bHkuIFNvIGFueSBj
aGFuZ2VzIHJlbGF0ZWQgdG8gbW9uaXRvciBzaG91bGRuJ3QgaGF2ZSBhbnkgaW1wYWN0IG9uIG13
aWZpZXgsIEFGQUlDUy4NCg0KVGhhbmtzLA0KQmluZw0KDQo+IA0KPiBUaGUgb3RoZXIgdGhpbmcg
d2UgbWlnaHQgdGhlbiB3YW50IHRvIGlzIG1ha2UgdGhpcyBtb3JlIGdlbmVyYWwgYW5kIG5vdA0K
PiBqdXN0IGluZm9ybSB0aGUgZHJpdmVyIGFib3V0IHRoZSBtb25pdG9yL25vLW1vbml0b3IgbGF5
b3V0IGNoYW5nZSwgYnV0DQo+IGFsc28gdGVsbCBpdCB3aGljaCBpbnRlcmZhY2UgY29tYmluYXRp
b24gd2UncmUgaW4gcmlnaHQgbm93PyBNaWdodCBsb29rDQo+IGEgYml0IG1vcmUgbGlrZQ0KPiAN
Cj4gc2V0X2lmYWNlX2NvbWJpbmF0aW9uKHdpcGh5LCBkZXYsIGNvbWJpbmF0aW9uKTsNCj4gDQo+
IG9yIGV2ZW4NCj4gc2V0X2lmYWNlX2NvbWJpbmF0aW9uKHdpcGh5LCBkZXYsIGNvbWJpbmF0aW9u
LCBoYXZlX21vbml0b3IpOw0KPiANCj4gDQo+IFRoZW4gcHVyZSBtb25pdG9yIHdvdWxkIGJlICJj
b21iaW5hdGlvbiA9PSBOVUxMLCBoYXZlX21vbml0b3I9VHJ1ZSIsDQo+IGV0Yy4gVGhlIG9ubHkg
ZG93bnNpZGUgaXMgdGhhdCB3ZSBkb24ndCBoYXZlIGNvbWJpbmF0aW9ucyBhZHZlcnRpc2VkIGZv
cg0KPiB3aGVuIHRoZXJlJ3MgYSBzaW5nbGUgaW50ZXJmYWNlIG9ubHksIHNvIHdlJ2QgaGF2ZSB0
byBwb2ludCB0byBzb21lDQo+IGludGVybmFsIHNpbmdsZS1pbnRlcmZhY2UgY29tYmluYXRpb25z
IHRoZW4gKHN0YXRpYyBpbiBjZmc4MDIxMSkuDQo+IFRob3VnaHRzPw0KPiANCj4gam9oYW5uZXMN
Cj4gDQoNCg==

[RFC 08/14] mac80211: refactor virtual monitor code

[Michal Kazior]

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

Change-Id: I2e881ab09294711aeec24d567b7fcf1bd19bee20
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 20a5910..07218cd 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2899,6 +2899,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)
 {
@@ -2974,6 +2984,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 d8a266e..826b98f 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1504,6 +1504,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);
+
 /* internal work items */
 void ieee80211_work_init(struct ieee80211_local *local);
 void ieee80211_add_work(struct ieee80211_work *wk);
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index c550945..25037ec 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;
@@ -630,7 +622,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);
@@ -704,9 +695,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

Re: [RFC 08/14] mac80211: refactor virtual monitor code

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> Use cfg80211 the new .set_monitor_enabled instead
> of tracking it inside mac80211.

I .. think I missed the introduction of set_monitor_enabled, where was
it?

johannes

Re: [RFC 08/14] mac80211: refactor virtual monitor code

[Johannes Berg]

On Wed, 2012-06-06 at 10:58 +0200, Johannes Berg wrote:
> On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> > Use cfg80211 the new .set_monitor_enabled instead
> > of tracking it inside mac80211.
> 
> I .. think I missed the introduction of set_monitor_enabled, where was
> it?

Nevermind, I skipped patch 7 -- because somebody else had replied it had
moved up in my email list.

johannes

Re: [RFC 08/14] mac80211: refactor virtual monitor code

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> Use cfg80211 the new .set_monitor_enabled instead
> of tracking it inside mac80211.

Makes sense with the previous patch as is, but changes there obviously
affect this.

johannes

[RFC 09/14] 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.

Change-Id: I2d579b483fe3c2998c50dcf89600740188f79733
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 45b28ab..4d71b38 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

Re: [RFC 09/14] cfg80211: refuse to .set_monitor_channel when non-monitors are present

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> Having .set_monitor_channel work with non-monitor
> interfaces running would make interface
> combinations accounting ambiguous.
> 
> Change-Id: I2d579b483fe3c2998c50dcf89600740188f79733
> 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 45b28ab..4d71b38 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;

Makes sense.

johannes

[RFC 10/14] cfg80211: track monitor channel

[Michal Kazior]

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

Change-Id: I607e12c99c38a51f5b0348510ff26b0ac0a30a69
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 4d71b38..3c83917 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;
 }
 
 struct ieee80211_channel *
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 5b37047..1596d18 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -787,9 +787,15 @@ static 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 79e27b1..2dd7508 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

Re: [RFC 10/14] cfg80211: track monitor channel

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> Make it even more obvious we support single
> monitor channel. This will allow us to remove
> .get_channel.

This has an interesting corner case -- where do we take the channel from
if it was never set, but monitors are created? It looks like it should
be initialised somehow, but then ...

Maybe we also need to explicitly set the monitor channel whenever we
call set_monitor_enabled()?

johannes

[RFC 11/14] cfg80211/mac80211: remove .get_channel

[Michal Kazior]

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

Change-Id: Ib0ea094217b510c364358a695e9a06b969e7b972
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 e2da8e4..550252c 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 07218cd..8a71401 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2889,16 +2889,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);
@@ -2982,7 +2972,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 7486653..e02ec2c 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

Re: [RFC 11/14] cfg80211/mac80211: remove .get_channel

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> We do not need it anymore since cfg80211 tracks
> monitor channel and monitor channel type.

Previously though we did handle the corner case of not having a monitor
channel at all by using whatever channel was last set by say a managed
interface or whatever default mac80211 used (channel 1)

johannes

[RFC 12/14] cfg80211: move devlist locking out of can_change_interface

[Michal Kazior]

Soon this function will be called for .start_ap,
.join_ibss, .join_mesh and .auth which may be run
under devlist_mtx lock already.

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

diff --git a/net/wireless/core.c b/net/wireless/core.c
index 1596d18..d0caca8 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1006,7 +1006,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_update_iface_num(rdev, wdev, 1);
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 252ba8d..9449a60 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -806,8 +806,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;
 
@@ -944,6 +946,7 @@ int cfg80211_can_change_interface(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))
@@ -953,7 +956,6 @@ int cfg80211_can_change_interface(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;
@@ -966,7 +968,6 @@ int cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
 		num[wdev_iter->iftype]++;
 		total++;
 	}
-	mutex_unlock(&rdev->devlist_mtx);
 
 	if (total == 1 && rdev->wiphy.n_iface_combinations == 0)
 		return 0;
-- 
1.7.0.4

[RFC 13/14] cfg80211: extend combination checking to consider channels

[Michal Kazior]

cfg80211_can_change_interface will soon be used
before .start_ap, .join_mesh, .join_ibss and .auth to
verify whether a given interface combination is
allowed.

.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).

Change-Id: I334c78aa9189f24fcf8829b98b7703ddde120eec
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/core.c |    2 +-
 net/wireless/core.h |    8 +++++---
 net/wireless/util.c |   46 ++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/net/wireless/core.c b/net/wireless/core.c
index d0caca8..67dbbbb 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -1007,7 +1007,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		if (rfkill_blocked(rdev->rfkill))
 			return notifier_from_errno(-ERFKILL);
 		mutex_lock(&rdev->devlist_mtx);
-		ret = cfg80211_can_add_interface(rdev, wdev->iftype);
+		ret = cfg80211_can_add_interface(rdev, wdev->iftype, NULL);
 		mutex_unlock(&rdev->devlist_mtx);
 		if (ret)
 			return notifier_from_errno(ret);
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 2dd7508..26f6625 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -448,13 +448,15 @@ 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);
+				  enum nl80211_iftype iftype,
+				  struct ieee80211_channel *chan);
 
 static inline int
 cfg80211_can_add_interface(struct cfg80211_registered_device *rdev,
-			   enum nl80211_iftype iftype)
+			   enum nl80211_iftype iftype,
+			   struct ieee80211_channel *chan)
 {
-	return cfg80211_can_change_interface(rdev, NULL, iftype);
+	return cfg80211_can_change_interface(rdev, NULL, iftype, chan);
 }
 
 struct ieee80211_channel *
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 9449a60..59c06bb 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -808,7 +808,7 @@ int cfg80211_change_iface(struct cfg80211_registered_device *rdev,
 	if (ntype != otype && netif_running(dev)) {
 		mutex_lock(&rdev->devlist_mtx);
 		err = cfg80211_can_change_interface(rdev, dev->ieee80211_ptr,
-						    ntype);
+						    ntype, NULL);
 		mutex_unlock(&rdev->devlist_mtx);
 		if (err)
 			return err;
@@ -938,10 +938,14 @@ int cfg80211_validate_beacon_int(struct cfg80211_registered_device *rdev,
 
 int cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
 				  struct wireless_dev *wdev,
-				  enum nl80211_iftype iftype)
+				  enum nl80211_iftype iftype,
+				  struct ieee80211_channel *chan)
 {
 	struct wireless_dev *wdev_iter;
 	int num[NUM_NL80211_IFTYPES];
+	struct ieee80211_channel **used_channels;
+	int num_different_channels = 0;
+	int num_max_channels = 0;
 	int total = 1;
 	int i, j;
 
@@ -952,6 +956,20 @@ 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;
+
 	memset(num, 0, sizeof(num));
 
 	num[iftype] = 1;
@@ -965,9 +983,26 @@ int cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
 		if (rdev->wiphy.software_iftypes & BIT(wdev_iter->iftype))
 			continue;
 
+		for (i = 0; i < num_max_channels; i++) {
+			if (!used_channels[i]) {
+				used_channels[i] = chan;
+				num_different_channels++;
+				break;
+			}
+			else if (used_channels[i] == chan) {
+				break;
+			}
+		}
+
+		if (i == num_max_channels) {
+			kfree(used_channels);
+			return -ENOMEM;
+		}
+
 		num[wdev_iter->iftype]++;
 		total++;
 	}
+	kfree(used_channels);
 
 	if (total == 1 && rdev->wiphy.n_iface_combinations == 0)
 		return 0;
@@ -978,12 +1013,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: [RFC 13/14] cfg80211: extend combination checking to consider channels

[Eliad Peller]

hi Michal,

On Mon, May 28, 2012 at 2:19 PM, Michal Kazior <michal.kazior@tieto.com> wrote:
> cfg80211_can_change_interface will soon be used
> before .start_ap, .join_mesh, .join_ibss and .auth to
> verify whether a given interface combination is
> allowed.
>
> .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).
>
> Change-Id: I334c78aa9189f24fcf8829b98b7703ddde120eec
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
[...]

> @@ -1007,7 +1007,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
>                if (rfkill_blocked(rdev->rfkill))
>                        return notifier_from_errno(-ERFKILL);
>                mutex_lock(&rdev->devlist_mtx);
> -               ret = cfg80211_can_add_interface(rdev, wdev->iftype);
> +               ret = cfg80211_can_add_interface(rdev, wdev->iftype, NULL);

you pass NULL here...

> @@ -965,9 +983,26 @@ int cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
>                if (rdev->wiphy.software_iftypes & BIT(wdev_iter->iftype))
>                        continue;
>
> +               for (i = 0; i < num_max_channels; i++) {
> +                       if (!used_channels[i]) {
> +                               used_channels[i] = chan;
> +                               num_different_channels++;
> +                               break;
> +                       }
> +                       else if (used_channels[i] == chan) {
> +                               break;
> +                       }
> +               }
> +
> +               if (i == num_max_channels) {
> +                       kfree(used_channels);
> +                       return -ENOMEM;
> +               }
> +

but this code will count it as a new channel.

Eliad.

Re: [RFC 13/14] cfg80211: extend combination checking to consider channels

[Johannes Berg]

On Mon, 2012-05-28 at 13:19 +0200, Michal Kazior wrote:
> cfg80211_can_change_interface will soon be used
> before .start_ap, .join_mesh, .join_ibss and .auth to
> verify whether a given interface combination is
> allowed.
> 
> .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).

 
>  int cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
>  				  struct wireless_dev *wdev,
> -				  enum nl80211_iftype iftype);
> +				  enum nl80211_iftype iftype,
> +				  struct ieee80211_channel *chan);
>  
>  static inline int
>  cfg80211_can_add_interface(struct cfg80211_registered_device *rdev,
> -			   enum nl80211_iftype iftype)
> +			   enum nl80211_iftype iftype,
> +			   struct ieee80211_channel *chan)
>  {
> -	return cfg80211_can_change_interface(rdev, NULL, iftype);
> +	return cfg80211_can_change_interface(rdev, NULL, iftype, chan);
>  }

I have a feeling we should preserve can_change_interface() as an inline
with the existing prototype, add something like can_use_channel that
doesn't get the iftype, and implement it with a common function like
can_use_iftype_channel():

can_use_iftype_channel(rdev, wdev, iftype, chan)

can_change_interface(rdev, wdev, new_iftype):
	return can_use_iftype_channel(rdev, wdev, new_iftype, NULL)

can_use_channel(rdev, wdev, chan):
	return can_use_iftype_channel(rdev, wdev, wdev->iftype, chan)


I think that would make the code simpler to read since the reader
doesn't have to know about the chan=NULL semantics etc. just like now
with can_add_interface they don't have to know about the wdev=NULL
semantics -- all of that we could explain in can_use_iftype_channel()

johannes

[RFC 14/14] cfg80211: respect iface combinations when starting operation

[Michal Kazior]

Use cfg80211_can_change_interface before starting
up any operation modes such as .start_ap or .auth
to verify whether we comply with
num_different_channels within at least one
interface combination.

Change-Id: I270c18855e28e3b7d63013f66fae5146bfd0826e
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/wireless/ibss.c    |    8 ++++++++
 net/wireless/mesh.c    |    7 +++++++
 net/wireless/mlme.c    |    6 ++++++
 net/wireless/nl80211.c |    8 ++++++++
 4 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/net/wireless/ibss.c b/net/wireless/ibss.c
index 30f20fe..2583148 100644
--- a/net/wireless/ibss.c
+++ b/net/wireless/ibss.c
@@ -115,6 +115,14 @@ int __cfg80211_join_ibss(struct cfg80211_registered_device *rdev,
 #ifdef CONFIG_CFG80211_WEXT
 	wdev->wext.ibss.channel = params->channel;
 #endif
+
+	err = cfg80211_can_change_interface(rdev, wdev, wdev->iftype,
+					    params->channel);
+	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..f26ad50 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_change_interface(rdev, wdev, wdev->iftype,
+					    setup->channel);
+	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);
@@ -168,7 +173,9 @@ int cfg80211_join_mesh(struct cfg80211_registered_device *rdev,
 	int err;
 
 	wdev_lock(wdev);
+	mutex_lock(&rdev->devlist_mtx);
 	err = __cfg80211_join_mesh(rdev, dev, setup, conf);
+	mutex_unlock(&rdev->devlist_mtx);
 	wdev_unlock(wdev);
 
 	return err;
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 83155ee..9e5cab6 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_change_interface(rdev, wdev, wdev->iftype,
+					    req.bss->channel);
+	if (err)
+		goto end;
+
 	err = rdev->ops->auth(&rdev->wiphy, dev, &req);
 
+end:
 	cfg80211_put_bss(req.bss);
 	return err;
 }
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e02ec2c..2757b39 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_change_interface(rdev, wdev, wdev->iftype,
+					    params.channel);
+	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;
-- 
1.7.0.4

Re: [RFC 14/14] cfg80211: respect iface combinations when starting operation

[Eliad Peller]

hi Michal,

On Mon, May 28, 2012 at 2:19 PM, Michal Kazior <michal.kazior@tieto.com> wrote:
> Use cfg80211_can_change_interface before starting
> up any operation modes such as .start_ap or .auth
> to verify whether we comply with
> num_different_channels within at least one
> interface combination.
>
> Change-Id: I270c18855e28e3b7d63013f66fae5146bfd0826e
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
> ---
[...]

> @@ -168,7 +173,9 @@ int cfg80211_join_mesh(struct cfg80211_registered_device *rdev,
>        int err;
>
>        wdev_lock(wdev);
> +       mutex_lock(&rdev->devlist_mtx);
>        err = __cfg80211_join_mesh(rdev, dev, setup, conf);

the locking order should be the other way around.

> diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
> index 83155ee..9e5cab6 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_change_interface(rdev, wdev, wdev->iftype,
> +                                           req.bss->channel);

i'm not sure whether it's intended, but cfg80211_mlme_auth() doesn't
take devlist_mtx, so it's missing.

Eliad.

Re: [RFC 14/14] cfg80211: respect iface combinations when starting operation

[Johannes Berg]

On Mon, 2012-05-28 at 13:19 +0200, Michal Kazior wrote:

> diff --git a/net/wireless/ibss.c b/net/wireless/ibss.c
> index 30f20fe..2583148 100644
> --- a/net/wireless/ibss.c
> +++ b/net/wireless/ibss.c
> @@ -115,6 +115,14 @@ int __cfg80211_join_ibss(struct cfg80211_registered_device *rdev,
>  #ifdef CONFIG_CFG80211_WEXT
>  	wdev->wext.ibss.channel = params->channel;
>  #endif
> +
> +	err = cfg80211_can_change_interface(rdev, wdev, wdev->iftype,
> +					    params->channel);

So here again we have the problem with IBSS -- that channel is not
guaranteed to be used.

I think we have to somehow take into account that IBSS cannot share
channel (channel context in mac80211, just the count here in the
combinations) unless its set to fixed channel (which isn't supported in
all devices)

johannes

Re: [RFC] multi-channel work

[Johannes Berg]

On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
> Hi Johannes,
> 
> I've done another patchset aimed at multi-channel operation. This one
> is mostly cfg80211 related.

Does this replace or amend your previous patchset?

johannes

Re: [RFC] multi-channel work

[Michal Kazior]

Johannes Berg wrote:
> On Mon, 2012-05-28 at 13:18 +0200, Michal Kazior wrote:
>> Hi Johannes,
>>
>> I've done another patchset aimed at multi-channel operation. This one
>> is mostly cfg80211 related.
>
> Does this replace or amend your previous patchset?

It doesn't replace it.


-- 
Pozdrawiam / Best regards, Michal Kazior.

Re: [RFC] multi-channel work

[Johannes Berg]

Hi Michal,

Sorry it took me so long to go through all your work.

> The patchset clarifies channel handling by cfg80211. As I understand
> we want to treat monitor interface as a special case. Monitor
> interface can be used as a passive way to sniff data on other
> interfaces. Monitor may have a channel set - this should be only
> possible when there are no other interfaces running. Otherwise we
> can't easily do channels accounting (with regard to interface
> combinations). In other words the virtual monitor interface concept is
> copied from mac80211 to cfg80211 itself making the whole thing more
> consitent in my opinion.
> 
> The patchset also introduces num_different_channel checks. It hasn't
> been thoroughly tested yet.

I think this looks good. The monitor/software iftypes thing is something
we should discuss more with other people like Kalle & Bing who work on
non-mac80211 drivers, but maybe we can remove the software iftypes thing
from the driver API (not from nl80211) and make monitor always have the
semantics that mac80211 has today.

The only other non-trivial issue I see is the IBSS handling.

Thanks for doing this!

johannes