[PATCHv7 0/3] Add DFS master ability

[PATCHv7 0/3] Add DFS master ability

[Simon Wunderlich]

This is patch series version 7 of the DFS master ability, based on Victor
Goldenshteins original work. See changelog in the various patches for
details. The main changes to v6 are:

 * change to channel state handling within cfg80211/mac80211. This moves
   the channel state management from hostapd to the kernel driver, as
   we discussed in previous mails - a change from the original concept.
   We can hopefully support scenarios like multiple interfaces, full DFS
   spectrum scans before enabling an AP, etc better. 
 * also add CAC/NOP handling
 * remove mac80211 start_radar_detection command, use hardware config
   flags instead. the nl80211/cfg80211 command will remain
 * add/change cfg80211 channel checking functions (set channel state,
   get info if radar is required, adapt reg_can_beacon())
 * add/fix tracing
 * more chandef usage
 * rebased on latest master, of course

Thanks again to Johannes and Zefir for the valuable input! I've verified
the implementation (again) with ath9k a patched hostapd (patches to be
released later). This patchset is more an RFC with a few open questions,
any comment/review/etc are very much appreciated. 

Questions:

Right now, channels at least on my machine have the NO IBSS and PASSIVE SCAN
flags set, which prevents reg_can_beacon from returning true, even if we do
have radar support. This is due to my reg database setting these flags for
country code 00, and this is still kept when changing the country code to
something else (DE in my case). How should we proceed with this? Shouldn't
these flags be removed when selecting a country code?

The channel states are now implemented in cfg80211. Shall we inform userspace
about channel changes? If yes, how should we do that? We could add channel states
to the channel list, and give "channel list changed" events to userspace as it
happens now, or define a new kind of event ("channel-available-again-event").
Suggestions welcome. :)

If you prefer, I can split off the channel state stuff from the radar detect
patches.

Cheers,
	Simon

Simon Wunderlich (1):
  nl80211: allow DFS in start_ap

Victor Goldenshtein (2):
  nl80211/cfg80211: add radar detection command/event
  mac80211: add radar detection command/event

 include/net/cfg80211.h       |   57 ++++++++++++++++++
 include/net/mac80211.h       |   18 +++++-
 include/uapi/linux/nl80211.h |   31 ++++++++++
 net/mac80211/cfg.c           |   43 +++++++++++++-
 net/mac80211/chan.c          |   31 ++++++++++
 net/mac80211/ieee80211_i.h   |   11 ++++
 net/mac80211/iface.c         |   12 ++++
 net/mac80211/mlme.c          |   32 +++++++++++
 net/mac80211/scan.c          |    3 +
 net/mac80211/trace.h         |   19 ++++++
 net/wireless/chan.c          |  130 +++++++++++++++++++++++++++++++++++++++++-
 net/wireless/core.c          |    3 +
 net/wireless/core.h          |    8 +++
 net/wireless/mlme.c          |  111 ++++++++++++++++++++++++++++++++++++
 net/wireless/nl80211.c       |  119 +++++++++++++++++++++++++++++++++++++-
 net/wireless/nl80211.h       |    7 +++
 net/wireless/reg.c           |    3 +
 net/wireless/trace.h         |   33 +++++++++++
 18 files changed, 665 insertions(+), 6 deletions(-)

-- 
1.7.10.4

[PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Simon Wunderlich]

From: Victor Goldenshtein <victorg@ti.com>

Add new NL80211_CMD_RADAR_DETECT, which starts the Channel Availability
Check (CAC). This command will also notify the usermode about events
(CAC finished, CAC aborted, radar detected).
Once radar detection has started it should continuously
monitor for radars as long as the channel is active.

Add new NL80211_FEATURE_DFS attribute which
indicates that driver/HW supports radar detection.

Signed-off-by: Victor Goldenshtein <victorg@ti.com>
[siwu@hrz.tu-chemnitz.de: change to chandef, add radar checking function
	to cfg80211, handle CAC and Non-Occupancy Period (NOP), add
	tracing, various fixes]
Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
Changes to PATCHv6:
 * use chandef in radar_detected_notify()
 * add channel states, and handle them in cfg80211
 * add Channel Switch Announcement (CAC) and Non-Occupancy Period (NOP)
   handling to cfg80211
 * add cfg80211_chandef_dfs_required function
 * add tracing for wireless event/functions

Changes to PATCHv5:
 * remove unused cac_type
 * fix doc for cac_started
 * remove dfs_nlportid
---
 include/net/cfg80211.h       |   57 ++++++++++++++++++
 include/uapi/linux/nl80211.h |   31 ++++++++++
 net/wireless/chan.c          |  130 +++++++++++++++++++++++++++++++++++++++++-
 net/wireless/core.c          |    3 +
 net/wireless/core.h          |    8 +++
 net/wireless/mlme.c          |  111 ++++++++++++++++++++++++++++++++++++
 net/wireless/nl80211.c       |  106 ++++++++++++++++++++++++++++++++++
 net/wireless/nl80211.h       |    7 +++
 net/wireless/reg.c           |    3 +
 net/wireless/trace.h         |   33 +++++++++++
 10 files changed, 487 insertions(+), 2 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 970da44..2ee2eb6 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -114,6 +114,24 @@ enum ieee80211_channel_flags {
 	(IEEE80211_CHAN_NO_HT40PLUS | IEEE80211_CHAN_NO_HT40MINUS)
 
 /**
+ * enum ieee80211_dfs_state - DFS states for channels
+ *
+ * Channel states used by the DFS code.
+ *
+ * @IEEE80211_DFS_USABLE: The channel can be used, but channel availability
+ *	check (CAC) must be performed before using it for AP or IBSS.
+ * @IEEE80211_DFS_UNAVAILABLE: A radar has been detected on this channel, it
+ *	is therefore marked as not available.
+ * @IEEE80211_DFS_AVAILABLE: The channel has been CAC checked and is available.
+ */
+
+enum ieee80211_dfs_state {
+	IEEE80211_DFS_USABLE,
+	IEEE80211_DFS_UNAVAILABLE,
+	IEEE80211_DFS_AVAILABLE,
+};
+
+/**
  * struct ieee80211_channel - channel definition
  *
  * This structure describes a single channel for use
@@ -133,6 +151,9 @@ enum ieee80211_channel_flags {
  *	to enable this, this is useful only on 5 GHz band.
  * @orig_mag: internal use
  * @orig_mpwr: internal use
+ * @dfs_state: current state of this channel. Only relevant if radar is required
+ *	on this channel.
+ * @dfs_state_entered: time when the dfs state was entered.
  */
 struct ieee80211_channel {
 	enum ieee80211_band band;
@@ -145,6 +166,8 @@ struct ieee80211_channel {
 	bool beacon_found;
 	u32 orig_flags;
 	int orig_mag, orig_mpwr;
+	enum ieee80211_dfs_state dfs_state;
+	unsigned long dfs_state_entered;
 };
 
 /**
@@ -412,6 +435,16 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
 			     u32 prohibited_flags);
 
 /**
+ * cfg80211_chandef_dfs_required - checks if radar detection
+ *	is required on any of the channels
+ * @wiphy: the wiphy to validate against
+ * @chandef: the channel definition to check
+ * Return: 1 if radar detection is required, 0 if it is not, < 0 on error
+ */
+int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
+				  const struct cfg80211_chan_def *c);
+
+/**
  * enum survey_info_flags - survey information flags
  *
  * @SURVEY_INFO_NOISE_DBM: noise (in dBm) was filled in
@@ -1796,6 +1829,8 @@ struct cfg80211_gtk_rekey_data {
  *
  * @start_p2p_device: Start the given P2P device.
  * @stop_p2p_device: Stop the given P2P device.
+ *
+ * @start_radar_detection: Start radar detection in the driver.
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -2016,6 +2051,10 @@ struct cfg80211_ops {
 				    struct wireless_dev *wdev);
 	void	(*stop_p2p_device)(struct wiphy *wiphy,
 				   struct wireless_dev *wdev);
+
+	int	(*start_radar_detection)(struct wiphy *wiphy,
+					 struct net_device *dev,
+					 struct cfg80211_chan_def *chandef);
 };
 
 /*
@@ -2587,6 +2626,7 @@ struct cfg80211_cached_keys;
  *	beacons, 0 when not valid
  * @address: The address for this device, valid only if @netdev is %NULL
  * @p2p_started: true if this is a P2P Device that has been started
+ * @cac_started: true if DFS channel availability check has been started
  */
 struct wireless_dev {
 	struct wiphy *wiphy;
@@ -2638,6 +2678,8 @@ struct wireless_dev {
 
 	u32 ap_unexpected_nlportid;
 
+	bool cac_started;
+
 #ifdef CONFIG_CFG80211_WEXT
 	/* wext data */
 	struct {
@@ -3629,6 +3671,21 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 			      gfp_t gfp);
 
 /**
+ * cfg80211_radar - radar detection event
+ * @dev: network device
+ * @chandef: chandef for the current channel
+ * @event: type of event
+ * @gfp: context flags
+ *
+ * This function is called when a radar is detected or a CAC event occured
+ * on the current channel. This must be called to notify the completion of
+ * a CAC process, also by full-MAC drivers.
+ */
+void cfg80211_radar_event(struct net_device *dev,
+			  struct cfg80211_chan_def *chandef,
+			  enum nl80211_radar_event event, gfp_t gfp);
+
+/**
  * cfg80211_cqm_pktloss_notify - notify userspace about packetloss to peer
  * @dev: network device
  * @peer: peer's MAC address
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index e6eeb4b..6d27d413 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -586,6 +586,14 @@
  * @NL80211_CMD_SET_MCAST_RATE: Change the rate used to send multicast frames
  *	for IBSS or MESH vif.
  *
+ * @NL80211_CMD_RADAR_DETECT: Start a Channel availability check (CAC). Once
+ *	a radar is detected or the channel availability scan (CAC) has finished
+ *	or was aborted, or a radar was detected, usermode will be notified with
+ *	this event. This command is also used to notify userspace about radars
+ *	while operating on this channel.
+ *	%NL80211_ATTR_RADAR_EVENT is used to inform about the type of the
+ *	event.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -736,6 +744,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_SET_MCAST_RATE,
 
+	NL80211_CMD_RADAR_DETECT,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -1313,6 +1323,9 @@ enum nl80211_commands {
  * @NL80211_ATTR_LOCAL_MESH_POWER_MODE: local mesh STA link-specific power mode
  *	defined in &enum nl80211_mesh_power_mode.
  *
+ * @NL80211_ATTR_RADAR_EVENT: Type of radar event for notification to userspace,
+ *	contains a value of enum nl80211_radar_event (u8).
+ *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -1585,6 +1598,8 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_LOCAL_MESH_POWER_MODE,
 
+	NL80211_ATTR_RADAR_EVENT,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -1640,6 +1655,9 @@ enum nl80211_attrs {
 
 #define NL80211_CQM_TXE_MAX_INTVL		1800
 
+#define NL80211_DFS_MIN_CAC_TIME_MS		60000
+#define NL80211_DFS_MIN_NOP_TIME_MS		(30 * 60 * 1000)
+
 /**
  * enum nl80211_iftype - (virtual) interface types
  *
@@ -3205,6 +3223,7 @@ enum nl80211_ap_sme_features {
  *	Note that even for drivers that support this, the default is to add
  *	stations in authenticated/associated state, so to add unauthenticated
  *	stations the authenticated/associated bits have to be set in the mask.
+ * @NL80211_FEATURE_DFS: Radar detection is supported in the HW/driver.
  */
 enum nl80211_feature_flags {
 	NL80211_FEATURE_SK_TX_STATUS			= 1 << 0,
@@ -3221,6 +3240,7 @@ enum nl80211_feature_flags {
 	NL80211_FEATURE_P2P_GO_CTWIN			= 1 << 11,
 	NL80211_FEATURE_P2P_GO_OPPPS			= 1 << 12,
 	NL80211_FEATURE_FULL_AP_CLIENT_STATE		= 1 << 13,
+	NL80211_FEATURE_DFS				= 1 << 14,
 };
 
 /**
@@ -3276,4 +3296,15 @@ enum nl80211_scan_flags {
 	NL80211_SCAN_FLAG_AP				= 1<<2,
 };
 
+/**
+ * enum nl80211_radar_event - type of radar event for DFS operation
+ *
+ * Type of event to be used with NL80211_ATTR_RADAR_EVENT to inform userspace
+ * about detected radars or success of the channel available check (CAC)
+ */
+enum nl80211_radar_event {
+	NL80211_RADAR_DETECTED,
+	NL80211_CAC_FINISHED,
+	NL80211_CAC_ABORTED,
+};
 #endif /* __LINUX_NL80211_H */
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 396373f..c95b142 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -147,6 +147,32 @@ static void chandef_primary_freqs(const struct cfg80211_chan_def *c,
 	}
 }
 
+static inline u32 cfg80211_chandef_get_width(const struct cfg80211_chan_def *c)
+{
+	u32 width;
+
+	switch (c->width) {
+	case NL80211_CHAN_WIDTH_20:
+	case NL80211_CHAN_WIDTH_20_NOHT:
+		width = 20;
+		break;
+	case NL80211_CHAN_WIDTH_40:
+		width = 40;
+		break;
+	case NL80211_CHAN_WIDTH_80P80:
+	case NL80211_CHAN_WIDTH_80:
+		width = 80;
+		break;
+	case NL80211_CHAN_WIDTH_160:
+		width = 160;
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return -1;
+	}
+	return width;
+}
+
 const struct cfg80211_chan_def *
 cfg80211_chandef_compatible(const struct cfg80211_chan_def *c1,
 			    const struct cfg80211_chan_def *c2)
@@ -192,6 +218,94 @@ cfg80211_chandef_compatible(const struct cfg80211_chan_def *c1,
 }
 EXPORT_SYMBOL(cfg80211_chandef_compatible);
 
+static void cfg80211_set_chans_dfs_state(struct wiphy *wiphy, u32 center_freq,
+					 u32 bandwidth,
+					 enum ieee80211_dfs_state dfs_state)
+{
+	struct ieee80211_channel *c;
+	u32 freq;
+
+	for (freq = center_freq - bandwidth/2 + 10;
+	     freq <= center_freq + bandwidth/2 - 10;
+	     freq += 20) {
+		c = ieee80211_get_channel(wiphy, freq);
+		if (!c || !(c->flags & IEEE80211_CHAN_RADAR))
+			continue;
+
+		c->dfs_state = dfs_state;
+		c->dfs_state_entered = jiffies;
+	}
+}
+
+void cfg80211_set_dfs_state(struct wiphy *wiphy,
+			    const struct cfg80211_chan_def *chandef,
+			    enum ieee80211_dfs_state dfs_state)
+{
+	u32 width;
+
+	if (WARN_ON(!cfg80211_chandef_valid(chandef)))
+		return;
+
+	width = cfg80211_chandef_get_width(chandef);
+	if (width < 0)
+		return;
+
+	cfg80211_set_chans_dfs_state(wiphy, chandef->center_freq1,
+				     width, dfs_state);
+
+	if (!chandef->center_freq2)
+		return;
+	cfg80211_set_chans_dfs_state(wiphy, chandef->center_freq2,
+				     width, dfs_state);
+}
+
+static int cfg80211_get_chans_dfs_required(struct wiphy *wiphy,
+					    u32 center_freq,
+					    u32 bandwidth)
+{
+	struct ieee80211_channel *c;
+	u32 freq;
+
+	for (freq = center_freq - bandwidth/2 + 10;
+	     freq <= center_freq + bandwidth/2 - 10;
+	     freq += 20) {
+		c = ieee80211_get_channel(wiphy, freq);
+		if (!c)
+			return -EINVAL;
+
+		if (c->flags & IEEE80211_CHAN_RADAR)
+			return 1;
+	}
+	return 0;
+}
+
+
+int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
+				  const struct cfg80211_chan_def *chandef)
+{
+	u32 width;
+	int r;
+
+	if (WARN_ON(!cfg80211_chandef_valid(chandef)))
+		return -EINVAL;
+
+	width = cfg80211_chandef_get_width(chandef);
+	if (width < 0)
+		return -EINVAL;
+
+	r = cfg80211_get_chans_dfs_required(wiphy, chandef->center_freq1,
+					    width);
+	if (r)
+		return r;
+
+	if (!chandef->center_freq2)
+		return 0;
+
+	return cfg80211_get_chans_dfs_required(wiphy, chandef->center_freq2,
+					       width);
+}
+EXPORT_SYMBOL(cfg80211_chandef_dfs_required);
+
 static bool cfg80211_secondary_chans_ok(struct wiphy *wiphy,
 					u32 center_freq, u32 bandwidth,
 					u32 prohibited_flags)
@@ -203,7 +317,16 @@ static bool cfg80211_secondary_chans_ok(struct wiphy *wiphy,
 	     freq <= center_freq + bandwidth/2 - 10;
 	     freq += 20) {
 		c = ieee80211_get_channel(wiphy, freq);
-		if (!c || c->flags & prohibited_flags)
+		if (!c)
+			return false;
+
+		/* check for radar flags */
+		if ((prohibited_flags & c->flags & IEEE80211_CHAN_RADAR) &&
+		    (c->dfs_state != IEEE80211_DFS_AVAILABLE))
+			return false;
+
+		/* check for the other flags */
+		if (c->flags & prohibited_flags & ~IEEE80211_CHAN_RADAR)
 			return false;
 	}
 
@@ -344,7 +467,10 @@ cfg80211_get_chan_state(struct wireless_dev *wdev,
 		break;
 	case NL80211_IFTYPE_AP:
 	case NL80211_IFTYPE_P2P_GO:
-		if (wdev->beacon_interval) {
+		if (wdev->cac_started) {
+			*chan = wdev->preset_chandef.chan;
+			*chanmode = CHAN_MODE_SHARED;
+		} else if (wdev->beacon_interval) {
 			*chan = wdev->channel;
 			*chanmode = CHAN_MODE_SHARED;
 		}
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 9245729..2da47eb 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -324,6 +324,8 @@ struct wiphy *wiphy_new(const struct cfg80211_ops *ops, int sizeof_priv)
 	INIT_LIST_HEAD(&rdev->bss_list);
 	INIT_WORK(&rdev->scan_done_wk, __cfg80211_scan_done);
 	INIT_WORK(&rdev->sched_scan_results_wk, __cfg80211_sched_scan_results);
+	INIT_DELAYED_WORK(&rdev->dfs_update_channels_wk,
+			  cfg80211_dfs_channels_update_work);
 #ifdef CONFIG_CFG80211_WEXT
 	rdev->wiphy.wext = &cfg80211_wext_handler;
 #endif
@@ -690,6 +692,7 @@ void wiphy_unregister(struct wiphy *wiphy)
 	flush_work(&rdev->scan_done_wk);
 	cancel_work_sync(&rdev->conn_work);
 	flush_work(&rdev->event_work);
+	cancel_delayed_work_sync(&rdev->dfs_update_channels_wk);
 
 	if (rdev->wowlan && rdev->ops->set_wakeup)
 		rdev_set_wakeup(rdev, false);
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 8396f76..fb5d630 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -87,6 +87,8 @@ struct cfg80211_registered_device {
 
 	struct cfg80211_wowlan *wowlan;
 
+	struct delayed_work dfs_update_channels_wk;
+
 	/* must be last because of the way we do wiphy_priv(),
 	 * and it should at least be aligned to NETDEV_ALIGN */
 	struct wiphy wiphy __aligned(NETDEV_ALIGN);
@@ -427,6 +429,12 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
 				 struct ieee80211_channel *chan,
 				 enum cfg80211_chan_mode chanmode,
 				 u8 radar_detect);
+void cfg80211_set_dfs_state(struct wiphy *wiphy,
+			    const struct cfg80211_chan_def *chandef,
+			    enum ieee80211_dfs_state dfs_state);
+
+void cfg80211_dfs_channels_update_work(struct work_struct *work);
+
 
 static inline int
 cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 461e692..17753f0 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -987,3 +987,114 @@ void cfg80211_pmksa_candidate_notify(struct net_device *dev, int index,
 	nl80211_pmksa_candidate_notify(rdev, dev, index, bssid, preauth, gfp);
 }
 EXPORT_SYMBOL(cfg80211_pmksa_candidate_notify);
+
+static inline void __cfg80211_dfs_clear_channel(struct ieee80211_channel *c,
+						bool *check_again,
+						unsigned long *check_again_time)
+{
+	unsigned long timeout;
+
+	if (c->dfs_state == IEEE80211_DFS_UNAVAILABLE) {
+		timeout = c->dfs_state_entered + NL80211_DFS_MIN_NOP_TIME_MS;
+		if (time_after_eq(jiffies, timeout)) {
+			/* TODO: we could notify userspace about that change */
+			c->dfs_state = IEEE80211_DFS_USABLE;
+		} else {
+			if (!*check_again)
+				*check_again_time = timeout - jiffies;
+			else
+				*check_again_time = min(*check_again_time,
+							timeout - jiffies);
+			*check_again = true;
+		}
+	}
+}
+
+void cfg80211_dfs_channels_update_work(struct work_struct *work)
+{
+	struct delayed_work *delayed_work;
+	struct cfg80211_registered_device *rdev;
+	struct ieee80211_supported_band *sband;
+	struct ieee80211_channel *c;
+	struct wiphy *wiphy;
+	bool check_again = false;
+	unsigned long check_again_time = 0;
+	int bandid, i;
+
+	delayed_work = container_of(work, struct delayed_work, work);
+	rdev = container_of(delayed_work, struct cfg80211_registered_device,
+			    dfs_update_channels_wk);
+	wiphy = &rdev->wiphy;
+
+	mutex_lock(&cfg80211_mutex);
+	for (bandid = 0; bandid < IEEE80211_NUM_BANDS; bandid++) {
+		sband = wiphy->bands[bandid];
+		if (!sband)
+			continue;
+
+		for (i = 0; i < sband->n_channels; i++) {
+			c = &sband->channels[i];
+
+			__cfg80211_dfs_clear_channel(c, &check_again,
+						     &check_again_time);
+		}
+	}
+	mutex_unlock(&cfg80211_mutex);
+
+	/* reschedule if there are other channels waiting to be cleared again */
+	if (check_again)
+		queue_delayed_work(cfg80211_wq, &rdev->dfs_update_channels_wk,
+				   check_again_time);
+}
+
+
+void cfg80211_radar_event(struct net_device *netdev,
+			  struct cfg80211_chan_def *chandef,
+			  enum nl80211_radar_event event,
+			  gfp_t gfp)
+{
+	struct wireless_dev *wdev = netdev->ieee80211_ptr;
+	struct wiphy *wiphy = wdev->wiphy;
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+	unsigned long timeout;
+
+	if (WARN_ON(!cfg80211_chandef_valid(chandef)))
+		return;
+
+	trace_cfg80211_radar_event(netdev, chandef, event);
+
+	switch (event) {
+	case NL80211_RADAR_DETECTED:
+		/*
+		 * only set the chandef supplied channel to unavailable, in
+		 * case the radar is detected on only one of multiple channels
+		 * spanned by the chandef.
+		 */
+		cfg80211_set_dfs_state(wiphy, chandef,
+				       IEEE80211_DFS_UNAVAILABLE);
+
+		timeout = msecs_to_jiffies(NL80211_DFS_MIN_NOP_TIME_MS);
+		queue_delayed_work(cfg80211_wq, &rdev->dfs_update_channels_wk,
+				   timeout);
+		break;
+	case NL80211_CAC_FINISHED:
+		timeout = wdev->preset_chandef.chan->dfs_state_entered;
+		timeout = msecs_to_jiffies(timeout +
+					   NL80211_DFS_MIN_CAC_TIME_MS);
+		WARN_ON(!time_after_eq(jiffies, timeout));
+		cfg80211_set_dfs_state(wiphy, &wdev->preset_chandef,
+				       IEEE80211_DFS_AVAILABLE);
+		break;
+	case NL80211_CAC_ABORTED:
+		/* Shouldn't happen if CAC was never started before. */
+		WARN_ON(!wdev->cac_started);
+		break;
+	default:
+		break;
+	}
+
+	wdev->cac_started = false;
+
+	nl80211_radar_notify(rdev, &wdev->preset_chandef, event, netdev, gfp);
+}
+EXPORT_SYMBOL(cfg80211_radar_event);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 33de803..2e0216a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4869,6 +4869,54 @@ static int nl80211_stop_sched_scan(struct sk_buff *skb,
 	return err;
 }
 
+static int nl80211_start_radar_detection(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;
+	struct cfg80211_chan_def chandef;
+	int err;
+
+	if (!(rdev->wiphy.features & NL80211_FEATURE_DFS))
+		return -EOPNOTSUPP;
+
+	err = nl80211_parse_chandef(rdev, info, &chandef);
+	if (err)
+		return err;
+
+	if (wdev->cac_started)
+		return -EBUSY;
+
+	err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef);
+	if (err < 1)
+		return err;
+
+	if (chandef.chan->dfs_state != IEEE80211_DFS_USABLE)
+		return -EINVAL;
+
+	if (!rdev->ops->start_radar_detection)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&rdev->devlist_mtx);
+	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
+					   chandef.chan, CHAN_MODE_SHARED,
+					   BIT(chandef.width));
+	mutex_unlock(&rdev->devlist_mtx);
+
+	if (err)
+		return err;
+
+	err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, &chandef);
+	if (!err) {
+		wdev->preset_chandef = chandef;
+		wdev->cac_started = true;
+		chandef.chan->dfs_state_entered = jiffies;
+	}
+
+	return err;
+}
+
 static int nl80211_send_bss(struct sk_buff *msg, struct netlink_callback *cb,
 			    u32 seq, int flags,
 			    struct cfg80211_registered_device *rdev,
@@ -5575,6 +5623,10 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
 	if (!cfg80211_reg_can_beacon(&rdev->wiphy, &ibss.chandef))
 		return -EINVAL;
 
+	if (cfg80211_chandef_usable(&rdev->wiphy, &ibss.chandef,
+				    IEEE80211_CHAN_NO_IBSS))
+		return -EINVAL;
+
 	if (ibss.chandef.width > NL80211_CHAN_WIDTH_40)
 		return -EINVAL;
 	if (ibss.chandef.width != NL80211_CHAN_WIDTH_20_NOHT &&
@@ -7876,6 +7928,14 @@ static struct genl_ops nl80211_ops[] = {
 		.internal_flags = NL80211_FLAG_NEED_NETDEV |
 				  NL80211_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL80211_CMD_RADAR_DETECT,
+		.doit = nl80211_start_radar_detection,
+		.policy = nl80211_policy,
+		.flags = GENL_ADMIN_PERM,
+		.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+				  NL80211_FLAG_NEED_RTNL,
+	},
 };
 
 static struct genl_multicast_group nl80211_mlme_mcgrp = {
@@ -9073,6 +9133,52 @@ nl80211_send_cqm_txe_notify(struct cfg80211_registered_device *rdev,
 }
 
 void
+nl80211_radar_notify(struct cfg80211_registered_device *rdev,
+		     struct cfg80211_chan_def *chandef,
+		     enum nl80211_radar_event event,
+		     struct net_device *netdev, gfp_t gfp)
+{
+	struct sk_buff *msg;
+	struct wireless_dev *wdev = netdev->ieee80211_ptr;
+	void *hdr;
+
+	msg = nlmsg_new(NLMSG_DEFAULT_SIZE, gfp);
+	if (!msg)
+		return;
+
+	hdr = nl80211hdr_put(msg, 0, 0, 0, NL80211_CMD_RADAR_DETECT);
+	if (!hdr) {
+		nlmsg_free(msg);
+		return;
+	}
+
+	if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
+	    nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex) ||
+	    nla_put_u64(msg, NL80211_ATTR_WDEV, wdev_id(wdev)))
+		goto nla_put_failure;
+
+	/* reason is unspecified, just notify that CAC has failed. */
+	if (nla_put_u8(msg, NL80211_ATTR_RADAR_EVENT, event))
+		goto nla_put_failure;
+
+	if (nl80211_send_chandef(msg, chandef))
+		goto nla_put_failure;
+
+	if (genlmsg_end(msg, hdr) < 0) {
+		nlmsg_free(msg);
+		return;
+	}
+
+	genlmsg_multicast_netns(wiphy_net(&rdev->wiphy), msg, 0,
+				nl80211_mlme_mcgrp.id, gfp);
+	return;
+
+ nla_put_failure:
+	genlmsg_cancel(msg, hdr);
+	nlmsg_free(msg);
+}
+
+void
 nl80211_send_cqm_pktloss_notify(struct cfg80211_registered_device *rdev,
 				struct net_device *netdev, const u8 *peer,
 				u32 num_packets, gfp_t gfp)
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index 2acba84..b061da4 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -108,6 +108,13 @@ nl80211_send_cqm_rssi_notify(struct cfg80211_registered_device *rdev,
 			     struct net_device *netdev,
 			     enum nl80211_cqm_rssi_threshold_event rssi_event,
 			     gfp_t gfp);
