[PATCH 0/3] cfg80211/mac80211 channel fixes

[PATCH 0/3] cfg80211/mac80211 channel fixes

[Johannes Berg]

Ok so the last series worked but wasn't really good. I realized
that even for normal multi-channel operation we're going to need
get_channel() since otherwise we don't know the channel *type*
that the interface is using (e.g. in managed mode) since the AP
is no real indication of it as we might decide to not use HT40
or whatever...

So instead of just reverting the get_channel removal, I'm adding
it back now with an interface argument. Nothing much changes for
mac80211, but in the future I'll adjust the cfg80211 channel
tracking code to make use of it so that it'll be required for
multi-channel (or the code will have to assume incompatibility.)

This series fixes the monitor mode problems and I've committed
them (but haven't pushed, in case somebody finds errors.)

johannes

[PATCH 1/3] Revert "mac80211: refactor virtual monitor code"

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

This reverts commit 870d37fc22f3e40f9f23e06c581c8538fc16a2f0.

This code doesn't work as cfg80211 will call
set_monitor_enabled at the wrong time and it
doesn't seem to be possible to fix this.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/cfg.c         |   11 -----------
 net/mac80211/ieee80211_i.h |    4 ----
 net/mac80211/iface.c       |   16 ++++++++++++++--
 3 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index cfdc03f..e95f24e 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2982,16 +2982,6 @@ static int ieee80211_probe_client(struct wiphy *wiphy, struct net_device *dev,
 	return 0;
 }
 
-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)
 {
@@ -3066,7 +3056,6 @@ struct cfg80211_ops mac80211_config_ops = {
 	.tdls_mgmt = ieee80211_tdls_mgmt,
 	.probe_client = ieee80211_probe_client,
 	.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 7998513..bb61f77 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1491,10 +1491,6 @@ int ieee80211_add_srates_ie(struct ieee80211_sub_if_data *sdata,
 int ieee80211_add_ext_srates_ie(struct ieee80211_sub_if_data *sdata,
 				struct sk_buff *skb, bool need_basic);
 
-/* 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 334ee0f..bfb57dc 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -331,7 +331,7 @@ static void ieee80211_set_default_queues(struct ieee80211_sub_if_data *sdata)
 	sdata->vif.cab_queue = IEEE80211_INVAL_HW_QUEUE;
 }
 
-int ieee80211_add_virtual_monitor(struct ieee80211_local *local)
+static int ieee80211_add_virtual_monitor(struct ieee80211_local *local)
 {
 	struct ieee80211_sub_if_data *sdata;
 	int ret = 0;
@@ -377,7 +377,7 @@ int ieee80211_add_virtual_monitor(struct ieee80211_local *local)
 	return ret;
 }
 
-void ieee80211_del_virtual_monitor(struct ieee80211_local *local)
+static void ieee80211_del_virtual_monitor(struct ieee80211_local *local)
 {
 	struct ieee80211_sub_if_data *sdata;
 
@@ -497,6 +497,12 @@ 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) {
@@ -511,6 +517,8 @@ 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;
@@ -745,6 +753,7 @@ 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);
@@ -818,6 +827,9 @@ 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.10.4

[PATCH 2/3] cfg80211/mac80211: re-add get_channel operation

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

This essentially reverts commit 2e165b818456 but
introduces the get_channel operation with a new
wireless_dev argument so that you can retrieve
the channel per interface. This is necessary as
even though we can track all interface channels
(except monitor) we can't track the channel type
used.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h     |    9 +++++++++
 net/mac80211/cfg.c         |   11 +++++++++++
 net/wireless/nl80211.c     |   16 +++++++++++-----
 net/wireless/wext-compat.c |    9 +++++++--
 4 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 5a67165..8115d68 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1612,6 +1612,10 @@ struct cfg80211_gtk_rekey_data {
  * @get_et_strings:  Ethtool API to get a set of strings to describe stats
  *	and perhaps other supported types of ethtool data-sets.
  *	See @ethtool_ops.get_strings
+ *
+ * @get_channel: Get the current operating channel for the virtual interface.
+ *	For monitor interfaces, it should return %NULL unless there's a single
+ *	current monitoring channel.
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -1821,6 +1825,11 @@ struct cfg80211_ops {
 				  u32 sset, u8 *data);
 
 	void (*set_monitor_enabled)(struct wiphy *wiphy, bool enabled);
+
+	struct ieee80211_channel *
+		(*get_channel)(struct wiphy *wiphy,
+			       struct wireless_dev *wdev,
+			       enum nl80211_channel_type *type);
 };
 
 /*
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index e95f24e..10dd963 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2982,6 +2982,16 @@ static int ieee80211_probe_client(struct wiphy *wiphy, struct net_device *dev,
 	return 0;
 }
 
+static struct ieee80211_channel *
+ieee80211_cfg_get_channel(struct wiphy *wiphy, struct wireless_dev *wdev,
+			  enum nl80211_channel_type *type)
+{
+	struct ieee80211_local *local = wiphy_priv(wiphy);
+
+	*type = local->_oper_channel_type;
+	return local->oper_channel;
+}
+
 #ifdef CONFIG_PM
 static void ieee80211_set_wakeup(struct wiphy *wiphy, bool enabled)
 {
@@ -3062,4 +3072,5 @@ struct cfg80211_ops mac80211_config_ops = {
 	.get_et_sset_count = ieee80211_get_et_sset_count,
 	.get_et_stats = ieee80211_get_et_stats,
 	.get_et_strings = ieee80211_get_et_strings,
+	.get_channel = ieee80211_cfg_get_channel,
 };
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 079fc49..6b001e4 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1759,11 +1759,17 @@ 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->monitor_channel) {
-		if (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))
+	if (rdev->ops->get_channel) {
+		struct ieee80211_channel *chan;
+		enum nl80211_channel_type channel_type;
+
+		chan = rdev->ops->get_channel(&rdev->wiphy, wdev,
+					      &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;
 	}
 
diff --git a/net/wireless/wext-compat.c b/net/wireless/wext-compat.c
index 7df42f5..494379e 100644
--- a/net/wireless/wext-compat.c
+++ b/net/wireless/wext-compat.c
@@ -827,6 +827,8 @@ 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:
@@ -834,10 +836,13 @@ 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->monitor_channel)
+		if (!rdev->ops->get_channel)
 			return -EINVAL;
 
-		freq->m = rdev->monitor_channel->center_freq;
+		chan = rdev->ops->get_channel(wdev->wiphy, wdev, &channel_type);
+		if (!chan)
+			return -EINVAL;
+		freq->m = chan->center_freq;
 		freq->e = 6;
 		return 0;
 	default:
-- 
1.7.10.4

[PATCH 3/3] cfg80211: reduce monitor interface tracking

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Revert commit b78e8ceac23655e1e06b30aa95ab11742d1ac7c0
("cfg80211: track monitor channel") and remove the
set_monitor_enabled() callback.

Due to the tracking happening in NETDEV_PRE_UP, it had
introduced bugs because the monitor interface callback
would be called before the device was started. It looks
like there's no way to fix this, and using NETDEV_PRE_UP
is broken anyway (since there's no NETDEV_UP_FAIL), so
remove all that code, track interfaces in NETDEV_UP and
also stop tracking the monitor channel in cfg80211.

This mostly reverts to before the tracking, except that
we keep the interface count tracking so that setting the
monitor channel can be rejected properly.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 include/net/cfg80211.h |    4 ----
 net/wireless/chan.c    |    9 +--------
 net/wireless/core.c    |   48 +-----------------------------------------------
 net/wireless/core.h    |    3 ---
 4 files changed, 2 insertions(+), 62 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8115d68..0245208 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1504,8 +1504,6 @@ 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.
  *
  * @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().
@@ -1824,8 +1822,6 @@ struct cfg80211_ops {
 	void	(*get_et_strings)(struct wiphy *wiphy, struct net_device *dev,
 				  u32 sset, u8 *data);
 
-	void (*set_monitor_enabled)(struct wiphy *wiphy, bool enabled);
-
 	struct ieee80211_channel *
 		(*get_channel)(struct wiphy *wiphy,
 			       struct wireless_dev *wdev,
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index a16cdff..d355f67 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -82,7 +82,6 @@ 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;
@@ -93,13 +92,7 @@ int cfg80211_set_monitor_channel(struct cfg80211_registered_device *rdev,
 	if (!chan)
 		return -EINVAL;
 
-	err = rdev->ops->set_monitor_channel(&rdev->wiphy, chan, chantype);
-	if (!err) {
-		rdev->monitor_channel = chan;
-		rdev->monitor_channel_type = chantype;
-	}
-
-	return err;
+	return rdev->ops->set_monitor_channel(&rdev->wiphy, chan, chantype);
 }
 
 void
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 0557bb1..71b684b 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -736,60 +736,14 @@ 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)
 {
-	bool has_monitors_only_old = cfg80211_has_monitors_only(rdev);
-	bool has_monitors_only_new;
-
 	ASSERT_RTNL();
 
 	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) {
-		if (rdev->ops->set_monitor_enabled)
-			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;
-		} else {
-			cfg80211_init_mon_chan(rdev);
-		}
-	}
 }
 
 static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
@@ -912,6 +866,7 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 			mutex_unlock(&rdev->devlist_mtx);
 			dev_put(dev);
 		}
+		cfg80211_update_iface_num(rdev, wdev->iftype, 1);
 		cfg80211_lock_rdev(rdev);
 		mutex_lock(&rdev->devlist_mtx);
 		wdev_lock(wdev);
@@ -1006,7 +961,6 @@ static int cfg80211_netdev_notifier_call(struct notifier_block *nb,
 		mutex_unlock(&rdev->devlist_mtx);
 		if (ret)
 			return notifier_from_errno(ret);
-		cfg80211_update_iface_num(rdev, wdev->iftype, 1);
 		break;
 	}
 
diff --git a/net/wireless/core.h b/net/wireless/core.h
index bac97da..5206c68 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -61,9 +61,6 @@ 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.10.4

Re: [PATCH 0/3] cfg80211/mac80211 channel fixes

[Sedat Dilek]

On Fri, Jul 13, 2012 at 1:58 PM, Johannes Berg
<johannes@sipsolutions.net> wrote:
> Ok so the last series worked but wasn't really good. I realized
> that even for normal multi-channel operation we're going to need
> get_channel() since otherwise we don't know the channel *type*
> that the interface is using (e.g. in managed mode) since the AP
> is no real indication of it as we might decide to not use HT40
> or whatever...
>
> So instead of just reverting the get_channel removal, I'm adding
> it back now with an interface argument. Nothing much changes for
> mac80211, but in the future I'll adjust the cfg80211 channel
> tracking code to make use of it so that it'll be required for
> multi-channel (or the code will have to assume incompatibility.)
>
> This series fixes the monitor mode problems and I've committed
> them (but haven't pushed, in case somebody finds errors.)
>

For what tree is this? -next aka 3.6 material?

- Sedat -

> johannes
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/3] cfg80211/mac80211 channel fixes

[Johannes Berg]

On Fri, 2012-07-13 at 14:01 +0200, Sedat Dilek wrote:
> On Fri, Jul 13, 2012 at 1:58 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > Ok so the last series worked but wasn't really good. I realized
> > that even for normal multi-channel operation we're going to need
> > get_channel() since otherwise we don't know the channel *type*
> > that the interface is using (e.g. in managed mode) since the AP
> > is no real indication of it as we might decide to not use HT40
> > or whatever...
> >
> > So instead of just reverting the get_channel removal, I'm adding
> > it back now with an interface argument. Nothing much changes for
> > mac80211, but in the future I'll adjust the cfg80211 channel
> > tracking code to make use of it so that it'll be required for
> > multi-channel (or the code will have to assume incompatibility.)
> >
> > This series fixes the monitor mode problems and I've committed
> > them (but haven't pushed, in case somebody finds errors.)
> >
> 
> For what tree is this? -next aka 3.6 material?

Yes.

johannes