[PATCH 0/3] cfg80211/mac80211: revert/fix monitor interfaces

[PATCH 0/3] cfg80211/mac80211: revert/fix monitor interfaces

[Johannes Berg]

So short of introducing a NETDEV_UP_FAIL hook that closes
NETDEV_PRE_UP, which I don't want to do right now, I don't
see a way to fix the monitor issue.

This mostly reverts back to the old code, but keeps the
monitor interface accounting so we can reject setting the
monitor channel.

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] Revert "cfg80211/mac80211: remove .get_channel"

[Johannes Berg]

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

This reverts commit 2e165b818456ecc1024dd0387eeac64745526377.

Since we're going to have to revert the monitor
interface tracking in cfg80211, we won't have
the ability to track the channel either so we
do need the get_channel callback.

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

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 5a67165..e0968d5 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1506,6 +1506,9 @@ 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().
@@ -1813,6 +1816,9 @@ 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 e95f24e..1e27017 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_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;
+}
+
 #ifdef CONFIG_PM
 static void ieee80211_set_wakeup(struct wiphy *wiphy, bool enabled)
 {
@@ -3055,6 +3065,7 @@ 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,
 #ifdef CONFIG_PM
 	.set_wakeup = ieee80211_set_wakeup,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 079fc49..54726a6 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1759,11 +1759,16 @@ 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, &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..bc87983 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, &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: fix monitor interface tracking

[Johannes Berg]

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

Reverts 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 e0968d5..2d78c00 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.
  * @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.
@@ -1825,8 +1823,6 @@ 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/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