+
+void
+nl80211_radar_notify(struct cfg80211_registered_device *rdev,
+		     struct cfg80211_chan_def *chandef,
+		     enum nl80211_radar_event event,
+		     struct net_device *netdev, gfp_t gfp);
+
 void
 nl80211_send_cqm_pktloss_notify(struct cfg80211_registered_device *rdev,
 				struct net_device *netdev, const u8 *peer,
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index de02d63..2c441dc 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -884,6 +884,9 @@ static void handle_channel(struct wiphy *wiphy,
 		return;
 	}
 
+	chan->dfs_state = IEEE80211_DFS_USABLE;
+	chan->dfs_state_entered = jiffies;
+
 	chan->beacon_found = false;
 	chan->flags = flags | bw_flags | map_regdom_flags(reg_rule->flags);
 	chan->max_antenna_gain =
diff --git a/net/wireless/trace.h b/net/wireless/trace.h
index 2134576..3eadb34 100644
--- a/net/wireless/trace.h
+++ b/net/wireless/trace.h
@@ -2033,6 +2033,21 @@ TRACE_EVENT(cfg80211_reg_can_beacon,
 		  WIPHY_PR_ARG, CHAN_DEF_PR_ARG)
 );
 
+TRACE_EVENT(cfg80211_chandef_dfs_required,
+	TP_PROTO(struct wiphy *wiphy, struct cfg80211_chan_def *chandef),
+	TP_ARGS(wiphy, chandef),
+	TP_STRUCT__entry(
+		WIPHY_ENTRY
+		CHAN_DEF_ENTRY
+	),
+	TP_fast_assign(
+		WIPHY_ASSIGN;
+		CHAN_DEF_ASSIGN(chandef);
+	),
+	TP_printk(WIPHY_PR_FMT ", " CHAN_DEF_PR_FMT,
+		  WIPHY_PR_ARG, CHAN_DEF_PR_ARG)
+);
+
 TRACE_EVENT(cfg80211_ch_switch_notify,
 	TP_PROTO(struct net_device *netdev,
 		 struct cfg80211_chan_def *chandef),
@@ -2049,6 +2064,24 @@ TRACE_EVENT(cfg80211_ch_switch_notify,
 		  NETDEV_PR_ARG, CHAN_DEF_PR_ARG)
 );
 
+TRACE_EVENT(cfg80211_radar_event,
+	TP_PROTO(struct net_device *netdev, struct cfg80211_chan_def *chandef,
+		 enum nl80211_radar_event evt),
+	TP_ARGS(netdev, chandef, evt),
+	TP_STRUCT__entry(
+		NETDEV_ENTRY
+		CHAN_DEF_ENTRY
+		__field(enum nl80211_radar_event, evt)
+	),
+	TP_fast_assign(
+		NETDEV_ASSIGN;
+		CHAN_DEF_ASSIGN(chandef);
+		__entry->evt = evt;
+	),
+	TP_printk(NETDEV_PR_FMT ", " CHAN_DEF_PR_FMT " event: %d",
+		  NETDEV_PR_ARG, CHAN_DEF_PR_ARG, __entry->evt)
+);
+
 DECLARE_EVENT_CLASS(cfg80211_rx_evt,
 	TP_PROTO(struct net_device *netdev, const u8 *addr),
 	TP_ARGS(netdev, addr),
-- 
1.7.10.4

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Zefir Kurtisi]

On 01/29/2013 01:21 PM, Simon Wunderlich wrote:
> From: Victor Goldenshtein <victorg@ti.com>
> 
> [...]
>  
>  /**
> + * enum ieee80211_dfs_state - DFS states for channels
> + *
> + * Channel states used by the DFS code.
> + *
> + * @IEEE80211_DFS_USABLE: The channel can be used, but channel availability
> + *	check (CAC) must be performed before using it for AP or IBSS.
> + * @IEEE80211_DFS_UNAVAILABLE: A radar has been detected on this channel, it
> + *	is therefore marked as not available.
> + * @IEEE80211_DFS_AVAILABLE: The channel has been CAC checked and is available.
> + */
> +
> +enum ieee80211_dfs_state {
> +	IEEE80211_DFS_USABLE,
> +	IEEE80211_DFS_UNAVAILABLE,
> +	IEEE80211_DFS_AVAILABLE,
> +};
> +
Not sure if IEEE80211_DFS_UNKNOWN is not missing here, i.e. whether a channel that
never passed a CAC (or the CAC has been aborted) is always USABLE. Once I realized
why ETSI defined an UNKNOWN state, but forgot meanwhile - so maybe only relevant
for managed operation (like an UNKNOWN state can be overridden by external
information, whereas e.g. UNAVAILABLE can't).

Furthermore, is there a reason to define an additional wireless_dev.cac_started
flag vs. adding a IEEE80211_DFS_CAC state?

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 2474 bytes --]

Hey Zefir,

On Tue, Jan 29, 2013 at 02:48:53PM +0100, Zefir Kurtisi wrote:
> On 01/29/2013 01:21 PM, Simon Wunderlich wrote:
> > From: Victor Goldenshtein <victorg@ti.com>
> > 
> > [...]
> >  
> >  /**
> > + * enum ieee80211_dfs_state - DFS states for channels
> > + *
> > + * Channel states used by the DFS code.
> > + *
> > + * @IEEE80211_DFS_USABLE: The channel can be used, but channel availability
> > + *	check (CAC) must be performed before using it for AP or IBSS.
> > + * @IEEE80211_DFS_UNAVAILABLE: A radar has been detected on this channel, it
> > + *	is therefore marked as not available.
> > + * @IEEE80211_DFS_AVAILABLE: The channel has been CAC checked and is available.
> > + */
> > +
> > +enum ieee80211_dfs_state {
> > +	IEEE80211_DFS_USABLE,
> > +	IEEE80211_DFS_UNAVAILABLE,
> > +	IEEE80211_DFS_AVAILABLE,
> > +};
> > +
> Not sure if IEEE80211_DFS_UNKNOWN is not missing here, i.e. whether a channel that
> never passed a CAC (or the CAC has been aborted) is always USABLE. Once I realized
> why ETSI defined an UNKNOWN state, but forgot meanwhile - so maybe only relevant
> for managed operation (like an UNKNOWN state can be overridden by external
> information, whereas e.g. UNAVAILABLE can't).

I've seen the UNKNOWN state in your last mail and your last slides, but didn't see
a purpose for it. If you can remember what we need it for, please tell me. :) We could
reject changes for an UNAVAILABLE channel from outside access if this is required.
> 
> Furthermore, is there a reason to define an additional wireless_dev.cac_started
> flag vs. adding a IEEE80211_DFS_CAC state?

We have discussed that we want to move cac-information from channel into wdev, so I
just did that.

The idea was that a CAC can always be started, and state transition is only performed
after a succesful CAC. This simplifies the state machine and some corner cases. A
channel stays USABLE through the CAC, and is only changed to AVAILABLE after CAC is
completed. We also don't forbid checking the same channel on different WiFi modules,
if any of them completes the channel is changed to AVAILABLE.

The same goes for the OPERATING state (defined in ETSI at least) - I don't have this
as dfs state, because it is just the same as AVAILABLE plus the information that
we currently use it, and that we know in the driver. Adding the state would just
add management overhead with no practical gain, IMHO.

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Zefir Kurtisi]

On 01/29/2013 03:36 PM, Simon Wunderlich wrote:
> Hey Zefir,
> 
> On Tue, Jan 29, 2013 at 02:48:53PM +0100, Zefir Kurtisi wrote:
>> On 01/29/2013 01:21 PM, Simon Wunderlich wrote:
>>> From: Victor Goldenshtein <victorg@ti.com>
>>>
>>> [...]
>>>  
>>>  /**
>>> + * enum ieee80211_dfs_state - DFS states for channels
>>> + *
>>> + * Channel states used by the DFS code.
>>> + *
>>> + * @IEEE80211_DFS_USABLE: The channel can be used, but channel availability
>>> + *	check (CAC) must be performed before using it for AP or IBSS.
>>> + * @IEEE80211_DFS_UNAVAILABLE: A radar has been detected on this channel, it
>>> + *	is therefore marked as not available.
>>> + * @IEEE80211_DFS_AVAILABLE: The channel has been CAC checked and is available.
>>> + */
>>> +
>>> +enum ieee80211_dfs_state {
>>> +	IEEE80211_DFS_USABLE,
>>> +	IEEE80211_DFS_UNAVAILABLE,
>>> +	IEEE80211_DFS_AVAILABLE,
>>> +};
>>> +
>> Not sure if IEEE80211_DFS_UNKNOWN is not missing here, i.e. whether a channel that
>> never passed a CAC (or the CAC has been aborted) is always USABLE. Once I realized
>> why ETSI defined an UNKNOWN state, but forgot meanwhile - so maybe only relevant
>> for managed operation (like an UNKNOWN state can be overridden by external
>> information, whereas e.g. UNAVAILABLE can't).
> 
> I've seen the UNKNOWN state in your last mail and your last slides, but didn't see
> a purpose for it. If you can remember what we need it for, please tell me. :) We could
> reject changes for an UNAVAILABLE channel from outside access if this is required.

Double checked it and realized that ETSI removed that state between v1.5 and v1.7.
Before, it enabled the frequency manager to differentiate between untouched
channels and channels that passed a NOP (which is the only way to become USABLE).

>>
>> Furthermore, is there a reason to define an additional wireless_dev.cac_started
>> flag vs. adding a IEEE80211_DFS_CAC state?
> 
> We have discussed that we want to move cac-information from channel into wdev, so I
> just did that.
> 
Sorry, did not follow that discussion fully (due to my inability to follow).
Having a dedicated CAC state would provide a convenient way to directly retrieve
the state transition info, but I understand it makes no sense to have it redundant.

> The idea was that a CAC can always be started, and state transition is only performed
> after a succesful CAC. This simplifies the state machine and some corner cases. A
> channel stays USABLE through the CAC, and is only changed to AVAILABLE after CAC is
> completed. We also don't forbid checking the same channel on different WiFi modules,
> if any of them completes the channel is changed to AVAILABLE.
>
Really, a CAC is done whenever you switch to a DFS channel? I'd expect that
switching from an operating channel and back does not require a CAC (that's the
sole purpose of the AVAILABLE state and a centralized state handler).

> The same goes for the OPERATING state (defined in ETSI at least) - I don't have this
> as dfs state, because it is just the same as AVAILABLE plus the information that
> we currently use it, and that we know in the driver. Adding the state would just
> add management overhead with no practical gain, IMHO.
> 
Would there be a benefit of an OPERATING state on systems with multiple wiphys?

> Cheers,
> 	Simon
> 

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 3461 bytes --]

Hey Zefir,

> [...]
> >> Not sure if IEEE80211_DFS_UNKNOWN is not missing here, i.e. whether a channel that
> >> never passed a CAC (or the CAC has been aborted) is always USABLE. Once I realized
> >> why ETSI defined an UNKNOWN state, but forgot meanwhile - so maybe only relevant
> >> for managed operation (like an UNKNOWN state can be overridden by external
> >> information, whereas e.g. UNAVAILABLE can't).
> > 
> > I've seen the UNKNOWN state in your last mail and your last slides, but didn't see
> > a purpose for it. If you can remember what we need it for, please tell me. :) We could
> > reject changes for an UNAVAILABLE channel from outside access if this is required.
> 
> Double checked it and realized that ETSI removed that state between v1.5 and v1.7.
> Before, it enabled the frequency manager to differentiate between untouched
> channels and channels that passed a NOP (which is the only way to become USABLE).
> 

OK, good, then we can skip it. :)

> > The idea was that a CAC can always be started, and state transition is only performed
> > after a succesful CAC. This simplifies the state machine and some corner cases. A
> > channel stays USABLE through the CAC, and is only changed to AVAILABLE after CAC is
> > completed. We also don't forbid checking the same channel on different WiFi modules,
> > if any of them completes the channel is changed to AVAILABLE.
> >
> Really, a CAC is done whenever you switch to a DFS channel? I'd expect that
> switching from an operating channel and back does not require a CAC (that's the
> sole purpose of the AVAILABLE state and a centralized state handler).
> 

No, as you say: if you switch operating channel to another one which is available, both
channels stay available. You can CAC all channels after booting and switch between them
without doing any other CAC (at least as long as you don't sense a radar).


What I was trying to say: If the channel is USABLE before (this is also the initial state),
it is only set AVAILABLE after CAC has finished on this device. If the CAC is aborted, no
state transition is done.

If you have a second AP instance (when multiple interfaces are supported at one point, they
are not in the current version), it can also do start a CAC independently from the first AP.

I like to keep the interfaces as independent as possible, to prevent weird race conditions/interactions
between the interfaces which would be hard to debug. :)

> > The same goes for the OPERATING state (defined in ETSI at least) - I don't have this
> > as dfs state, because it is just the same as AVAILABLE plus the information that
> > we currently use it, and that we know in the driver. Adding the state would just
> > add management overhead with no practical gain, IMHO.
> > 
> Would there be a benefit of an OPERATING state on systems with multiple wiphys?
> 

I don't know - I've not looked up who creates the channels, but Johannes told me that different
wiphys might have independent channel structs, if I remember correctly.

If we have an OPERATING state we have to sync it over multiple wiphys and/or multiple interfaces
- for example, when an interface goes down we have to check all other interfaces if they still
use the channel before setting it back to AVAILABLE. Certainly doable, but bothersome.

I currently don't see any benefit, but maybe someone else does and can explain. :)

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Zefir Kurtisi]

On 01/30/2013 05:25 PM, Simon Wunderlich wrote:
> Hey Zefir,
> 
>> [...]
>>>> Not sure if IEEE80211_DFS_UNKNOWN is not missing here, i.e. whether a channel that
>>>> never passed a CAC (or the CAC has been aborted) is always USABLE. Once I realized
>>>> why ETSI defined an UNKNOWN state, but forgot meanwhile - so maybe only relevant
>>>> for managed operation (like an UNKNOWN state can be overridden by external
>>>> information, whereas e.g. UNAVAILABLE can't).
>>>
>>> I've seen the UNKNOWN state in your last mail and your last slides, but didn't see
>>> a purpose for it. If you can remember what we need it for, please tell me. :) We could
>>> reject changes for an UNAVAILABLE channel from outside access if this is required.
>>
>> Double checked it and realized that ETSI removed that state between v1.5 and v1.7.
>> Before, it enabled the frequency manager to differentiate between untouched
>> channels and channels that passed a NOP (which is the only way to become USABLE).
>>
> 
> OK, good, then we can skip it. :)
> 
>>> The idea was that a CAC can always be started, and state transition is only performed
>>> after a succesful CAC. This simplifies the state machine and some corner cases. A
>>> channel stays USABLE through the CAC, and is only changed to AVAILABLE after CAC is
>>> completed. We also don't forbid checking the same channel on different WiFi modules,
>>> if any of them completes the channel is changed to AVAILABLE.
>>>
>> Really, a CAC is done whenever you switch to a DFS channel? I'd expect that
>> switching from an operating channel and back does not require a CAC (that's the
>> sole purpose of the AVAILABLE state and a centralized state handler).
>>
> 
> No, as you say: if you switch operating channel to another one which is available, both
> channels stay available. You can CAC all channels after booting and switch between them
> without doing any other CAC (at least as long as you don't sense a radar).
> 
> 
> What I was trying to say: If the channel is USABLE before (this is also the initial state),
> it is only set AVAILABLE after CAC has finished on this device. If the CAC is aborted, no
> state transition is done.
> 
> If you have a second AP instance (when multiple interfaces are supported at one point, they
> are not in the current version), it can also do start a CAC independently from the first AP.
> 
> I like to keep the interfaces as independent as possible, to prevent weird race conditions/interactions
> between the interfaces which would be hard to debug. :)
> 
>>> The same goes for the OPERATING state (defined in ETSI at least) - I don't have this
>>> as dfs state, because it is just the same as AVAILABLE plus the information that
>>> we currently use it, and that we know in the driver. Adding the state would just
>>> add management overhead with no practical gain, IMHO.
>>>
>> Would there be a benefit of an OPERATING state on systems with multiple wiphys?
>>
> 
> I don't know - I've not looked up who creates the channels, but Johannes told me that different
> wiphys might have independent channel structs, if I remember correctly.
> 
> If we have an OPERATING state we have to sync it over multiple wiphys and/or multiple interfaces
> - for example, when an interface goes down we have to check all other interfaces if they still
> use the channel before setting it back to AVAILABLE. Certainly doable, but bothersome.
> 
> I currently don't see any benefit, but maybe someone else does and can explain. :)
> 
> Cheers,
> 	Simon
> 
Hey Simon,

all my comments were based on a centralized frequency manager I implemented for
our system (in user-space) and my assumption that its functionality should be
portable to mac80211, i.e. channel states being managed by reg.c and wdevs being
continuously notified on state changes.

Take a setup where you have two cards running, one as master and the other as
monitor doing off-channel-CAC. If the monitor detects radar on the master's
operating channel (and the master doesn't), with the current implementation the
master can't be notified to switch. I understand that supporting this very
specific use-case would add unjustifiable complexity, while at the same time, with
the updates you made this can be achieved at higher layers - so it is good enough
as it is.


Looking forward your patches to get integrated soon, to allow people actually
testing DFS. I'm eagerly waiting for more active testers to share some ugly
problems I am observing with ath9k's pulse detector. Therefore: full ACK.


Thanks,
Zefir

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 1967 bytes --]

Hey Zefir,

On Thu, Jan 31, 2013 at 09:52:33AM +0100, Zefir Kurtisi wrote:
> On 01/30/2013 05:25 PM, Simon Wunderlich wrote:
> Hey Simon,
> 
> all my comments were based on a centralized frequency manager I implemented for
> our system (in user-space) and my assumption that its functionality should be
> portable to mac80211, i.e. channel states being managed by reg.c and wdevs being
> continuously notified on state changes.
> 
> Take a setup where you have two cards running, one as master and the other as
> monitor doing off-channel-CAC. If the monitor detects radar on the master's
> operating channel (and the master doesn't), with the current implementation the
> master can't be notified to switch. I understand that supporting this very
> specific use-case would add unjustifiable complexity, while at the same time, with
> the updates you made this can be achieved at higher layers - so it is good enough
> as it is.

OK ... yeah I guess there are some things which can be handled in userspace. Interaction
between userspace programs will be a topic later anyways, think of say wpa_supplicant and
hostapd to handle IBSS and AP interface, one of them (or both?) receive a radar event and
now have to negotiate the next channel ... :)

> 
> 
> Looking forward your patches to get integrated soon, to allow people actually
> testing DFS. I'm eagerly waiting for more active testers to share some ugly
> problems I am observing with ath9k's pulse detector. Therefore: full ACK.

Thanks a lot! I've got quite a few mails of people offering their help to check
and verify this implementation or test it in their country. :) If you want to
help in the integration for ath9k, I'd be glad ... the interface should be
rather simple so maybe we don't have too much hassle? I have a little patch for
ath9k to add dummy radar support and simulate events etc as baseline, if you
(or anyone else) is interested ...

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Zefir Kurtisi]

Note: removed folks from CC to reduce noise
On 01/31/2013 06:54 PM, Simon Wunderlich wrote:
> Hey Zefir,
> [...]
> 
> Thanks a lot! I've got quite a few mails of people offering their help to check
> and verify this implementation or test it in their country. :) If you want to
> help in the integration for ath9k, I'd be glad ... the interface should be
> rather simple so maybe we don't have too much hassle? I have a little patch for
> ath9k to add dummy radar support and simulate events etc as baseline, if you
> (or anyone else) is interested ...
> 
I am currently bound to another project (interference detection based on
ath9k-spectral ;)), but since I started integration once long ago based on
Victor's initial interface proposal, I might take a look again (if noone else
wants to try).

Do you have the updated driver interface ready?

> Cheers,
> 	Simon
> 

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]

Hey Zefir,

On Fri, Feb 01, 2013 at 11:08:45AM +0100, Zefir Kurtisi wrote:
> Note: removed folks from CC to reduce noise
> On 01/31/2013 06:54 PM, Simon Wunderlich wrote:
> > Hey Zefir,
> > [...]
> > 
> > Thanks a lot! I've got quite a few mails of people offering their help to check
> > and verify this implementation or test it in their country. :) If you want to
> > help in the integration for ath9k, I'd be glad ... the interface should be
> > rather simple so maybe we don't have too much hassle? I have a little patch for
> > ath9k to add dummy radar support and simulate events etc as baseline, if you
> > (or anyone else) is interested ...
> > 
> I am currently bound to another project (interference detection based on
> ath9k-spectral ;)), but since I started integration once long ago based on
> Victor's initial interface proposal, I might take a look again (if noone else
> wants to try).
> 
> Do you have the updated driver interface ready?

As the nl80211/mac80211 part is merged now, I've sent a implementation reference
patch for ath9k [1]. It just adds a debug file and claims that it can support DFS,
obviously you have to enable radar detection and hook in your dfs pattern detector. :)

It would be very nice if you can give it a try. :)

Cheers,
	Simon

[1] http://permalink.gmane.org/gmane.linux.kernel.wireless.general/104056
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Johannes Berg]

On Tue, 2013-01-29 at 13:21 +0100, Simon Wunderlich wrote:

> From: Victor Goldenshtein <victorg@ti.com>

Probably time for you to claim ownership ... ;-)

> Changes to PATCHv6:

Thanks for your patience and the frequent posting :-)

>  /**
> + * enum ieee80211_dfs_state - DFS states for channels
> + *
> + * Channel states used by the DFS code.
> + *
> + * @IEEE80211_DFS_USABLE: The channel can be used, but channel availability
> + *	check (CAC) must be performed before using it for AP or IBSS.
> + * @IEEE80211_DFS_UNAVAILABLE: A radar has been detected on this channel, it
> + *	is therefore marked as not available.
> + * @IEEE80211_DFS_AVAILABLE: The channel has been CAC checked and is available.
> + */
> +
> +enum ieee80211_dfs_state {
> +	IEEE80211_DFS_USABLE,
> +	IEEE80211_DFS_UNAVAILABLE,
> +	IEEE80211_DFS_AVAILABLE,

Should UNAVAILABLE be = 0, so that's the default?

> @@ -133,6 +151,9 @@ enum ieee80211_channel_flags {
>   *	to enable this, this is useful only on 5 GHz band.
>   * @orig_mag: internal use
>   * @orig_mpwr: internal use
> + * @dfs_state: current state of this channel. Only relevant if radar is required
> + *	on this channel.
> + * @dfs_state_entered: time when the dfs state was entered.

This is relevant for "unavailable", presumably, to make sure it stays
there for long enough? What unit is that, and how long does it have to
stay? "jiffies" can become unreliable after a long uptime so that might
cause issues. It's unlikely, the issue would be that ~25 days after it
was supposed to be available again it would be rejected as jiffies got
back to the same value ... :)
Also depends on your implementation which I haven't seen yet.

> @@ -412,6 +435,16 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
>  			     u32 prohibited_flags);
>  
>  /**
> + * cfg80211_chandef_dfs_required - checks if radar detection
> + *	is required on any of the channels
> + * @wiphy: the wiphy to validate against
> + * @chandef: the channel definition to check
> + * Return: 1 if radar detection is required, 0 if it is not, < 0 on error
> + */
> +int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
> +				  const struct cfg80211_chan_def *c);

Why does that need to be exported to mac80211/drivers? Shouldn't
cfg80211 be able to check everything?

>  struct wireless_dev {
>  	struct wiphy *wiphy;
> @@ -2638,6 +2678,8 @@ struct wireless_dev {
>  
>  	u32 ap_unexpected_nlportid;
>  
> +	bool cac_started;

Don't you need a cac_chandef or something?

>  /**
> + * cfg80211_radar - radar detection event
> + * @dev: network device
> + * @chandef: chandef for the current channel

Doesn't cfg80211 know what channel the device is operating/doing CAC on?

> +#define NL80211_DFS_MIN_CAC_TIME_MS		60000
> +#define NL80211_DFS_MIN_NOP_TIME_MS		(30 * 60 * 1000)

Why are those needed in the userspace API?

> @@ -3221,6 +3240,7 @@ enum nl80211_feature_flags {
>  	NL80211_FEATURE_P2P_GO_CTWIN			= 1 << 11,
>  	NL80211_FEATURE_P2P_GO_OPPPS			= 1 << 12,
>  	NL80211_FEATURE_FULL_AP_CLIENT_STATE		= 1 << 13,
> +	NL80211_FEATURE_DFS				= 1 << 14,

I don't think we need this any more with the interface combinations
thing?

> +static void cfg80211_set_chans_dfs_state(struct wiphy *wiphy, u32 center_freq,
> +					 u32 bandwidth,
> +					 enum ieee80211_dfs_state dfs_state)
> +{
> +	struct ieee80211_channel *c;
> +	u32 freq;
> +
> +	for (freq = center_freq - bandwidth/2 + 10;
> +	     freq <= center_freq + bandwidth/2 - 10;
> +	     freq += 20) {
> +		c = ieee80211_get_channel(wiphy, freq);
> +		if (!c || !(c->flags & IEEE80211_CHAN_RADAR))
> +			continue;
> +
> +		c->dfs_state = dfs_state;
> +		c->dfs_state_entered = jiffies;

I wonder if it'd make sense to not skip if the radar flag isn't set,
that could be relevant with regdom changes? OTOH, if the regdom changes
(much) I *suspect* we need to invalidate all the radar measurements
anyway since the rules might now be different?

> +int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
> +				  const struct cfg80211_chan_def *chandef)
> +{
> +	u32 width;
> +	int r;
> +
> +	if (WARN_ON(!cfg80211_chandef_valid(chandef)))
> +		return -EINVAL;
> +
> +	width = cfg80211_chandef_get_width(chandef);
> +	if (width < 0)

never true with a u32, I think you might want to change the function
prototype and the variable :)

> @@ -344,7 +467,10 @@ cfg80211_get_chan_state(struct wireless_dev *wdev,
>  		break;
>  	case NL80211_IFTYPE_AP:
>  	case NL80211_IFTYPE_P2P_GO:
> -		if (wdev->beacon_interval) {
> +		if (wdev->cac_started) {
> +			*chan = wdev->preset_chandef.chan;
> +			*chanmode = CHAN_MODE_SHARED;

Ah, ok, so you're using the preset_chandef for CAC as well. I'm not
entirely sure that is correct though, since it could be changed by
userspace without much effect, e.g. by setting the channel with iw or
iwconfig? Unless you added "if (!cac_started)" there everywhere?

> +static inline void __cfg80211_dfs_clear_channel(struct ieee80211_channel *c,

no reason for inline

> +						bool *check_again,
> +						unsigned long *check_again_time)
> +{
> +	unsigned long timeout;
> +
> +	if (c->dfs_state == IEEE80211_DFS_UNAVAILABLE) {

could save on indentation by returning early if it's not unavailable :-)

I guess this addresses the jiffies concern I had above.

> +		timeout = c->dfs_state_entered + NL80211_DFS_MIN_NOP_TIME_MS;
> +		if (time_after_eq(jiffies, timeout)) {
> +			/* TODO: we could notify userspace about that change */
> +			c->dfs_state = IEEE80211_DFS_USABLE;
> +		} else {
> +			if (!*check_again)

should probably set it to false when the function starts, now you rely
on it being set outside which is a bit odd? (or did I just snip that
from my reply and don't see it any more?)

> +	err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef);
> +	if (err < 1)
> +		return err;

That doesn't make sense, if userspace starts CAC and that is successful
it would expect to eventually receive an event that it completed? Thus
if you return 0 here it would get confused, no?

> +	if (chandef.chan->dfs_state != IEEE80211_DFS_USABLE)
> +		return -EINVAL;
> +
> +	if (!rdev->ops->start_radar_detection)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&rdev->devlist_mtx);
> +	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
> +					   chandef.chan, CHAN_MODE_SHARED,
> +					   BIT(chandef.width));
> +	mutex_unlock(&rdev->devlist_mtx);

You need to keep holding the mutex until you've set cac_started to true
(or failed), otherwise you introduce races.

> +	if (err)
> +		return err;
> +
> +	err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, &chandef);
> +	if (!err) {
> +		wdev->preset_chandef = chandef;
> +		wdev->cac_started = true;
> +		chandef.chan->dfs_state_entered = jiffies;

No reason to assign dfs_state_entered since it won't be used in this
state anyway, will it?

> @@ -5575,6 +5623,10 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
>  	if (!cfg80211_reg_can_beacon(&rdev->wiphy, &ibss.chandef))
>  		return -EINVAL;
>  
> +	if (cfg80211_chandef_usable(&rdev->wiphy, &ibss.chandef,
> +				    IEEE80211_CHAN_NO_IBSS))
> +		return -EINVAL;

That seems like an unrelated change/fix?

> +	/* reason is unspecified, just notify that CAC has failed. */
> +	if (nla_put_u8(msg, NL80211_ATTR_RADAR_EVENT, event))

I think enums should generally be u32.

> +++ b/net/wireless/reg.c
> @@ -884,6 +884,9 @@ static void handle_channel(struct wiphy *wiphy,
>  		return;
>  	}
>  
> +	chan->dfs_state = IEEE80211_DFS_USABLE;
> +	chan->dfs_state_entered = jiffies;

Here also you don't really need the time assignment.

(I skipped this before, so pasting here)

> +void cfg80211_radar_event(struct net_device *netdev,
> +                         struct cfg80211_chan_def *chandef,
> +                         enum nl80211_radar_event event,
> +                         gfp_t gfp)
> +{
> +       struct wireless_dev *wdev = netdev->ieee80211_ptr;
> +       struct wiphy *wiphy = wdev->wiphy;
> +       struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
> +       unsigned long timeout;
> +
> +       if (WARN_ON(!cfg80211_chandef_valid(chandef)))
> +               return;
> +
> +       trace_cfg80211_radar_event(netdev, chandef, event);
> +
> +       switch (event) {
> +       case NL80211_RADAR_DETECTED:
> +               /*
> +                * only set the chandef supplied channel to unavailable, in
> +                * case the radar is detected on only one of multiple channels
> +                * spanned by the chandef.
> +                */
> +               cfg80211_set_dfs_state(wiphy, chandef,
> +                                      IEEE80211_DFS_UNAVAILABLE);
> +
> +               timeout = msecs_to_jiffies(NL80211_DFS_MIN_NOP_TIME_MS);
> +               queue_delayed_work(cfg80211_wq, &rdev->dfs_update_channels_wk,
> +                                  timeout);
> +               break;
> +       case NL80211_CAC_FINISHED:
> +               timeout = wdev->preset_chandef.chan->dfs_state_entered;
> +               timeout = msecs_to_jiffies(timeout +
> +                                          NL80211_DFS_MIN_CAC_TIME_MS);
> +               WARN_ON(!time_after_eq(jiffies, timeout));
> +               cfg80211_set_dfs_state(wiphy, &wdev->preset_chandef,
> +                                      IEEE80211_DFS_AVAILABLE);
> +               break;
> +       case NL80211_CAC_ABORTED:
> +               /* Shouldn't happen if CAC was never started before. */
> +               WARN_ON(!wdev->cac_started);
> +               break;
> +       default:
> +               break;

I think a warning (and maybe return) would be useful here.

> +       }
> +
> +       wdev->cac_started = false;


Whew. :)
Overall I think this is looking good, mostly minor comments.

johannes

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 14201 bytes --]

Hey Johannes,

thanks for the review!

On Thu, Jan 31, 2013 at 03:25:21PM +0100, Johannes Berg wrote:
> On Tue, 2013-01-29 at 13:21 +0100, Simon Wunderlich wrote:
> 
> > From: Victor Goldenshtein <victorg@ti.com>
> 
> Probably time for you to claim ownership ... ;-)
> 

maybe ... I've changed most of the original patch, actually. I'll keep
his name in the commit message though ..

> > Changes to PATCHv6:
> 
> Thanks for your patience and the frequent posting :-)
> 

Well, thanks for your patience ;)

> >  /**
> > + * enum ieee80211_dfs_state - DFS states for channels
> > + *
> > + * Channel states used by the DFS code.
> > + *
> > + * @IEEE80211_DFS_USABLE: The channel can be used, but channel availability
> > + *	check (CAC) must be performed before using it for AP or IBSS.
> > + * @IEEE80211_DFS_UNAVAILABLE: A radar has been detected on this channel, it
> > + *	is therefore marked as not available.
> > + * @IEEE80211_DFS_AVAILABLE: The channel has been CAC checked and is available.
> > + */
> > +
> > +enum ieee80211_dfs_state {
> > +	IEEE80211_DFS_USABLE,
> > +	IEEE80211_DFS_UNAVAILABLE,
> > +	IEEE80211_DFS_AVAILABLE,
> 
> Should UNAVAILABLE be = 0, so that's the default?
> 

yeah, good idea.


> > @@ -133,6 +151,9 @@ enum ieee80211_channel_flags {
> >   *	to enable this, this is useful only on 5 GHz band.
> >   * @orig_mag: internal use
> >   * @orig_mpwr: internal use
> > + * @dfs_state: current state of this channel. Only relevant if radar is required
> > + *	on this channel.
> > + * @dfs_state_entered: time when the dfs state was entered.
> 
> This is relevant for "unavailable", presumably, to make sure it stays
> there for long enough? What unit is that, and how long does it have to
> stay? "jiffies" can become unreliable after a long uptime so that might
> cause issues. It's unlikely, the issue would be that ~25 days after it
> was supposed to be available again it would be rejected as jiffies got
> back to the same value ... :)
> Also depends on your implementation which I haven't seen yet.
> 
Right, this is used for unavailable - The Non-Occupancy period (NOP) time
is 30 minutes, for FCC it should be the same. All times in DFS are < 24 hours,
and we check this time with our own timers in kernel, so I don't think we will
ever hit the jiffies overrun. I would prefer using jiffies because it's independent
of system time changes and enough for this task.

I'll add that the time is jiffies in documentation.

> > @@ -412,6 +435,16 @@ bool cfg80211_chandef_usable(struct wiphy *wiphy,
> >  			     u32 prohibited_flags);
> >  
> >  /**
> > + * cfg80211_chandef_dfs_required - checks if radar detection
> > + *	is required on any of the channels
> > + * @wiphy: the wiphy to validate against
> > + * @chandef: the channel definition to check
> > + * Return: 1 if radar detection is required, 0 if it is not, < 0 on error
> > + */
> > +int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
> > +				  const struct cfg80211_chan_def *c);
> 
> Why does that need to be exported to mac80211/drivers? Shouldn't
> cfg80211 be able to check everything?
> 
I'm using it in mac80211/ieee80211_start_ap() to find out whether radar detection
is required. We could put into struct cfg80211_ap_settings *params if you prefer?
I guess similar params exist for IBSS.

> >  struct wireless_dev {
> >  	struct wiphy *wiphy;
> > @@ -2638,6 +2678,8 @@ struct wireless_dev {
> >  
> >  	u32 ap_unexpected_nlportid;
> >  
> > +	bool cac_started;
> 
> Don't you need a cac_chandef or something?
> 

The chandef is stored into wdev->preset_chandef anyway, so I didn't see any
need to save it twice?

> >  /**
> > + * cfg80211_radar - radar detection event
> > + * @dev: network device
> > + * @chandef: chandef for the current channel
> 
> Doesn't cfg80211 know what channel the device is operating/doing CAC on?
> 

Whoops, typo in the description - forgot the _event :)

Anyway, the idea was that a driver can report a radar only for a certain part of
the currently used spectrum - e.g. we sense a radar on the extension channel in HT40+,
we would need to move but could still use the primary channel.

I don't know if this is overkill since we don't support any more than HT20 yet, but
that would be kind of future proof.


> > +#define NL80211_DFS_MIN_CAC_TIME_MS		60000
> > +#define NL80211_DFS_MIN_NOP_TIME_MS		(30 * 60 * 1000)
> 
> Why are those needed in the userspace API?
> 

Hm, not needed anymore I guess, it's just a nice reference ... will move it to other header
files (probably cfg80211.h) then.

> > @@ -3221,6 +3240,7 @@ enum nl80211_feature_flags {
> >  	NL80211_FEATURE_P2P_GO_CTWIN			= 1 << 11,
> >  	NL80211_FEATURE_P2P_GO_OPPPS			= 1 << 12,
> >  	NL80211_FEATURE_FULL_AP_CLIENT_STATE		= 1 << 13,
> > +	NL80211_FEATURE_DFS				= 1 << 14,
> 
> I don't think we need this any more with the interface combinations
> thing?
> 

Right, this is obsolete now. Will remove it.

> > +static void cfg80211_set_chans_dfs_state(struct wiphy *wiphy, u32 center_freq,
> > +					 u32 bandwidth,
> > +					 enum ieee80211_dfs_state dfs_state)
> > +{
> > +	struct ieee80211_channel *c;
> > +	u32 freq;
> > +
> > +	for (freq = center_freq - bandwidth/2 + 10;
> > +	     freq <= center_freq + bandwidth/2 - 10;
> > +	     freq += 20) {
> > +		c = ieee80211_get_channel(wiphy, freq);
> > +		if (!c || !(c->flags & IEEE80211_CHAN_RADAR))
> > +			continue;
> > +
> > +		c->dfs_state = dfs_state;
> > +		c->dfs_state_entered = jiffies;
> 
> I wonder if it'd make sense to not skip if the radar flag isn't set,
> that could be relevant with regdom changes? OTOH, if the regdom changes
> (much) I *suspect* we need to invalidate all the radar measurements
> anyway since the rules might now be different?
> 

IMHO the channels should be cleared on each regdom change. At least radar
patterns are different from FCC and ETSI (although I don't know if there is
a pattern in FCC which can be ignored in ETSI and vice versa). To be sure,
I would vote for complete reset.

> > +int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
> > +				  const struct cfg80211_chan_def *chandef)
> > +{
> > +	u32 width;
> > +	int r;
> > +
> > +	if (WARN_ON(!cfg80211_chandef_valid(chandef)))
> > +		return -EINVAL;
> > +
> > +	width = cfg80211_chandef_get_width(chandef);
> > +	if (width < 0)
> 
> never true with a u32, I think you might want to change the function
> prototype and the variable :)
> 

Whoops, right, thanks!

> > @@ -344,7 +467,10 @@ cfg80211_get_chan_state(struct wireless_dev *wdev,
> >  		break;
> >  	case NL80211_IFTYPE_AP:
> >  	case NL80211_IFTYPE_P2P_GO:
> > -		if (wdev->beacon_interval) {
> > +		if (wdev->cac_started) {
> > +			*chan = wdev->preset_chandef.chan;
> > +			*chanmode = CHAN_MODE_SHARED;
> 
> Ah, ok, so you're using the preset_chandef for CAC as well. I'm not
> entirely sure that is correct though, since it could be changed by
> userspace without much effect, e.g. by setting the channel with iw or
> iwconfig? Unless you added "if (!cac_started)" there everywhere?
> 

Hmm, actually I've tried setting the frequency with iw and got a EINVAL back.
I'll look into it again if I missed something, but thought it would be good to
not have this stuff redundant.

> > +static inline void __cfg80211_dfs_clear_channel(struct ieee80211_channel *c,
> 
> no reason for inline
> 
OK

> > +						bool *check_again,
> > +						unsigned long *check_again_time)
> > +{
> > +	unsigned long timeout;
> > +
> > +	if (c->dfs_state == IEEE80211_DFS_UNAVAILABLE) {
> 
> could save on indentation by returning early if it's not unavailable :-)

right, thanks.
> 
> I guess this addresses the jiffies concern I had above.
> 

OK - the state is entered via cfg80211_radar_event() when a radar is received.

> > +		timeout = c->dfs_state_entered + NL80211_DFS_MIN_NOP_TIME_MS;
> > +		if (time_after_eq(jiffies, timeout)) {
> > +			/* TODO: we could notify userspace about that change */
> > +			c->dfs_state = IEEE80211_DFS_USABLE;
> > +		} else {
> > +			if (!*check_again)
> 
> should probably set it to false when the function starts, now you rely
> on it being set outside which is a bit odd? (or did I just snip that
> from my reply and don't see it any more?)
> 

Actually I just have this function because I refactored
cfg80211_dfs_channels_update_work(), I had some trouble with the 80 characters
limit ... ;)

So yes, I rely on check_again set outside because I expect to only be used by
cfg80211_dfs_channels_update_work(). I'll try to write this nicer ...

> > +	err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef);
> > +	if (err < 1)
> > +		return err;
> 
> That doesn't make sense, if userspace starts CAC and that is successful
> it would expect to eventually receive an event that it completed? Thus
> if you return 0 here it would get confused, no?
> 

Ah yes, I should probably return EINVAL in this case, or the appropriate
error code otherwise ... 

> > +	if (chandef.chan->dfs_state != IEEE80211_DFS_USABLE)
> > +		return -EINVAL;
> > +
> > +	if (!rdev->ops->start_radar_detection)
> > +		return -EOPNOTSUPP;
> > +
> > +	mutex_lock(&rdev->devlist_mtx);
> > +	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
> > +					   chandef.chan, CHAN_MODE_SHARED,
> > +					   BIT(chandef.width));
> > +	mutex_unlock(&rdev->devlist_mtx);
> 
> You need to keep holding the mutex until you've set cac_started to true
> (or failed), otherwise you introduce races.
> 

OK

> > +	if (err)
> > +		return err;
> > +
> > +	err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, &chandef);
> > +	if (!err) {
> > +		wdev->preset_chandef = chandef;
> > +		wdev->cac_started = true;
> > +		chandef.chan->dfs_state_entered = jiffies;
> 
> No reason to assign dfs_state_entered since it won't be used in this
> state anyway, will it?
> 

Hmm ... yeah, that's stupid. I'll need to add wdev->cac_entered or something like
that to track CAC time. Using dfs_state_entered is just wrong.

Thanks.

> > @@ -5575,6 +5623,10 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
> >  	if (!cfg80211_reg_can_beacon(&rdev->wiphy, &ibss.chandef))
> >  		return -EINVAL;
> >  
> > +	if (cfg80211_chandef_usable(&rdev->wiphy, &ibss.chandef,
> > +				    IEEE80211_CHAN_NO_IBSS))
> > +		return -EINVAL;
> 
> That seems like an unrelated change/fix?
> 

Well, yeah, that is a survivor from some intermediate state that I forgot to remove.
I still have some confusion/questions regarding the other flags, there is a question
in the cover letter regarding this - we should discuss it there.

> > +	/* reason is unspecified, just notify that CAC has failed. */
> > +	if (nla_put_u8(msg, NL80211_ATTR_RADAR_EVENT, event))
> 
> I think enums should generally be u32.
> 

OK

> > +++ b/net/wireless/reg.c
> > @@ -884,6 +884,9 @@ static void handle_channel(struct wiphy *wiphy,
> >  		return;
> >  	}
> >  
> > +	chan->dfs_state = IEEE80211_DFS_USABLE;
> > +	chan->dfs_state_entered = jiffies;
> 
> Here also you don't really need the time assignment.
> 
> (I skipped this before, so pasting here)
> 

Hm, aren't channels initialized in this function? I wanted to set some
sane values here - although time is not relevant for the USABLE state,
I thought it might be useful if this info is exported to userspace or
for debugging.

> > +void cfg80211_radar_event(struct net_device *netdev,
> > +                         struct cfg80211_chan_def *chandef,
> > +                         enum nl80211_radar_event event,
> > +                         gfp_t gfp)
> > +{
> > +       struct wireless_dev *wdev = netdev->ieee80211_ptr;
> > +       struct wiphy *wiphy = wdev->wiphy;
> > +       struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
> > +       unsigned long timeout;
> > +
> > +       if (WARN_ON(!cfg80211_chandef_valid(chandef)))
> > +               return;
> > +
> > +       trace_cfg80211_radar_event(netdev, chandef, event);
> > +
> > +       switch (event) {
> > +       case NL80211_RADAR_DETECTED:
> > +               /*
> > +                * only set the chandef supplied channel to unavailable, in
> > +                * case the radar is detected on only one of multiple channels
> > +                * spanned by the chandef.
> > +                */
> > +               cfg80211_set_dfs_state(wiphy, chandef,
> > +                                      IEEE80211_DFS_UNAVAILABLE);
> > +
> > +               timeout = msecs_to_jiffies(NL80211_DFS_MIN_NOP_TIME_MS);
> > +               queue_delayed_work(cfg80211_wq, &rdev->dfs_update_channels_wk,
> > +                                  timeout);
> > +               break;
> > +       case NL80211_CAC_FINISHED:
> > +               timeout = wdev->preset_chandef.chan->dfs_state_entered;
> > +               timeout = msecs_to_jiffies(timeout +
> > +                                          NL80211_DFS_MIN_CAC_TIME_MS);
> > +               WARN_ON(!time_after_eq(jiffies, timeout));
> > +               cfg80211_set_dfs_state(wiphy, &wdev->preset_chandef,
> > +                                      IEEE80211_DFS_AVAILABLE);
> > +               break;
> > +       case NL80211_CAC_ABORTED:
> > +               /* Shouldn't happen if CAC was never started before. */
> > +               WARN_ON(!wdev->cac_started);
> > +               break;
> > +       default:
> > +               break;
> 
> I think a warning (and maybe return) would be useful here.
> 

OK

> > +       }
> > +
> > +       wdev->cac_started = false;
> 
> 
> Whew. :)
> Overall I think this is looking good, mostly minor comments.

I'm glad you say that! ;) Thanks as always for the comments. There
are a few questions in the cover letter I think we should discuss,
then I'll repost the patchset - hopefully quicker as most conceptional
changes have been included in this version already.

Cheers,
	Simon


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Johannes Berg]

On Thu, 2013-01-31 at 17:13 +0100, Simon Wunderlich wrote:

> > > +int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
> > > +				  const struct cfg80211_chan_def *c);
> > 
> > Why does that need to be exported to mac80211/drivers? Shouldn't
> > cfg80211 be able to check everything?
> > 
> I'm using it in mac80211/ieee80211_start_ap() to find out whether radar detection
> is required. We could put into struct cfg80211_ap_settings *params if you prefer?
> I guess similar params exist for IBSS.

I kinda think that would make more sense because then it's right there
when you look at the parameters, rather than having to remember it.

> > Don't you need a cac_chandef or something?

> The chandef is stored into wdev->preset_chandef anyway, so I didn't see any
> need to save it twice?

(I'm just worried about that changing, see below. We could use
"wdev->channel" though, which I need to change to chandef anyway)

> Anyway, the idea was that a driver can report a radar only for a certain part of
> the currently used spectrum - e.g. we sense a radar on the extension channel in HT40+,
> we would need to move but could still use the primary channel.
> 
> I don't know if this is overkill since we don't support any more than HT20 yet, but
> that would be kind of future proof.

I guess I'm not worried about internal APIs much. Does that even make
sense? Can you detect radar on one 20 MHz subchannel and then still use
the other subchannel?

> IMHO the channels should be cleared on each regdom change. At least radar
> patterns are different from FCC and ETSI (although I don't know if there is
> a pattern in FCC which can be ignored in ETSI and vice versa). To be sure,
> I would vote for complete reset.

Your call.

> > > @@ -344,7 +467,10 @@ cfg80211_get_chan_state(struct wireless_dev *wdev,
> > >  		break;
> > >  	case NL80211_IFTYPE_AP:
> > >  	case NL80211_IFTYPE_P2P_GO:
> > > -		if (wdev->beacon_interval) {
> > > +		if (wdev->cac_started) {
> > > +			*chan = wdev->preset_chandef.chan;
> > > +			*chanmode = CHAN_MODE_SHARED;
> > 
> > Ah, ok, so you're using the preset_chandef for CAC as well. I'm not
> > entirely sure that is correct though, since it could be changed by
> > userspace without much effect, e.g. by setting the channel with iw or
> > iwconfig? Unless you added "if (!cac_started)" there everywhere?
> > 
> 
> Hmm, actually I've tried setting the frequency with iw and got a EINVAL back.
> I'll look into it again if I missed something, but thought it would be good to
> not have this stuff redundant.

Ok. Hmm. EINVAL? Maybe you tried setting to a radar frequency or
something? Can you try setting to say channel 1? I don't think you
changed __nl80211_set_channel() to check cac_started, so ...

> > > +	err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef);
> > > +	if (err < 1)
> > > +		return err;
> > 
> > That doesn't make sense, if userspace starts CAC and that is successful
> > it would expect to eventually receive an event that it completed? Thus
> > if you return 0 here it would get confused, no?
> > 
> 
> Ah yes, I should probably return EINVAL in this case, or the appropriate
> error code otherwise ... 

Maybe return some more useful error code? Can't really find any one that
is appropriate though.

Do we export the state yet when you do try to get the channel list? That
would be helpful for userspace, particularly in this case, I think.

> > > +	err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, &chandef);
> > > +	if (!err) {
> > > +		wdev->preset_chandef = chandef;
> > > +		wdev->cac_started = true;
> > > +		chandef.chan->dfs_state_entered = jiffies;
> > 
> > No reason to assign dfs_state_entered since it won't be used in this
> > state anyway, will it?
> > 
> 
> Hmm ... yeah, that's stupid. I'll need to add wdev->cac_entered or something like
> that to track CAC time. Using dfs_state_entered is just wrong.

I didn't see you read that, or did I miss that?

> > > +	if (cfg80211_chandef_usable(&rdev->wiphy, &ibss.chandef,
> > > +				    IEEE80211_CHAN_NO_IBSS))
> > > +		return -EINVAL;
> > 
> > That seems like an unrelated change/fix?
> > 
> 
> Well, yeah, that is a survivor from some intermediate state that I forgot to remove.
> I still have some confusion/questions regarding the other flags, there is a question
> in the cover letter regarding this - we should discuss it there.

Oh I missed that, let me see.

> > > @@ -884,6 +884,9 @@ static void handle_channel(struct wiphy *wiphy,
> > >  		return;
> > >  	}
> > >  
> > > +	chan->dfs_state = IEEE80211_DFS_USABLE;
> > > +	chan->dfs_state_entered = jiffies;
> > 
> > Here also you don't really need the time assignment.
> > 
> > (I skipped this before, so pasting here)
> > 
> 
> Hm, aren't channels initialized in this function? I wanted to set some
> sane values here - although time is not relevant for the USABLE state,
> I thought it might be useful if this info is exported to userspace or
> for debugging.

Maybe so, I just don't think you need the time there since it won't be
of relevance in the USABLE state. The "state entered" time is only used
for UNAVAILABLE.

Maybe therefore state_entered should be renamed to "unavailable_until"
with the corresponding change in the logic of adding the time when it's
set to that state?

johannes

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 6655 bytes --]

On Thu, Jan 31, 2013 at 05:46:00PM +0100, Johannes Berg wrote:
> On Thu, 2013-01-31 at 17:13 +0100, Simon Wunderlich wrote:
> 
> > > > +int cfg80211_chandef_dfs_required(struct wiphy *wiphy,
> > > > +				  const struct cfg80211_chan_def *c);
> > > 
> > > Why does that need to be exported to mac80211/drivers? Shouldn't
> > > cfg80211 be able to check everything?
> > > 
> > I'm using it in mac80211/ieee80211_start_ap() to find out whether radar detection
> > is required. We could put into struct cfg80211_ap_settings *params if you prefer?
> > I guess similar params exist for IBSS.
> 
> I kinda think that would make more sense because then it's right there
> when you look at the parameters, rather than having to remember it.
> 

OK, will change that.

> > > Don't you need a cac_chandef or something?
> 
> > The chandef is stored into wdev->preset_chandef anyway, so I didn't see any
> > need to save it twice?
> 
> (I'm just worried about that changing, see below. We could use
> "wdev->channel" though, which I need to change to chandef anyway)
> 

I'll have a look at wdev->channel then. :)

> > Anyway, the idea was that a driver can report a radar only for a certain part of
> > the currently used spectrum - e.g. we sense a radar on the extension channel in HT40+,
> > we would need to move but could still use the primary channel.
> > 
> > I don't know if this is overkill since we don't support any more than HT20 yet, but
> > that would be kind of future proof.
> 
> I guess I'm not worried about internal APIs much. Does that even make
> sense? Can you detect radar on one 20 MHz subchannel and then still use
> the other subchannel?
> 

According to ETSI, that is possible. To quote from the EN 301 893 v1.7.1:

"If the master device has detected a radar signal on an Operating Channel during In-Service Monitoring, the
master device shall instruct all its associated slave devices to stop transmitting on this channel which becomes
an Unavailable Channel. For devices operating on multiple (adjacent or non-adjacent) Operating Channels
simultaneously, only the Operating Channel containing the frequency on which radar was detected shall
become an Unavailable Channel."

At least in ath9k it appears that the radar header contains some information whether
the radar was received on the primary or extension channel.

I don't know how upcoming 80 MHz devices handle that though.

We can remove it now and re-add it later if you prefer?

> > > > @@ -344,7 +467,10 @@ cfg80211_get_chan_state(struct wireless_dev *wdev,
> > > >  		break;
> > > >  	case NL80211_IFTYPE_AP:
> > > >  	case NL80211_IFTYPE_P2P_GO:
> > > > -		if (wdev->beacon_interval) {
> > > > +		if (wdev->cac_started) {
> > > > +			*chan = wdev->preset_chandef.chan;
> > > > +			*chanmode = CHAN_MODE_SHARED;
> > > 
> > > Ah, ok, so you're using the preset_chandef for CAC as well. I'm not
> > > entirely sure that is correct though, since it could be changed by
> > > userspace without much effect, e.g. by setting the channel with iw or
> > > iwconfig? Unless you added "if (!cac_started)" there everywhere?
> > > 
> > 
> > Hmm, actually I've tried setting the frequency with iw and got a EINVAL back.
> > I'll look into it again if I missed something, but thought it would be good to
> > not have this stuff redundant.
> 
> Ok. Hmm. EINVAL? Maybe you tried setting to a radar frequency or
> something? Can you try setting to say channel 1? I don't think you
> changed __nl80211_set_channel() to check cac_started, so ...
> 

I can try again ... but maybe this is obsolete when using wdev->channel.

> > > > +	err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef);
> > > > +	if (err < 1)
> > > > +		return err;
> > > 
> > > That doesn't make sense, if userspace starts CAC and that is successful
> > > it would expect to eventually receive an event that it completed? Thus
> > > if you return 0 here it would get confused, no?
> > > 
> > 
> > Ah yes, I should probably return EINVAL in this case, or the appropriate
> > error code otherwise ... 
> 
> Maybe return some more useful error code? Can't really find any one that
> is appropriate though.

We should add EUSELESS. :D
Can't think of anything better, so will keep it at EINVAL until someone has
a better idea.

> 
> Do we export the state yet when you do try to get the channel list? That
> would be helpful for userspace, particularly in this case, I think.
> 

No, not yet, I've just put that on my TODO.

> > > > +	err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, &chandef);
> > > > +	if (!err) {
> > > > +		wdev->preset_chandef = chandef;
> > > > +		wdev->cac_started = true;
> > > > +		chandef.chan->dfs_state_entered = jiffies;
> > > 
> > > No reason to assign dfs_state_entered since it won't be used in this
> > > state anyway, will it?
> > > 
> > 
> > Hmm ... yeah, that's stupid. I'll need to add wdev->cac_entered or something like
> > that to track CAC time. Using dfs_state_entered is just wrong.
> 
> I didn't see you read that, or did I miss that?
> 

chan->dfs_state_entered was read in cfg80211_radar_event() for the CAC_FINISHED event,
which only worked "by accident" because we set the state entered above. :) I'll change
this part too.

> [...]
> > > > @@ -884,6 +884,9 @@ static void handle_channel(struct wiphy *wiphy,
> > > >  		return;
> > > >  	}
> > > >  
> > > > +	chan->dfs_state = IEEE80211_DFS_USABLE;
> > > > +	chan->dfs_state_entered = jiffies;
> > > 
> > > Here also you don't really need the time assignment.
> > > 
> > > (I skipped this before, so pasting here)
> > > 
> > 
> > Hm, aren't channels initialized in this function? I wanted to set some
> > sane values here - although time is not relevant for the USABLE state,
> > I thought it might be useful if this info is exported to userspace or
> > for debugging.
> 
> Maybe so, I just don't think you need the time there since it won't be
> of relevance in the USABLE state. The "state entered" time is only used
> for UNAVAILABLE.
> 
> Maybe therefore state_entered should be renamed to "unavailable_until"
> with the corresponding change in the logic of adding the time when it's
> set to that state?

Can do that, if no one is interested in when we, say, change from unavailable
to usable (after NOP). This is what Zefir asked for. 

Personally I don't care at all, and we discuss that in the cover letter thread
anyway. Let's see what Zefir says or if anyone else objects, I put that onto
the "TODO if nobody objects list". :)

Thanks!
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Zefir Kurtisi]

On 01/31/2013 06:44 PM, Simon Wunderlich wrote:
> On Thu, Jan 31, 2013 at 05:46:00PM +0100, Johannes Berg wrote:
>> On Thu, 2013-01-31 at 17:13 +0100, Simon Wunderlich wrote:
>> [...]
>>
>> Maybe so, I just don't think you need the time there since it won't be
>> of relevance in the USABLE state. The "state entered" time is only used
>> for UNAVAILABLE.
>>
>> Maybe therefore state_entered should be renamed to "unavailable_until"
>> with the corresponding change in the logic of adding the time when it's
>> set to that state?
> 
> Can do that, if no one is interested in when we, say, change from unavailable
> to usable (after NOP). This is what Zefir asked for. 
> 
My assumption was that the user (or the certification lab engineer) might want to
know how long the device is going to remain in CAC. And since a time-stamp must be
kept anyhow for NOP, with the (channel_state / ts) tuple everything needed to
track the state transition is already there. Otherwise, it is not needed.

> Personally I don't care at all, and we discuss that in the cover letter thread
> anyway. Let's see what Zefir says or if anyone else objects, I put that onto
> the "TODO if nobody objects list". :)
> 
No objections here. The updated concept makes my work already way easier to
handle, and since I need to implement a frequency manager / planner anyhow on
higher layers, there's no need to consider my comments unless they are relevant
for stand-alone operation.

> Thanks!
> 	Simon
> 

Re: [PATCHv7 1/3] nl80211/cfg80211: add radar detection command/event

[Johannes Berg]

On Thu, 2013-01-31 at 18:44 +0100, Simon Wunderlich wrote:

> "If the master device has detected a radar signal on an Operating Channel during In-Service Monitoring, the
> master device shall instruct all its associated slave devices to stop transmitting on this channel which becomes
> an Unavailable Channel. For devices operating on multiple (adjacent or non-adjacent) Operating Channels
> simultaneously, only the Operating Channel containing the frequency on which radar was detected shall
> become an Unavailable Channel."
> 
> At least in ath9k it appears that the radar header contains some information whether
> the radar was received on the primary or extension channel.
> 
> I don't know how upcoming 80 MHz devices handle that though.
> 
> We can remove it now and re-add it later if you prefer?

No no, it's fine, I was just wondering.

> > > Hmm, actually I've tried setting the frequency with iw and got a EINVAL back.
> > > I'll look into it again if I missed something, but thought it would be good to
> > > not have this stuff redundant.
> > 
> > Ok. Hmm. EINVAL? Maybe you tried setting to a radar frequency or
> > something? Can you try setting to say channel 1? I don't think you
> > changed __nl80211_set_channel() to check cac_started, so ...
> > 
> 
> I can try again ... but maybe this is obsolete when using wdev->channel.

Yes, then you'd not have the problem.

> > > > > +	err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef);
> > > > > +	if (err < 1)
> > > > > +		return err;
> > > > 
> > > > That doesn't make sense, if userspace starts CAC and that is successful
> > > > it would expect to eventually receive an event that it completed? Thus
> > > > if you return 0 here it would get confused, no?
> > > > 
> > > 
> > > Ah yes, I should probably return EINVAL in this case, or the appropriate
> > > error code otherwise ... 
> > 
> > Maybe return some more useful error code? Can't really find any one that
> > is appropriate though.
> 
> We should add EUSELESS. :D
> Can't think of anything better, so will keep it at EINVAL until someone has
> a better idea.

:)

> > [...]
> > > > > @@ -884,6 +884,9 @@ static void handle_channel(struct wiphy *wiphy,
> > > > >  		return;
> > > > >  	}
> > > > >  
> > > > > +	chan->dfs_state = IEEE80211_DFS_USABLE;
> > > > > +	chan->dfs_state_entered = jiffies;
> > > > 
> > > > Here also you don't really need the time assignment.
> > > > 
> > > > (I skipped this before, so pasting here)
> > > > 
> > > 
> > > Hm, aren't channels initialized in this function? I wanted to set some
> > > sane values here - although time is not relevant for the USABLE state,
> > > I thought it might be useful if this info is exported to userspace or
> > > for debugging.
> > 
> > Maybe so, I just don't think you need the time there since it won't be
> > of relevance in the USABLE state. The "state entered" time is only used
> > for UNAVAILABLE.
> > 
> > Maybe therefore state_entered should be renamed to "unavailable_until"
> > with the corresponding change in the logic of adding the time when it's
> > set to that state?
> 
> Can do that, if no one is interested in when we, say, change from unavailable
> to usable (after NOP). This is what Zefir asked for. 
> 
> Personally I don't care at all, and we discuss that in the cover letter thread
> anyway. Let's see what Zefir says or if anyone else objects, I put that onto
> the "TODO if nobody objects list". :)

Heh. Ok I see Zefir's argument, so I guess that's reasonable. I just
didn't see a use for it in the kernel so was wondering.

johannes

[PATCHv7 2/3] mac80211: add radar detection command/event

[Simon Wunderlich]

From: Victor Goldenshtein <victorg@ti.com>

Add command to trigger radar detection in the driver/FW.
Once radar detection is started it should continuously
monitor for radars as long as the channel active.
If radar is detected usermode notified with 'radar
detected' event.

Scanning and remain on channel functionality must be disabled
while doing radar detection/scanning, and vice versa.

Signed-off-by: Victor Goldenshtein <victorg@ti.com>
[siwu@hrz.tu-chemnitz.de:
	rebase, change to chandef, interaction with scan/roc,
	add CAC timer, various fixes]
Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
Changes to PATCHv6:
 * squash "check radar interaction with scan and roc" patch
 * add radar detection flag to chanctx conf
 * remove mac80211 radar detection command for drivers
 * remove trace.h chandef macros and drv_start_radar_detection command
 * use chandef in radar_detected_notification, remove channel argument
   (mac80211 already knows it)
 * add timer for channel availability check notification to cfg80211
 * add vif channel release
 * check for dfs required using cfg80211 function

Changes to PATCHv5:
 * use local instead of sdata->local
 * change API to chandef
---
 include/net/mac80211.h     |   18 +++++++++++++++++-
 net/mac80211/cfg.c         |   43 ++++++++++++++++++++++++++++++++++++++++++-
 net/mac80211/chan.c        |   31 +++++++++++++++++++++++++++++++
 net/mac80211/ieee80211_i.h |   11 +++++++++++
 net/mac80211/iface.c       |   12 ++++++++++++
 net/mac80211/mlme.c        |   32 ++++++++++++++++++++++++++++++++
 net/mac80211/scan.c        |    3 +++
 net/mac80211/trace.h       |   19 +++++++++++++++++++
 8 files changed, 167 insertions(+), 2 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 3037f49..838423b 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -147,10 +147,12 @@ struct ieee80211_low_level_stats {
  * enum ieee80211_chanctx_change - change flag for channel context
  * @IEEE80211_CHANCTX_CHANGE_WIDTH: The channel width changed
  * @IEEE80211_CHANCTX_CHANGE_RX_CHAINS: The number of RX chains changed
+ * @IEEE80211_CHANCTX_CHANGE_RADAR: radar detection flag changed
  */
 enum ieee80211_chanctx_change {
 	IEEE80211_CHANCTX_CHANGE_WIDTH		= BIT(0),
 	IEEE80211_CHANCTX_CHANGE_RX_CHAINS	= BIT(1),
+	IEEE80211_CHANCTX_CHANGE_RADAR		= BIT(2),
 };
 
 /**
@@ -165,6 +167,7 @@ enum ieee80211_chanctx_change {
  * @rx_chains_dynamic: The number of RX chains that must be enabled
  *	after RTS/CTS handshake to receive SMPS MIMO transmissions;
  *	this will always be >= @rx_chains_static.
+ * @radar_enabled: whether radar detection is enabled on this channel.
  * @drv_priv: data area for driver use, will always be aligned to
  *	sizeof(void *), size is determined in hw information.
  */
@@ -173,6 +176,8 @@ struct ieee80211_chanctx_conf {
 
 	u8 rx_chains_static, rx_chains_dynamic;
 
+	bool radar_enabled;
+
 	u8 drv_priv[0] __aligned(sizeof(void *));
 };
 
@@ -953,6 +958,7 @@ enum ieee80211_smps_mode {
  *
  * @channel: the channel to tune to
  * @channel_type: the channel (HT) type
+ * @radar_enabled: whether radar detection is enabled on this channel
  *
  * @long_frame_max_tx_count: Maximum number of transmissions for a "long" frame
  *    (a frame not RTS protected), called "dot11LongRetryLimit" in 802.11,
@@ -979,6 +985,7 @@ struct ieee80211_conf {
 
 	struct ieee80211_channel *channel;
 	enum nl80211_channel_type channel_type;
+	bool radar_enabled;
 	enum ieee80211_smps_mode smps_mode;
 };
 
@@ -2080,7 +2087,7 @@ enum ieee80211_frame_release_type {
  * @IEEE80211_RC_SMPS_CHANGED: The SMPS state of the station changed.
  * @IEEE80211_RC_SUPP_RATES_CHANGED: The supported rate set of this peer
  *	changed (in IBSS mode) due to discovering more information about
- *	the peer.
+ *	the peern.
  */
 enum ieee80211_rate_control_changed {
 	IEEE80211_RC_BW_CHANGED		= BIT(0),
@@ -3936,6 +3943,15 @@ void ieee80211_cqm_rssi_notify(struct ieee80211_vif *vif,
 			       gfp_t gfp);
 
 /**
+ * ieee80211_radar_detected - inform a configured connection that
+ * radar was detected on the current channel
+ *
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ * @gfp: context flags.
+ */
+void ieee80211_radar_detected(struct ieee80211_vif *vif, gfp_t gfp);
+
+/**
  * ieee80211_chswitch_done - Complete channel switch process
  * @vif: &struct ieee80211_vif pointer from the add_interface callback.
  * @success: make the channel switch successful or not
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 661b878..eede26b 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -922,6 +922,10 @@ static int ieee80211_start_ap(struct wiphy *wiphy, struct net_device *dev,
 	/* TODO: make hostapd tell us what it wants */
 	sdata->smps_mode = IEEE80211_SMPS_OFF;
 	sdata->needed_rx_chains = sdata->local->rx_chains;
+	err = cfg80211_chandef_dfs_required(wiphy, &params->chandef);
+	if (err < 0)
+		return err;
+	sdata->radar_required = !!err;
 
 	err = ieee80211_vif_use_channel(sdata, &params->chandef,
 					IEEE80211_CHANCTX_SHARED);
@@ -1796,6 +1800,7 @@ static int ieee80211_join_mesh(struct wiphy *wiphy, struct net_device *dev,
 	/* can mesh use other SMPS modes? */
 	sdata->smps_mode = IEEE80211_SMPS_OFF;
 	sdata->needed_rx_chains = sdata->local->rx_chains;
+	sdata->radar_required = true;
 
 	err = ieee80211_vif_use_channel(sdata, &setup->chandef,
 					IEEE80211_CHANCTX_SHARED);
@@ -2368,7 +2373,9 @@ static int ieee80211_start_roc_work(struct ieee80211_local *local,
 	INIT_LIST_HEAD(&roc->dependents);
 
 	/* if there's one pending or we're scanning, queue this one */
-	if (!list_empty(&local->roc_list) || local->scanning)
+	if (!list_empty(&local->roc_list) ||
+	    local->scanning ||
+	    local->radar_detect_enabled)
 		goto out_check_combine;
 
 	/* if not HW assist, just queue & schedule work */
@@ -2618,6 +2625,39 @@ static int ieee80211_cancel_remain_on_channel(struct wiphy *wiphy,
 	return ieee80211_cancel_roc(local, cookie, false);
 }
 
+static int ieee80211_start_radar_detection(struct wiphy *wiphy,
+					   struct net_device *dev,
+					   struct cfg80211_chan_def *chandef)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct ieee80211_local *local = sdata->local;
+	unsigned long timeout;
+	int res;
+
+	if (!list_empty(&local->roc_list) || local->scanning)
+		return -EBUSY;
+
+	/* whatever, but channel contexts should not complain about that one */
+	sdata->smps_mode = IEEE80211_SMPS_OFF;
+	sdata->needed_rx_chains = local->rx_chains;
+	sdata->radar_required = true;
+
+	res = ieee80211_vif_use_channel(sdata, chandef,
+					IEEE80211_CHANCTX_SHARED);
+	if (res)
+		return -EBUSY;
+
+	/* TODO: some channels (e.g. ETSI weather channels between 5.600 and
+	 * 5.6500 require longer waiting time, if we want to support them at
+	 * all.
+	 */
+	timeout = msecs_to_jiffies(NL80211_DFS_MIN_CAC_TIME_MS);
+	ieee80211_queue_delayed_work(&sdata->local->hw,
+				     &sdata->dfs_cac_timer_work, timeout);
+
+	return 0;
+}
+
 static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 			     struct ieee80211_channel *chan, bool offchan,
 			     unsigned int wait, const u8 *buf, size_t len,
@@ -3322,4 +3362,5 @@ struct cfg80211_ops mac80211_config_ops = {
 	.get_et_stats = ieee80211_get_et_stats,
 	.get_et_strings = ieee80211_get_et_strings,
 	.get_channel = ieee80211_cfg_get_channel,
+	.start_radar_detection = ieee80211_start_radar_detection,
 };
diff --git a/net/mac80211/chan.c b/net/mac80211/chan.c
index 1bfe0a8..8de8b28 100644
--- a/net/mac80211/chan.c
+++ b/net/mac80211/chan.c
@@ -180,6 +180,7 @@ static void ieee80211_unassign_vif_chanctx(struct ieee80211_sub_if_data *sdata,
 	if (ctx->refcount > 0) {
 		ieee80211_recalc_chanctx_chantype(sdata->local, ctx);
 		ieee80211_recalc_smps_chanctx(local, ctx);
+		ieee80211_recalc_radar_chanctx(local, ctx);
 	}
 }
 
@@ -212,6 +213,35 @@ static void __ieee80211_vif_release_channel(struct ieee80211_sub_if_data *sdata)
 		ieee80211_free_chanctx(local, ctx);
 }
 
+void ieee80211_recalc_radar_chanctx(struct ieee80211_local *local,
+				    struct ieee80211_chanctx *chanctx)
+{
+	struct ieee80211_sub_if_data *sdata;
+	bool radar_enabled = false;
+
+	lockdep_assert_held(&local->chanctx_mtx);
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+		if (sdata->radar_required)
+			radar_enabled = true;
+	}
+	rcu_read_unlock();
+
+	if (radar_enabled == chanctx->conf.radar_enabled)
+		return;
+
+	chanctx->conf.radar_enabled = radar_enabled;
+	local->radar_detect_enabled = chanctx->conf.radar_enabled;
+
+	if (!local->use_chanctx) {
+		local->hw.conf.radar_enabled = chanctx->conf.radar_enabled;
+		ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_CHANNEL);
+	}
+
+	drv_change_chanctx(local, chanctx, IEEE80211_CHANCTX_CHANGE_RADAR);
+}
+
 void ieee80211_recalc_smps_chanctx(struct ieee80211_local *local,
 				   struct ieee80211_chanctx *chanctx)
 {
@@ -336,6 +366,7 @@ int ieee80211_vif_use_channel(struct ieee80211_sub_if_data *sdata,
 	}
 
 	ieee80211_recalc_smps_chanctx(local, ctx);
+	ieee80211_recalc_radar_chanctx(local, ctx);
  out:
 	mutex_unlock(&local->chanctx_mtx);
 	return ret;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 63f0430..e3960c7 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -755,6 +755,9 @@ struct ieee80211_sub_if_data {
 	int user_power_level; /* in dBm */
 	int ap_power_level; /* in dBm */
 
+	bool radar_required;
+	struct delayed_work dfs_cac_timer_work;
+
 	/*
 	 * AP this belongs to: self in AP mode and
 	 * corresponding AP in VLAN mode, NULL for
@@ -974,6 +977,9 @@ struct ieee80211_local {
 	/* wowlan is enabled -- don't reconfig on resume */
 	bool wowlan;
 
+	/* DFS/radar detection is enabled */
+	bool radar_detect_enabled;
+
 	/* number of RX chains the hardware has */
 	u8 rx_chains;
 
@@ -1640,6 +1646,11 @@ void ieee80211_vif_vlan_copy_chanctx(struct ieee80211_sub_if_data *sdata);
 
 void ieee80211_recalc_smps_chanctx(struct ieee80211_local *local,
 				   struct ieee80211_chanctx *chanctx);
+void ieee80211_recalc_radar_chanctx(struct ieee80211_local *local,
+				    struct ieee80211_chanctx *chanctx);
+
+void ieee80211_dfs_cac_timer(unsigned long data);
+void ieee80211_dfs_cac_timer_work(struct work_struct *work);
 
 #ifdef CONFIG_MAC80211_NOINLINE
 #define debug_noinline noinline
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 06fac29..ed878ce 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -817,6 +817,15 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 
 	cancel_work_sync(&sdata->recalc_smps);
 
+	cancel_delayed_work_sync(&sdata->dfs_cac_timer_work);
+
+	/* only inform about abort cac if it was started before. */
+	if (sdata->wdev.cac_started) {
+		ieee80211_vif_release_channel(sdata);
+		cfg80211_radar_event(sdata->dev, &sdata->wdev.preset_chandef,
+				     NL80211_CAC_ABORTED, GFP_KERNEL);
+	}
+
 	/* APs need special treatment */
 	if (sdata->vif.type == NL80211_IFTYPE_AP) {
 		struct ieee80211_sub_if_data *vlan, *tmpsdata;
@@ -1586,6 +1595,9 @@ int ieee80211_if_add(struct ieee80211_local *local, const char *name,
 	spin_lock_init(&sdata->cleanup_stations_lock);
 	INIT_LIST_HEAD(&sdata->cleanup_stations);
 	INIT_WORK(&sdata->cleanup_stations_wk, ieee80211_cleanup_sdata_stas_wk);
+	INIT_DELAYED_WORK(&sdata->dfs_cac_timer_work,
+			  ieee80211_dfs_cac_timer_work);
+
 
 	for (i = 0; i < IEEE80211_NUM_BANDS; i++) {
 		struct ieee80211_supported_band *sband;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index e930175..a006149 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -1244,6 +1244,21 @@ void ieee80211_dynamic_ps_timer(unsigned long data)
 	ieee80211_queue_work(&local->hw, &local->dynamic_ps_enable_work);
 }
 
+void ieee80211_dfs_cac_timer_work(struct work_struct *work)
+{
+	struct delayed_work *delayed_work =
+		container_of(work, struct delayed_work, work);
+	struct ieee80211_sub_if_data *sdata =
+		container_of(delayed_work, struct ieee80211_sub_if_data,
+			     dfs_cac_timer_work);
+
+	rtnl_lock();
+	ieee80211_vif_release_channel(sdata);
+	cfg80211_radar_event(sdata->dev, &sdata->vif.bss_conf.chandef,
+			     NL80211_CAC_FINISHED, GFP_KERNEL);
+	rtnl_unlock();
+}
+
 /* MLME */
 static bool ieee80211_sta_wmm_params(struct ieee80211_local *local,
 				     struct ieee80211_sub_if_data *sdata,
@@ -4131,3 +4146,20 @@ void ieee80211_cqm_rssi_notify(struct ieee80211_vif *vif,
 	cfg80211_cqm_rssi_notify(sdata->dev, rssi_event, gfp);
 }
 EXPORT_SYMBOL(ieee80211_cqm_rssi_notify);
+
+void ieee80211_radar_detected(struct ieee80211_vif *vif, gfp_t gfp)
+{
+	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+
+	trace_api_radar_detected(sdata);
+
+	/* may happen to devices which have been added but are not up */
+	if (!cfg80211_chandef_valid(&sdata->vif.bss_conf.chandef))
+		return;
+
+	cancel_delayed_work_sync(&sdata->dfs_cac_timer_work);
+
+	cfg80211_radar_event(sdata->dev, &sdata->vif.bss_conf.chandef,
+			     NL80211_RADAR_DETECTED, gfp);
+}
+EXPORT_SYMBOL(ieee80211_radar_detected);
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 607684c..100ab2d 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -356,6 +356,9 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local)
 static bool ieee80211_can_scan(struct ieee80211_local *local,
 			       struct ieee80211_sub_if_data *sdata)
 {
+	if (local->radar_detect_enabled)
+		return false;
+
 	if (!list_empty(&local->roc_list))
 		return false;
 
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 41861b9..c5dc8db 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -1821,6 +1821,25 @@ TRACE_EVENT(stop_queue,
 	)
 );
 
+TRACE_EVENT(api_radar_detected,
+	TP_PROTO(struct ieee80211_sub_if_data *sdata),
+
+	TP_ARGS(sdata),
+
+	TP_STRUCT__entry(
+		VIF_ENTRY
+	),
+
+	TP_fast_assign(
+		VIF_ASSIGN;
+	),
+
+	TP_printk(
+		VIF_PR_FMT " radar detected",
+		VIF_PR_ARG
+	)
+);
+
 #ifdef CONFIG_MAC80211_MESSAGE_TRACING
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM mac80211_msg
-- 
1.7.10.4

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Zefir Kurtisi]

On 01/29/2013 01:21 PM, Simon Wunderlich wrote:
> From: Victor Goldenshtein <victorg@ti.com>
> 
> [...]
> @@ -2080,7 +2087,7 @@ enum ieee80211_frame_release_type {
>   * @IEEE80211_RC_SMPS_CHANGED: The SMPS state of the station changed.
>   * @IEEE80211_RC_SUPP_RATES_CHANGED: The supported rate set of this peer
>   *	changed (in IBSS mode) due to discovering more information about
> - *	the peer.
> + *	the peern.
>   */
typo?
> [...]
> +	/* TODO: some channels (e.g. ETSI weather channels between 5.600 and
> +	 * 5.6500 require longer waiting time, if we want to support them at
> +	 * all.
> +	 */
> +	timeout = msecs_to_jiffies(NL80211_DFS_MIN_CAC_TIME_MS);
> +	ieee80211_queue_delayed_work(&sdata->local->hw,
> +				     &sdata->dfs_cac_timer_work, timeout);
> +
> +	return 0;
> +}
> +
The required minimum detection probability for weather channels is purposely
defined such that it can't be reached, so it is safe to assume they will never be
supported. As for CAC and NOP timings, they are luckily same for ETSI and FCC
right now, and if they differ in the future, we have the countrycode's DFS
regulatory domain available to differentiate.

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 1516 bytes --]

Hey,

On Tue, Jan 29, 2013 at 02:26:42PM +0100, Zefir Kurtisi wrote:
> On 01/29/2013 01:21 PM, Simon Wunderlich wrote:
> > From: Victor Goldenshtein <victorg@ti.com>
> > 
> > [...]
> > @@ -2080,7 +2087,7 @@ enum ieee80211_frame_release_type {
> >   * @IEEE80211_RC_SMPS_CHANGED: The SMPS state of the station changed.
> >   * @IEEE80211_RC_SUPP_RATES_CHANGED: The supported rate set of this peer
> >   *	changed (in IBSS mode) due to discovering more information about
> > - *	the peer.
> > + *	the peern.
> >   */
> typo?

yup ... sorry about that.

> > [...]
> > +	/* TODO: some channels (e.g. ETSI weather channels between 5.600 and
> > +	 * 5.6500 require longer waiting time, if we want to support them at
> > +	 * all.
> > +	 */
> > +	timeout = msecs_to_jiffies(NL80211_DFS_MIN_CAC_TIME_MS);
> > +	ieee80211_queue_delayed_work(&sdata->local->hw,
> > +				     &sdata->dfs_cac_timer_work, timeout);
> > +
> > +	return 0;
> > +}
> > +
> The required minimum detection probability for weather channels is purposely
> defined such that it can't be reached, so it is safe to assume they will never be
> supported. As for CAC and NOP timings, they are luckily same for ETSI and FCC
> right now, and if they differ in the future, we have the countrycode's DFS
> regulatory domain available to differentiate.

Ah ok, good that they are the same :)

I agree about weather channels, 99.9% detection rate just appears not feasible.
Will remove the comment then.

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Johannes Berg]

On Tue, 2013-01-29 at 13:21 +0100, Simon Wunderlich wrote:

> +void ieee80211_recalc_radar_chanctx(struct ieee80211_local *local,
> +				    struct ieee80211_chanctx *chanctx)
> +{
> +	struct ieee80211_sub_if_data *sdata;
> +	bool radar_enabled = false;
> +
> +	lockdep_assert_held(&local->chanctx_mtx);
> +
> +	rcu_read_lock();
> +	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
> +		if (sdata->radar_required)
> +			radar_enabled = true;

I guess you could break here. Technically though I'm not sure this is
right, since it should only iterate interfaces on this chanctx. OTOH,
multiple channel contexts aren't (currently?) allowed anyway.

> +	chanctx->conf.radar_enabled = radar_enabled;
> +	local->radar_detect_enabled = chanctx->conf.radar_enabled;

What's the reason for "local->radar_detect_enabled"?

Btw, do we also make the driver responsible for turning off any
powersave when radar detection is enabled? I guess so?

> @@ -817,6 +817,15 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
>  
>  	cancel_work_sync(&sdata->recalc_smps);
>  
> +	cancel_delayed_work_sync(&sdata->dfs_cac_timer_work);
> +
> +	/* only inform about abort cac if it was started before. */
> +	if (sdata->wdev.cac_started) {

I'm not entirely sure we should use the wdev data here, OTOH it seems
safe, so why not.

> +void ieee80211_dfs_cac_timer_work(struct work_struct *work)
> +{
> +	struct delayed_work *delayed_work =
> +		container_of(work, struct delayed_work, work);
> +	struct ieee80211_sub_if_data *sdata =
> +		container_of(delayed_work, struct ieee80211_sub_if_data,
> +			     dfs_cac_timer_work);
> +
> +	rtnl_lock();

what part requires rtnl?

> +void ieee80211_radar_detected(struct ieee80211_vif *vif, gfp_t gfp)
> +{
> +	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> +
> +	trace_api_radar_detected(sdata);
> +
> +	/* may happen to devices which have been added but are not up */
> +	if (!cfg80211_chandef_valid(&sdata->vif.bss_conf.chandef))
> +		return;

Huh, what does device and up have to do with that?

>  static bool ieee80211_can_scan(struct ieee80211_local *local,
>  			       struct ieee80211_sub_if_data *sdata)
>  {
> +	if (local->radar_detect_enabled)
> +		return false;

Oh, ok. I guess that explains the reason for radar_detect_enabled :)

johannes

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 3694 bytes --]

On Thu, Jan 31, 2013 at 03:44:08PM +0100, Johannes Berg wrote:
> On Tue, 2013-01-29 at 13:21 +0100, Simon Wunderlich wrote:
> 
> > +void ieee80211_recalc_radar_chanctx(struct ieee80211_local *local,
> > +				    struct ieee80211_chanctx *chanctx)
> > +{
> > +	struct ieee80211_sub_if_data *sdata;
> > +	bool radar_enabled = false;
> > +
> > +	lockdep_assert_held(&local->chanctx_mtx);
> > +
> > +	rcu_read_lock();
> > +	list_for_each_entry_rcu(sdata, &local->interfaces, list) {
> > +		if (sdata->radar_required)
> > +			radar_enabled = true;
> 
> I guess you could break here. 

yes, OK.

> Technically though I'm not sure this is
> right, since it should only iterate interfaces on this chanctx. OTOH,
> multiple channel contexts aren't (currently?) allowed anyway.

I wonder of multiple channel contexts ever make sense for the DFS case.
If they do, we would have to change this anyway to detect radars on
one channel but (maybe) not on the other ... Anyway, I would leave that
to future strategists. ;)

> 
> > +	chanctx->conf.radar_enabled = radar_enabled;
> > +	local->radar_detect_enabled = chanctx->conf.radar_enabled;
> 
> What's the reason for "local->radar_detect_enabled"?

Interaction checking with ROC and scan.
> 
> Btw, do we also make the driver responsible for turning off any
> powersave when radar detection is enabled? I guess so?
> 
> > @@ -817,6 +817,15 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
> >  
> >  	cancel_work_sync(&sdata->recalc_smps);
> >  
> > +	cancel_delayed_work_sync(&sdata->dfs_cac_timer_work);
> > +
> > +	/* only inform about abort cac if it was started before. */
> > +	if (sdata->wdev.cac_started) {
> 
> I'm not entirely sure we should use the wdev data here, OTOH it seems
> safe, so why not.
> 

If you don't mind, it's easier like that ... ;)

> > +void ieee80211_dfs_cac_timer_work(struct work_struct *work)
> > +{
> > +	struct delayed_work *delayed_work =
> > +		container_of(work, struct delayed_work, work);
> > +	struct ieee80211_sub_if_data *sdata =
> > +		container_of(delayed_work, struct ieee80211_sub_if_data,
> > +			     dfs_cac_timer_work);
> > +
> > +	rtnl_lock();
> 
> what part requires rtnl?
> 
ieee80211_vif_release_channel() calls __ieee80211_vif_release_channel()
and has ASSERT_RTNL() before parsing the AP VLAN list.

cfg80211_radar_event() probably doesn't need it ... I should remove it
from the rtnl lock, I guess?

> > +void ieee80211_radar_detected(struct ieee80211_vif *vif, gfp_t gfp)
> > +{
> > +	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> > +
> > +	trace_api_radar_detected(sdata);
> > +
> > +	/* may happen to devices which have been added but are not up */
> > +	if (!cfg80211_chandef_valid(&sdata->vif.bss_conf.chandef))
> > +		return;
> 
> Huh, what does device and up have to do with that?
> 

What I've tried:
 * configure 2 SSIDs in hostapd, start it
 * both wlan0 and wlan0-1 got created
 * only wlan0 comes up, wlan0-1 was rejected because of missing channel combinations
 * now I've injected a radar - which should be sent to wlan0 and wlan0-1
 * wlan0 could send the event, but wlan0-1 had no bss configured and therefore no chandef

I can change this comment to "may happen to devices which have currently no BSS configured",
maybe that it is not so confusing ...

> >  static bool ieee80211_can_scan(struct ieee80211_local *local,
> >  			       struct ieee80211_sub_if_data *sdata)
> >  {
> > +	if (local->radar_detect_enabled)
> > +		return false;
> 
> Oh, ok. I guess that explains the reason for radar_detect_enabled :)

Yup. :)

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Johannes Berg]

On Thu, 2013-01-31 at 17:31 +0100, Simon Wunderlich wrote:

> > > +void ieee80211_dfs_cac_timer_work(struct work_struct *work)
> > > +{
> > > +	struct delayed_work *delayed_work =
> > > +		container_of(work, struct delayed_work, work);
> > > +	struct ieee80211_sub_if_data *sdata =
> > > +		container_of(delayed_work, struct ieee80211_sub_if_data,
> > > +			     dfs_cac_timer_work);
> > > +
> > > +	rtnl_lock();
> > 
> > what part requires rtnl?
> > 
> ieee80211_vif_release_channel() calls __ieee80211_vif_release_channel()
> and has ASSERT_RTNL() before parsing the AP VLAN list.
> 
> cfg80211_radar_event() probably doesn't need it ... I should remove it
> from the rtnl lock, I guess?

Doesn't really matter, I was just wondering in general but the AP VLAN
list makes sense.

> > > +void ieee80211_radar_detected(struct ieee80211_vif *vif, gfp_t gfp)
> > > +{
> > > +	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> > > +
> > > +	trace_api_radar_detected(sdata);
> > > +
> > > +	/* may happen to devices which have been added but are not up */
> > > +	if (!cfg80211_chandef_valid(&sdata->vif.bss_conf.chandef))
> > > +		return;
> > 
> > Huh, what does device and up have to do with that?
> > 
> 
> What I've tried:
>  * configure 2 SSIDs in hostapd, start it
>  * both wlan0 and wlan0-1 got created
>  * only wlan0 comes up, wlan0-1 was rejected because of missing channel combinations
>  * now I've injected a radar - which should be sent to wlan0 and wlan0-1
>  * wlan0 could send the event, but wlan0-1 had no bss configured and therefore no chandef
> 
> I can change this comment to "may happen to devices which have currently no BSS configured",
> maybe that it is not so confusing ...

Not sure I understand, how would the radar detected event come to an
interface that doesn't really exist for the driver?

johannes

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 1641 bytes --]

On Thu, Jan 31, 2013 at 05:48:22PM +0100, Johannes Berg wrote:
> > > > +void ieee80211_radar_detected(struct ieee80211_vif *vif, gfp_t gfp)
> > > > +{
> > > > +	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
> > > > +
> > > > +	trace_api_radar_detected(sdata);
> > > > +
> > > > +	/* may happen to devices which have been added but are not up */
> > > > +	if (!cfg80211_chandef_valid(&sdata->vif.bss_conf.chandef))
> > > > +		return;
> > > 
> > > Huh, what does device and up have to do with that?
> > > 
> > 
> > What I've tried:
> >  * configure 2 SSIDs in hostapd, start it
> >  * both wlan0 and wlan0-1 got created
> >  * only wlan0 comes up, wlan0-1 was rejected because of missing channel combinations
> >  * now I've injected a radar - which should be sent to wlan0 and wlan0-1
> >  * wlan0 could send the event, but wlan0-1 had no bss configured and therefore no chandef
> > 
> > I can change this comment to "may happen to devices which have currently no BSS configured",
> > maybe that it is not so confusing ...
> 
> Not sure I understand, how would the radar detected event come to an
> interface that doesn't really exist for the driver?

wlan0-1 exists and was created, but no AP was ever started - because hostapd tried
to start the AP on a DFS channel when wlan0 was already active, and thanks to our
interface combinations this is not allowed. Therefore, the vif.bss_conf.chandef is empty.

The interface does exist for the driver (interface add succeeded), but start_ap failed,
so it is a virgin AP interface.

I think this behaviour is correct like that ...

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Johannes Berg]

On Thu, 2013-01-31 at 18:47 +0100, Simon Wunderlich wrote:

> > > What I've tried:
> > >  * configure 2 SSIDs in hostapd, start it
> > >  * both wlan0 and wlan0-1 got created
> > >  * only wlan0 comes up, wlan0-1 was rejected because of missing channel combinations
> > >  * now I've injected a radar - which should be sent to wlan0 and wlan0-1
> > >  * wlan0 could send the event, but wlan0-1 had no bss configured and therefore no chandef
> > > 
> > > I can change this comment to "may happen to devices which have currently no BSS configured",
> > > maybe that it is not so confusing ...
> > 
> > Not sure I understand, how would the radar detected event come to an
> > interface that doesn't really exist for the driver?
> 
> wlan0-1 exists and was created, but no AP was ever started - because hostapd tried
> to start the AP on a DFS channel when wlan0 was already active, and thanks to our
> interface combinations this is not allowed. Therefore, the vif.bss_conf.chandef is empty.
> 
> The interface does exist for the driver (interface add succeeded), but start_ap failed,
> so it is a virgin AP interface.
> 
> I think this behaviour is correct like that ...

So ... starting the AP failed because it was a different channel, it was
added to the driver because multiple AP interfaces were allowed but the
specific channel wasn't allowed (in addition) when it was started? But I
still don't see why that interface should get an event since it doesn't
even have a channel yet, except maybe preset_chan which is really only
for backward compatibility reasons?

What am I missing? Where does the event on wlan0-1 come from anyway?

johannes

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]

On Fri, Feb 01, 2013 at 10:57:00AM +0100, Johannes Berg wrote:
> On Thu, 2013-01-31 at 18:47 +0100, Simon Wunderlich wrote:
> 
> > > > What I've tried:
> > > >  * configure 2 SSIDs in hostapd, start it
> > > >  * both wlan0 and wlan0-1 got created
> > > >  * only wlan0 comes up, wlan0-1 was rejected because of missing channel combinations
> > > >  * now I've injected a radar - which should be sent to wlan0 and wlan0-1
> > > >  * wlan0 could send the event, but wlan0-1 had no bss configured and therefore no chandef
> > > > 
> > > > I can change this comment to "may happen to devices which have currently no BSS configured",
> > > > maybe that it is not so confusing ...
> > > 
> > > Not sure I understand, how would the radar detected event come to an
> > > interface that doesn't really exist for the driver?
> > 
> > wlan0-1 exists and was created, but no AP was ever started - because hostapd tried
> > to start the AP on a DFS channel when wlan0 was already active, and thanks to our
> > interface combinations this is not allowed. Therefore, the vif.bss_conf.chandef is empty.
> > 
> > The interface does exist for the driver (interface add succeeded), but start_ap failed,
> > so it is a virgin AP interface.
> > 
> > I think this behaviour is correct like that ...
> 
> So ... starting the AP failed because it was a different channel, it was
> added to the driver because multiple AP interfaces were allowed but the
> specific channel wasn't allowed (in addition) when it was started? But I
> still don't see why that interface should get an event since it doesn't
> even have a channel yet, except maybe preset_chan which is really only
> for backward compatibility reasons?
> 
> What am I missing? Where does the event on wlan0-1 come from anyway?

In the (dummy) ath9k part I'm handling the radar by simply sending radar events
to all ieee80211_vifs which are registered on this phy - regardless their
status, if they are up or not. That's why both wlan0 and wlan0-1 get the
event in my example. Of course, sending the event to wlan0-1 is pretty
useless in my case, but we should better check. :)

Cheers,
	Simon
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Johannes Berg]

On Sat, 2013-02-02 at 23:15 +0100, Simon Wunderlich wrote:
> On Fri, Feb 01, 2013 at 10:57:00AM +0100, Johannes Berg wrote:
> > On Thu, 2013-01-31 at 18:47 +0100, Simon Wunderlich wrote:
> > 
> > > > > What I've tried:
> > > > >  * configure 2 SSIDs in hostapd, start it
> > > > >  * both wlan0 and wlan0-1 got created
> > > > >  * only wlan0 comes up, wlan0-1 was rejected because of missing channel combinations
> > > > >  * now I've injected a radar - which should be sent to wlan0 and wlan0-1
> > > > >  * wlan0 could send the event, but wlan0-1 had no bss configured and therefore no chandef
> > > > > 
> > > > > I can change this comment to "may happen to devices which have currently no BSS configured",
> > > > > maybe that it is not so confusing ...
> > > > 
> > > > Not sure I understand, how would the radar detected event come to an
> > > > interface that doesn't really exist for the driver?
> > > 
> > > wlan0-1 exists and was created, but no AP was ever started - because hostapd tried
> > > to start the AP on a DFS channel when wlan0 was already active, and thanks to our
> > > interface combinations this is not allowed. Therefore, the vif.bss_conf.chandef is empty.
> > > 
> > > The interface does exist for the driver (interface add succeeded), but start_ap failed,
> > > so it is a virgin AP interface.
> > > 
> > > I think this behaviour is correct like that ...
> > 
> > So ... starting the AP failed because it was a different channel, it was
> > added to the driver because multiple AP interfaces were allowed but the
> > specific channel wasn't allowed (in addition) when it was started? But I
> > still don't see why that interface should get an event since it doesn't
> > even have a channel yet, except maybe preset_chan which is really only
> > for backward compatibility reasons?
> > 
> > What am I missing? Where does the event on wlan0-1 come from anyway?
> 
> In the (dummy) ath9k part I'm handling the radar by simply sending radar events
> to all ieee80211_vifs which are registered on this phy - regardless their
> status, if they are up or not. That's why both wlan0 and wlan0-1 get the
> event in my example. Of course, sending the event to wlan0-1 is pretty
> useless in my case, but we should better check. :)

That sounds strange -- how does ath9k even know about an interface
that's not UP? Maybe you mean it didn't start an AP?

Why does the driver even report it by interface? It seems it should do
it on a channel context, or even on the hw since it can only be a single
channel in that case anyway?

johannes

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 1279 bytes --]

On Mon, Feb 04, 2013 at 06:32:05PM +0100, Johannes Berg wrote:
> On Sat, 2013-02-02 at 23:15 +0100, Simon Wunderlich wrote:
> > On Fri, Feb 01, 2013 at 10:57:00AM +0100, Johannes Berg wrote:
> > > On Thu, 2013-01-31 at 18:47 +0100, Simon Wunderlich wrote:
> > > 
> 
> That sounds strange -- how does ath9k even know about an interface
> that's not UP? Maybe you mean it didn't start an AP?

Yeah, that is what I meant - the AP wasn't started on this on that interface.
> 
> Why does the driver even report it by interface? It seems it should do
> it on a channel context, or even on the hw since it can only be a single
> channel in that case anyway?

Well, yeah the reporting is currently done per interface. Reporting radars
for the hw is certainly possible as well - channel contexts are currently
not implemented in ath9k (where I test). The latest patchset works the same
way, although we can change that if you prefer. For the current implementation
(single channel, single vif) it makes no difference, maybe for future implementations.
I don't know if there are any weird corner cases though, and we probably need to
adjust the mac80211 interface too when to report radars on certain channels only
(e.g. extension channel radar).

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Johannes Berg]

On Tue, 2013-02-05 at 09:44 +0100, Simon Wunderlich wrote:
> On Mon, Feb 04, 2013 at 06:32:05PM +0100, Johannes Berg wrote:
> > On Sat, 2013-02-02 at 23:15 +0100, Simon Wunderlich wrote:
> > > On Fri, Feb 01, 2013 at 10:57:00AM +0100, Johannes Berg wrote:
> > > > On Thu, 2013-01-31 at 18:47 +0100, Simon Wunderlich wrote:
> > > > 
> > 
> > That sounds strange -- how does ath9k even know about an interface
> > that's not UP? Maybe you mean it didn't start an AP?
> 
> Yeah, that is what I meant - the AP wasn't started on this on that interface.

Ok.

> > Why does the driver even report it by interface? It seems it should do
> > it on a channel context, or even on the hw since it can only be a single
> > channel in that case anyway?
> 
> Well, yeah the reporting is currently done per interface. Reporting radars
> for the hw is certainly possible as well - channel contexts are currently
> not implemented in ath9k (where I test). The latest patchset works the same
> way, although we can change that if you prefer. For the current implementation
> (single channel, single vif) it makes no difference, maybe for future implementations.
> I don't know if there are any weird corner cases though, and we probably need to
> adjust the mac80211 interface too when to report radars on certain channels only
> (e.g. extension channel radar).

It really just seems strange to me to report radar on all/any other
interfaces to userspace. If we leave it up to the drivers, we'll get all
kinds of strange behaviour ;-)

So it seems better to let mac80211 manage that?

johannes

Re: [PATCHv7 2/3] mac80211: add radar detection command/event

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 1159 bytes --]

On Tue, Feb 05, 2013 at 10:35:00AM +0100, Johannes Berg wrote:
> > 
> > Well, yeah the reporting is currently done per interface. Reporting radars
> > for the hw is certainly possible as well - channel contexts are currently
> > not implemented in ath9k (where I test). The latest patchset works the same
> > way, although we can change that if you prefer. For the current implementation
> > (single channel, single vif) it makes no difference, maybe for future implementations.
> > I don't know if there are any weird corner cases though, and we probably need to
> > adjust the mac80211 interface too when to report radars on certain channels only
> > (e.g. extension channel radar).
> 
> It really just seems strange to me to report radar on all/any other
> interfaces to userspace. If we leave it up to the drivers, we'll get all
> kinds of strange behaviour ;-)
> 
> So it seems better to let mac80211 manage that?

I was afraid you'd say that ... :)

I'll include that change in the next iteration (it is a rather small fix). If you
find anything else in the v8 patchset, please let me know so I can include that.

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[PATCHv7 3/3] nl80211: allow DFS in start_ap

[Simon Wunderlich]

This patch enables DFS within nl80211/cfg80211. start_ap now checks if
DFS is allowed on the channel with the selected channel width.

Signed-off-by: Simon Wunderlich <siwu@hrz.tu-chemnitz.de>
---
Changes to PATCHv6:
 * let reg_can_beacon() and others check for channel states, simplify
   start_ap
 * check using cfg80211_chandef_dfs_required

Changes to PATCHv5:
 * check radar detection width
---
 net/wireless/nl80211.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 2e0216a..fd7b654 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2608,6 +2608,7 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_ap_settings params;
 	int err;
+	u8 radar_detect_width = 0;
 
 	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
 	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
@@ -2726,9 +2727,17 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
 	if (!cfg80211_reg_can_beacon(&rdev->wiphy, &params.chandef))
 		return -EINVAL;
 
+	err = cfg80211_chandef_dfs_required(wdev->wiphy, &params.chandef);
+	if (err < 0)
+		return -EINVAL;
+	if (err)
+		radar_detect_width = BIT(params.chandef.width);
+
 	mutex_lock(&rdev->devlist_mtx);
-	err = cfg80211_can_use_chan(rdev, wdev, params.chandef.chan,
-				    CHAN_MODE_SHARED);
+	err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
+					   params.chandef.chan,
+					   CHAN_MODE_SHARED,
+					   radar_detect_width);
 	mutex_unlock(&rdev->devlist_mtx);
 
 	if (err)
-- 
1.7.10.4

Re: [PATCHv7 0/3] Add DFS master ability

[Zefir Kurtisi]

On 01/29/2013 01:21 PM, Simon Wunderlich wrote:
> [...]
> 
> The channel states are now implemented in cfg80211. Shall we inform userspace
> about channel changes? If yes, how should we do that? We could add channel states
> to the channel list, and give "channel list changed" events to userspace as it
> happens now, or define a new kind of event ("channel-available-again-event").
> Suggestions welcome. :)
> 
An event whenever a channel state changes is perfect, ideally provided with the
time-stamp when this change happened.

With the centralized channel state handling proposed here, the required
modification to allow managed DFS operation can be minimized to a proprietary (or
even upstreamed but CONFIG_CFG80211_CERTIFICATION_ONUS guarded) function to modify
channel states.

Can't contribute much to code review, but full ACK for the updated concept.

> If you prefer, I can split off the channel state stuff from the radar detect
> patches.
> 
> Cheers,
> 	Simon
> 

Re: [PATCHv7 0/3] Add DFS master ability

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 1726 bytes --]

Hey Zefir,

On Tue, Jan 29, 2013 at 02:14:26PM +0100, Zefir Kurtisi wrote:
> On 01/29/2013 01:21 PM, Simon Wunderlich wrote:
> > [...]
> > 
> > The channel states are now implemented in cfg80211. Shall we inform userspace
> > about channel changes? If yes, how should we do that? We could add channel states
> > to the channel list, and give "channel list changed" events to userspace as it
> > happens now, or define a new kind of event ("channel-available-again-event").
> > Suggestions welcome. :)
> > 
> An event whenever a channel state changes is perfect, ideally provided with the
> time-stamp when this change happened.

Yeah. "channel list changed" events are already available (at least I get them in
hostapd). If we put the current state + timestamp when entered into the channel list,
we would have what you want, too.

> 
> With the centralized channel state handling proposed here, the required
> modification to allow managed DFS operation can be minimized to a proprietary (or
> even upstreamed but CONFIG_CFG80211_CERTIFICATION_ONUS guarded) function to modify
> channel states.

Yes, the idea is that one can have a small proprietary patch or default-off command
(CONFIG_CFG80211_CERTIFICATION_ONUS sounds good) or something like that to change a
DFS state with the new cfg80211 function. I also thought about having a nl80211-command
without implementing an iw counter part or keeping a special "state change" program
somewhere with appropriate warnings. But this might still be too "liberal" for the
regulatory statement. :)
> 
> Can't contribute much to code review, but full ACK for the updated concept.
> 

Cool, also thanks for the code comments!

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

Re: [PATCHv7 0/3] Add DFS master ability

[Johannes Berg]

On Tue, 2013-01-29 at 13:21 +0100, Simon Wunderlich wrote:

> Right now, channels at least on my machine have the NO IBSS and PASSIVE SCAN
> flags set, which prevents reg_can_beacon from returning true, even if we do
> have radar support. This is due to my reg database setting these flags for
> country code 00, and this is still kept when changing the country code to
> something else (DE in my case). How should we proceed with this? Shouldn't
> these flags be removed when selecting a country code?

I don't think any country sets that, you may be running into that whole
"regdb intersect" thing ... FWIW the flags are set for 00 since for
world roaming we don't know if we're in a country that even allows
transmission on that AP.

> The channel states are now implemented in cfg80211. Shall we inform userspace
> about channel changes? If yes, how should we do that? We could add channel states
> to the channel list, and give "channel list changed" events to userspace as it
> happens now, or define a new kind of event ("channel-available-again-event").
> Suggestions welcome. :)

You already inform it of the changes by the radar event, but it would
probably also be good to export the state as part of the channel list
that you get with things like "iw list".

johannes

Re: [PATCHv7 0/3] Add DFS master ability

[Simon Wunderlich]

[-- Attachment #1: Type: text/plain, Size: 2709 bytes --]

On Thu, Jan 31, 2013 at 05:50:44PM +0100, Johannes Berg wrote:
> On Tue, 2013-01-29 at 13:21 +0100, Simon Wunderlich wrote:
> 
> > Right now, channels at least on my machine have the NO IBSS and PASSIVE SCAN
> > flags set, which prevents reg_can_beacon from returning true, even if we do
> > have radar support. This is due to my reg database setting these flags for
> > country code 00, and this is still kept when changing the country code to
> > something else (DE in my case). How should we proceed with this? Shouldn't
> > these flags be removed when selecting a country code?
> 
> I don't think any country sets that, you may be running into that whole
> "regdb intersect" thing ... FWIW the flags are set for 00 since for
> world roaming we don't know if we're in a country that even allows
> transmission on that AP.
> 

Yes, that is certainly the regdb intersect thing - my atheros card is branded to "00", which
is the world country code - I would expect to do anything now, not nothing. :D

I mean, I need to set some country code anyway (at least this will be a requirement
in hostapd), so is this intersection thing really useful for the AP case?

reg_beacon() checks all these flags (NO_IBSS, PASSIVE_SCAN), but it actually should
just care about the radar flag I assume?
BTW, I have not a single country in my regdb list which sets either passive scan or
NO_IBSS (according to regdbdump on my laptop). All non-00 countries only set DFS or
NO_OUTDOOR (and NO-OFDM in japan for channel 14).

> > The channel states are now implemented in cfg80211. Shall we inform userspace
> > about channel changes? If yes, how should we do that? We could add channel states
> > to the channel list, and give "channel list changed" events to userspace as it
> > happens now, or define a new kind of event ("channel-available-again-event").
> > Suggestions welcome. :)
> 
> You already inform it of the changes by the radar event, but it would
> probably also be good to export the state as part of the channel list
> that you get with things like "iw list".

Well I do that when I finish/abort CAC or detect a radar. But I (currently)
don't send an event when changing from unvailable to usable (after non-occupancy
period). I see the options:

 1) add channel-usable-again event to cfg80211_radar_event 
 2) send a "channel list changed" (seems this also sent when the regdb is changed).

Any preferences? Maybe option 1 for completeness?

I wanted to export the state on the channel list anyway. Zefir asked to have the
timestamp there as well (though I don't know what the specific usecase is ...).
Will include that in the next revision.

Cheers,
	Simon

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]