[PATCH v4 0/6] nl/cfg/mac80211: add DFS master ability

[PATCH v4 0/6] nl/cfg/mac80211: add DFS master ability

[Victor Goldenshtein]

v4:
========================================================
mac80211: add DFS support to monitor interface
        1. patch deleted.

mac80211: add ap channel switch command/event
        1. reordered tracing in ieee80211_ap_ch_switch_work().
        2. add return value to the ap_channel_switch callback.
        3. handle channel switch complete in new.
        "ieee80211_ap_ch_switch_work" to prevent possible circular
        locking dependency.
        4. new "ap_cs_channel" & "ap_cs_type" parameters in ieee80211_local.
        5. add "ieee80211_stop_queues_by_reason" in
        ieee80211_ap_process_chanswitch().
        6. add "ieee80211_wake_queues_by_reason" in ieee80211_ap_ch_switch_work.

nl80211/cfg80211: add ap channel switch command
        1. move the NL80211_CMD_AP_CH_SWITCH to the end of the enum
        nl80211_commands.
        2. document types for NL80211_ATTR_CH_SWITCH_COUNT,
        NL80211_ATTR_CH_SWITCH_BLOCK_TX, NL80211_ATTR_CH_SWITCH_POST_BLOCK_TX.
        3. updated documentation for @NL80211_CMD_AP_CH_SWITCH.
        4. validate that device supports AP channel switch in
        nl80211_start_radar_detection() & nl80211_dfs_en_tx().
        5. add NL80211_CHAN_NO_HT support in nl80211_ap_channel_switch().
        6. fix bogus documentation of the struct ieee80211_ap_ch_switch.   

mac80211: add ability to enable TX on op-channel
        1. document that dfs_en_tx callback can sleep.
        2. added channel frequency to the trace_drv_dfs_en_tx.

nl80211/cfg80211: add ability to enable TX on op-channel
        1. remove dfs_supported flag from nl80211_dfs_en_tx().
        2. add NL80211_CHAN_NO_HT support in nl80211_dfs_en_tx().
        3. add chan->cac_started check to the nl80211_dfs_en_tx().

mac80211: add radar detection command/event
        1. remove ret variable in ieee80211_start_radar_detection().
        2. add channel frequancy to the trace_drv_start_radar_detection.
        3. remove "sdata->wdev.preset_chan->cac_type = 0;" line from
        ieee80211_do_stop().
        4. document that start_radar_detection callback can sleep.

nl80211/cfg80211: add radar detection command/event
        1. remove dfs_supported flag in nl80211_start_radar_detection().
        2. new bool cac_started in the struct ieee80211_channel.
        3. manually inline cfg80211_start_radar_detection() into
        nl80211_start_radar_detection().
        4. rename IEEE80211_DFS_MIN_CAC_TIME_MS to
        NL80211_DFS_MIN_CAC_TIME_MS and move it to nl80211.h
        5. add NL80211_CHAN_NO_HT support in 
        nl80211_start_radar_detection().
        6. rename NL80211_FEATURE_DFS to NL80211_FEATURE_20MHZ_DFS.
        7. reset the "cac_started" flag in __nl80211_set_channel().
        8. don't reset radar_detect_timeout flag.


hostap TODO:
====================================
NL80211_FEATURE_DFS should be renamed to NL80211_FEATURE_20MHZ_DFS in
phy_info_handler().


Victor Goldenshtein (6):
  nl80211/cfg80211: add radar detection command/event
  mac80211: add radar detection command/event
  nl80211/cfg80211: add ability to enable TX on op-channel
  mac80211: add ability to enable TX on op-channel
  nl80211/cfg80211: add ap channel switch command
  mac80211: add ap channel switch command/event

 include/linux/nl80211.h    |   48 ++++++++++++
 include/net/cfg80211.h     |   65 ++++++++++++++++
 include/net/mac80211.h     |   45 +++++++++++
 net/mac80211/cfg.c         |   57 ++++++++++++++
 net/mac80211/driver-ops.h  |   40 ++++++++++
 net/mac80211/ieee80211_i.h |    4 +
 net/mac80211/iface.c       |    7 ++
 net/mac80211/main.c        |    3 +
 net/mac80211/mlme.c        |   53 +++++++++++++
 net/mac80211/trace.h       |  116 ++++++++++++++++++++++++++++
 net/wireless/mlme.c        |   13 +++
 net/wireless/nl80211.c     |  184 ++++++++++++++++++++++++++++++++++++++++++++
 net/wireless/nl80211.h     |    6 ++
 13 files changed, 641 insertions(+), 0 deletions(-)

-- 
1.7.5.4

[PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Victor Goldenshtein]

Add new NL80211_CMD_RADAR_DETECT, which triggers radar
detection in the driver/FW, this command will also notify
usermode with 'radar detected' event.
Once radar detection has started it should continuously
monitor for radars as long as the channel is active.

Add new NL80211_FEATURE_20MHZ_DFS attribute which
indicates that driver/HW supports radar detection
on 20MHz channel bandwidth.

Signed-off-by: Victor Goldenshtein <victorg@ti.com>
---
 include/linux/nl80211.h |   10 +++++
 include/net/cfg80211.h  |   24 +++++++++++++
 net/wireless/mlme.c     |   13 +++++++
 net/wireless/nl80211.c  |   89 +++++++++++++++++++++++++++++++++++++++++++++++
 net/wireless/nl80211.h  |    6 +++
 5 files changed, 142 insertions(+), 0 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index 7df9b50..ba4c697 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -578,6 +578,9 @@
  *	station, due to particular reason. %NL80211_ATTR_CONN_FAILED_REASON
  *	is used for this.
  *
+ * @NL80211_CMD_RADAR_DETECT: Start radar detection in the driver/HW. Once
+ *	radar detected usermode notified with this event.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -726,6 +729,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_CONN_FAILED,
 
+	NL80211_CMD_RADAR_DETECT,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -1584,6 +1589,8 @@ enum nl80211_attrs {
 
 #define NL80211_CQM_TXE_MAX_INTVL		1800
 
+#define NL80211_DFS_MIN_CAC_TIME_MS		60000
+
 /**
  * enum nl80211_iftype - (virtual) interface types
  *
@@ -3028,6 +3035,8 @@ enum nl80211_ap_sme_features {
  *	in the interface combinations, even when it's only used for scan
  *	and remain-on-channel. This could be due to, for example, the
  *	remain-on-channel implementation requiring a channel context.
+ * @NL80211_FEATURE_20MHZ_DFS: Radar detection is supported in the
+ *	HW/driver on 20 MHz channel bandwidth.
  */
 enum nl80211_feature_flags {
 	NL80211_FEATURE_SK_TX_STATUS			= 1 << 0,
@@ -3035,6 +3044,7 @@ enum nl80211_feature_flags {
 	NL80211_FEATURE_INACTIVITY_TIMER		= 1 << 2,
 	NL80211_FEATURE_CELL_BASE_REG_HINTS		= 1 << 3,
 	NL80211_FEATURE_P2P_DEVICE_NEEDS_CHANNEL	= 1 << 4,
+	NL80211_FEATURE_20MHZ_DFS			= 1 << 5,
 };
 
 /**
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index ab78b53..238d0e2 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -131,6 +131,11 @@ enum ieee80211_channel_flags {
  *	to enable this, this is useful only on 5 GHz band.
  * @orig_mag: internal use
  * @orig_mpwr: internal use
+ * @radar_detect_timeout: this timeout indicates the end of the channel
+ *	availability check for radar channels (in jiffies), only after this
+ *	period the user may initiate the tx on the channel.
+ * @cac_type: indicates that channel availability check is started for this
+ *	channel type.
  */
 struct ieee80211_channel {
 	enum ieee80211_band band;
@@ -143,6 +148,9 @@ struct ieee80211_channel {
 	bool beacon_found;
 	u32 orig_flags;
 	int orig_mag, orig_mpwr;
+	unsigned long radar_detect_timeout;
+	enum nl80211_channel_type cac_type;
+	bool cac_started;
 };
 
 /**
@@ -1621,6 +1629,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);
@@ -1842,6 +1852,9 @@ 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 ieee80211_channel *chan);
 };
 
 /*
@@ -3430,6 +3443,17 @@ void cfg80211_cqm_rssi_notify(struct net_device *dev,
 			      gfp_t gfp);
 
 /**
+ * cfg80211_radar_detected - radar detection event
+ * @dev: network device
+ * @chan: radar detected on this channel.
+ * @gfp: context flags
+ *
+ * This function is called when a radar is detected on the current channel.
+ */
+void cfg80211_radar_detected(struct net_device *dev,
+			     struct ieee80211_channel *chan, gfp_t gfp);
+
+/**
  * cfg80211_cqm_pktloss_notify - notify userspace about packetloss to peer
  * @dev: network device
  * @peer: peer's MAC address
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 3df195a..1a732a8 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -1021,3 +1021,16 @@ bool cfg80211_rx_unexpected_4addr_frame(struct net_device *dev,
 	return nl80211_unexpected_4addr_frame(dev, addr, gfp);
 }
 EXPORT_SYMBOL(cfg80211_rx_unexpected_4addr_frame);
+
+void cfg80211_radar_detected(struct net_device *dev,
+			     struct ieee80211_channel *chan, gfp_t gfp)
+{
+	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct wiphy *wiphy = wdev->wiphy;
+	struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
+
+	chan->cac_started = false;
+
+	nl80211_radar_detected_notify(rdev, chan, dev, gfp);
+}
+EXPORT_SYMBOL(cfg80211_radar_detected);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index f1047ae..4c83c28 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1411,6 +1411,7 @@ static int __nl80211_set_channel(struct cfg80211_registered_device *rdev,
 			result = -EINVAL;
 			break;
 		}
+		channel->cac_started = false;
 		wdev->preset_chan = channel;
 		wdev->preset_chantype = channel_type;
 		result = 0;
@@ -4604,6 +4605,49 @@ 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];
+	int freq, err;
+	struct ieee80211_channel *chan;
+	enum nl80211_channel_type cac_type;
+
+	if (!(rdev->wiphy.features & NL80211_FEATURE_20MHZ_DFS))
+		return -EOPNOTSUPP;
+
+	if (!info->attrs[NL80211_ATTR_WIPHY_FREQ] ||
+	    !info->attrs[NL80211_ATTR_WIPHY_CHANNEL_TYPE])
+		return -EINVAL;
+
+	cac_type = nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_CHANNEL_TYPE]);
+	if ((cac_type != NL80211_CHAN_HT20) && (cac_type != NL80211_CHAN_NO_HT))
+		return -EOPNOTSUPP;
+
+	freq = nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]);
+	chan = ieee80211_get_channel(&rdev->wiphy, freq);
+
+	if (!chan || !(chan->flags & IEEE80211_CHAN_RADAR))
+		return -EINVAL;
+
+	if (chan->cac_started)
+		return -EBUSY;
+
+	if (!rdev->ops->start_radar_detection)
+		return -EOPNOTSUPP;
+
+	err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, chan);
+	if (!err) {
+		chan->cac_type = cac_type;
+		chan->cac_started = true;
+		chan->radar_detect_timeout = jiffies +
+			msecs_to_jiffies(NL80211_DFS_MIN_CAC_TIME_MS);
+	}
+
+	return err;
+}
+
 static int nl80211_send_bss(struct sk_buff *msg, struct netlink_callback *cb,
 			    u32 seq, int flags,
 			    struct cfg80211_registered_device *rdev,
@@ -7552,6 +7596,14 @@ static struct genl_ops nl80211_ops[] = {
 		.internal_flags = NL80211_FLAG_NEED_WDEV_UP |
 				  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 = {
@@ -8748,6 +8800,43 @@ nl80211_send_cqm_txe_notify(struct cfg80211_registered_device *rdev,
 }
 
 void
+nl80211_radar_detected_notify(struct cfg80211_registered_device *rdev,
+			      struct ieee80211_channel *chan,
+			      struct net_device *netdev, gfp_t gfp)
+{
+	struct sk_buff *msg;
+	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_u32(msg, NL80211_ATTR_WIPHY_FREQ, chan->center_freq))
+		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 f615351..42203ae 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -110,6 +110,12 @@ 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_detected_notify(struct cfg80211_registered_device *rdev,
+			      struct ieee80211_channel *chan,
+			      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,
-- 
1.7.5.4

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Johannes Berg]

On Sun, 2012-10-14 at 16:48 +0200, Victor Goldenshtein wrote:

> + * @radar_detect_timeout: this timeout indicates the end of the channel
> + *	availability check for radar channels (in jiffies), only after this
> + *	period the user may initiate the tx on the channel.
> + * @cac_type: indicates that channel availability check is started for this
> + *	channel type.

You're missing docs for cac_started

I'll fix it if I don't have any comments on the other patches and the
answer to my question below doesn't mean a change:

> +++ b/net/wireless/nl80211.c
> @@ -1411,6 +1411,7 @@ static int __nl80211_set_channel(struct cfg80211_registered_device *rdev,
>  			result = -EINVAL;
>  			break;
>  		}
> +		channel->cac_started = false;

Why this? If the device supports channel contexts then maybe one vif
could set the channel and the other could be doing radar detection? But
anyway this only presets the channel, so nothing will happen until the
AP interface is started? So basically I don't understand this at all.

It also raises a question: can you do radar detection properly while
doing channel TDM (multi-channel)? I guess not?

johannes

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Zefir Kurtisi]

On 10/16/2012 09:01 PM, Johannes Berg wrote:
> [...]
> 
> It also raises a question: can you do radar detection properly while
> doing channel TDM (multi-channel)? I guess not?
> 
> johannes
> 
At least ETSI differentiates between CAC and Off-Channel CAC, with
dedicated requirements for times and detection probability for
continuously monitoring the operating channel vs. accumulated periods
listening for radars on a different one.

OC-CAC is considered as an optional optimization to enable switching to
a new channel without the need to perform a CAC there. Weighting the
potential gain vs. the required effort to support that feature, I'd
assume OC-CAC is not a hot topic (maybe even not feasible in the long run).

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Johannes Berg]

On Wed, 2012-10-17 at 18:20 +0200, Zefir Kurtisi wrote:
> On 10/16/2012 09:01 PM, Johannes Berg wrote:
> > [...]
> > 
> > It also raises a question: can you do radar detection properly while
> > doing channel TDM (multi-channel)? I guess not?

> At least ETSI differentiates between CAC and Off-Channel CAC, with
> dedicated requirements for times and detection probability for
> continuously monitoring the operating channel vs. accumulated periods
> listening for radars on a different one.
> 
> OC-CAC is considered as an optional optimization to enable switching to
> a new channel without the need to perform a CAC there. Weighting the
> potential gain vs. the required effort to support that feature, I'd
> assume OC-CAC is not a hot topic (maybe even not feasible in the long run).

Ok, cool, nice to know.

johannes

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Victor Goldenshtein]

On 16/10/2012 21:01, Johannes Berg wrote:
> On Sun, 2012-10-14 at 16:48 +0200, Victor Goldenshtein wrote:
>
>> + * @radar_detect_timeout: this timeout indicates the end of the channel
>> + *	availability check for radar channels (in jiffies), only after this
>> + *	period the user may initiate the tx on the channel.
>> + * @cac_type: indicates that channel availability check is started for this
>> + *	channel type.
>
> You're missing docs for cac_started
>

missed that..

> I'll fix it if I don't have any comments on the other patches and the
> answer to my question below doesn't mean a change:
>
>> +++ b/net/wireless/nl80211.c
>> @@ -1411,6 +1411,7 @@ static int __nl80211_set_channel(struct cfg80211_registered_device *rdev,
>>   			result = -EINVAL;
>>   			break;
>>   		}
>> +		channel->cac_started = false;
>
> Why this? If the device supports channel contexts then maybe one vif
> could set the channel and the other could be doing radar detection? But
> anyway this only presets the channel, so nothing will happen until the
> AP interface is started? So basically I don't understand this at all.

Here we just clearing the cac_started flag, this was previously done in 
the ieee80211_do_stop() for the "sdata->wdev.preset_chan", but as we 
didn't want to modify cfg80211 fields from mac I suggested to move it here.

I"ll try to clarify things a bit, the channel availability check (CAC) 
could be started during:
1. AP init phase - when AP is configured on DFS channel.
2. As a result of a channel switch - AP moved to a DFS channel.

In both cases we set the cac_started flag from the 
nl80211_start_radar_detection() and clear it:
1. As a result of radar event.
2. In __nl80211_set_channel() - to cover the case when the CAC was 
initiated on a "preset_chan" (during AP init phase) and the IF was 
removed before the AP was even started (local->oper_channel wasn't set yet).


-- 
Thanks,
Victor.

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Johannes Berg]

On Sun, 2012-10-21 at 18:40 +0200, Victor Goldenshtein wrote:

> >> +++ b/net/wireless/nl80211.c
> >> @@ -1411,6 +1411,7 @@ static int __nl80211_set_channel(struct cfg80211_registered_device *rdev,
> >>   			result = -EINVAL;
> >>   			break;
> >>   		}
> >> +		channel->cac_started = false;
> >
> > Why this? If the device supports channel contexts then maybe one vif
> > could set the channel and the other could be doing radar detection? But
> > anyway this only presets the channel, so nothing will happen until the
> > AP interface is started? So basically I don't understand this at all.
> 
> Here we just clearing the cac_started flag, this was previously done in 
> the ieee80211_do_stop() for the "sdata->wdev.preset_chan", but as we 
> didn't want to modify cfg80211 fields from mac I suggested to move it here.

We have an explicit AP stop operation in cfg80211 now, maybe you want
that?

> I"ll try to clarify things a bit, the channel availability check (CAC) 
> could be started during:
> 1. AP init phase - when AP is configured on DFS channel.
> 2. As a result of a channel switch - AP moved to a DFS channel.

But it'll be started explicitly by userspace.

> In both cases we set the cac_started flag from the 
> nl80211_start_radar_detection() 

right

> and clear it:
> 1. As a result of radar event.

makes sense

> 2. In __nl80211_set_channel() - to cover the case when the CAC was 
> initiated on a "preset_chan" (during AP init phase) and the IF was 
> removed before the AP was even started (local->oper_channel wasn't set yet).

Hmm, I'm not sure I get it. How is "local->oper_channel" (a mac80211
variable) related to this cfg80211 code?

start_ap() isn't expected to be able to succeed until CAC passed
successfully, but OTOH the channel isn't configured until then?

johannes

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Victor Goldenshtein]

On 22/10/2012 14:55, Johannes Berg wrote:
>> 2. In __nl80211_set_channel() - to cover the case when the CAC was
>> initiated on a "preset_chan" (during AP init phase) and the IF was
>> removed before the AP was even started (local->oper_channel wasn't set yet).
>
> Hmm, I'm not sure I get it. How is "local->oper_channel" (a mac80211
> variable) related to this cfg80211 code?

It's not, just saying that its not set at this point.

> start_ap() isn't expected to be able to succeed until CAC passed
> successfully, but OTOH the channel isn't configured until then?

right, the initial CAC performed before start_ap(), only by setting the 
channel with __nl80211_set_channel() + radar detection command.

-- 
Thanks,
Victor.

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Johannes Berg]

On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:
> On 22/10/2012 14:55, Johannes Berg wrote:
> >> 2. In __nl80211_set_channel() - to cover the case when the CAC was
> >> initiated on a "preset_chan" (during AP init phase) and the IF was
> >> removed before the AP was even started (local->oper_channel wasn't set yet).
> >
> > Hmm, I'm not sure I get it. How is "local->oper_channel" (a mac80211
> > variable) related to this cfg80211 code?
> 
> It's not, just saying that its not set at this point.
> 
> > start_ap() isn't expected to be able to succeed until CAC passed
> > successfully, but OTOH the channel isn't configured until then?
> 
> right, the initial CAC performed before start_ap(), only by setting the 
> channel with __nl80211_set_channel() + radar detection command.

Hmm. Maybe then the channel should be passed to the radar detection
command instead? That way, it can be passed through, you can allocate a
channel context, etc. Much easier?

johannes

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Victor Goldenshtein]

On 05/11/2012 17:21, Johannes Berg wrote:
> On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:
>> On 22/10/2012 14:55, Johannes Berg wrote:
>>>> 2. In __nl80211_set_channel() - to cover the case when the CAC was
>>>> initiated on a "preset_chan" (during AP init phase) and the IF was
>>>> removed before the AP was even started (local->oper_channel wasn't set yet).
>>>
>>> Hmm, I'm not sure I get it. How is "local->oper_channel" (a mac80211
>>> variable) related to this cfg80211 code?
>>
>> It's not, just saying that its not set at this point.
>>
>>> start_ap() isn't expected to be able to succeed until CAC passed
>>> successfully, but OTOH the channel isn't configured until then?
>>
>> right, the initial CAC performed before start_ap(), only by setting the
>> channel with __nl80211_set_channel() + radar detection command.
>
> Hmm. Maybe then the channel should be passed to the radar detection
> command instead? That way, it can be passed through, you can allocate a
> channel context, etc. Much easier?

We already pass the frequency in the radar detection command, the set 
channel operation comes during hostapd init flow regardless DFS 
implementation.

-- 
Thanks,
Victor.

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Simon Wunderlich]

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

On Tue, Nov 13, 2012 at 05:04:03PM +0200, Victor Goldenshtein wrote:
> On 05/11/2012 17:21, Johannes Berg wrote:
> >On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:
> >>On 22/10/2012 14:55, Johannes Berg wrote:
> >>>>2. In __nl80211_set_channel() - to cover the case when the CAC was
> >>>>initiated on a "preset_chan" (during AP init phase) and the IF was
> >>>>removed before the AP was even started (local->oper_channel wasn't set yet).
> >>>
> >>>Hmm, I'm not sure I get it. How is "local->oper_channel" (a mac80211
> >>>variable) related to this cfg80211 code?
> >>
> >>It's not, just saying that its not set at this point.
> >>
> >>>start_ap() isn't expected to be able to succeed until CAC passed
> >>>successfully, but OTOH the channel isn't configured until then?
> >>
> >>right, the initial CAC performed before start_ap(), only by setting the
> >>channel with __nl80211_set_channel() + radar detection command.
> >
> >Hmm. Maybe then the channel should be passed to the radar detection
> >command instead? That way, it can be passed through, you can allocate a
> >channel context, etc. Much easier?
> 
> We already pass the frequency in the radar detection command, the
> set channel operation comes during hostapd init flow regardless DFS
> implementation.

Just for clarification for the API: Is the driver required to (re)set the channel
itself in the radar detection command, or do we expect that the channel was already
set via __nl80211_set_channel() (or other means) before?

This would be important for other DFS modes like IBSS later.

Thanks,
	Simon

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

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Victor Goldenshtein]

On 13/11/2012 20:05, Simon Wunderlich wrote:
> On Tue, Nov 13, 2012 at 05:04:03PM +0200, Victor Goldenshtein wrote:
>> On 05/11/2012 17:21, Johannes Berg wrote:
>>> On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:
>>>> On 22/10/2012 14:55, Johannes Berg wrote:
>>>>>> 2. In __nl80211_set_channel() - to cover the case when the CAC was
>>>>>> initiated on a "preset_chan" (during AP init phase) and the IF was
>>>>>> removed before the AP was even started (local->oper_channel wasn't set yet).
>>>>>
>>>>> Hmm, I'm not sure I get it. How is "local->oper_channel" (a mac80211
>>>>> variable) related to this cfg80211 code?
>>>>
>>>> It's not, just saying that its not set at this point.
>>>>
>>>>> start_ap() isn't expected to be able to succeed until CAC passed
>>>>> successfully, but OTOH the channel isn't configured until then?
>>>>
>>>> right, the initial CAC performed before start_ap(), only by setting the
>>>> channel with __nl80211_set_channel() + radar detection command.
>>>
>>> Hmm. Maybe then the channel should be passed to the radar detection
>>> command instead? That way, it can be passed through, you can allocate a
>>> channel context, etc. Much easier?
>>
>> We already pass the frequency in the radar detection command, the
>> set channel operation comes during hostapd init flow regardless DFS
>> implementation.
>
> Just for clarification for the API: Is the driver required to (re)set the channel
> itself in the radar detection command, or do we expect that the channel was already
> set via __nl80211_set_channel() (or other means) before?
>
> This would be important for other DFS modes like IBSS later.
>

Already discussed this privately, still maybe someone else also interested.

The set channel operation is relevant only during initial CAC test,
it set and re-set (if necessary - e.g. radar detected on the first
channel) by the hostapd it self, so the driver doesn't required to set 
the channel in the radar detection command.


-- 
Thanks,
Victor.

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Johannes Berg]

On Tue, 2012-11-20 at 17:14 +0200, Victor Goldenshtein wrote:
> On 13/11/2012 20:05, Simon Wunderlich wrote:
> > On Tue, Nov 13, 2012 at 05:04:03PM +0200, Victor Goldenshtein wrote:
> >> On 05/11/2012 17:21, Johannes Berg wrote:
> >>> On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:
> >>>> On 22/10/2012 14:55, Johannes Berg wrote:
> >>>>>> 2. In __nl80211_set_channel() - to cover the case when the CAC was
> >>>>>> initiated on a "preset_chan" (during AP init phase) and the IF was
> >>>>>> removed before the AP was even started (local->oper_channel wasn't set yet).
> >>>>>
> >>>>> Hmm, I'm not sure I get it. How is "local->oper_channel" (a mac80211
> >>>>> variable) related to this cfg80211 code?
> >>>>
> >>>> It's not, just saying that its not set at this point.
> >>>>
> >>>>> start_ap() isn't expected to be able to succeed until CAC passed
> >>>>> successfully, but OTOH the channel isn't configured until then?
> >>>>
> >>>> right, the initial CAC performed before start_ap(), only by setting the
> >>>> channel with __nl80211_set_channel() + radar detection command.
> >>>
> >>> Hmm. Maybe then the channel should be passed to the radar detection
> >>> command instead? That way, it can be passed through, you can allocate a
> >>> channel context, etc. Much easier?
> >>
> >> We already pass the frequency in the radar detection command, the
> >> set channel operation comes during hostapd init flow regardless DFS
> >> implementation.
> >
> > Just for clarification for the API: Is the driver required to (re)set the channel
> > itself in the radar detection command, or do we expect that the channel was already
> > set via __nl80211_set_channel() (or other means) before?
> >
> > This would be important for other DFS modes like IBSS later.
> >
> 
> Already discussed this privately, still maybe someone else also interested.
> 
> The set channel operation is relevant only during initial CAC test,
> it set and re-set (if necessary - e.g. radar detected on the first
> channel) by the hostapd it self, so the driver doesn't required to set 
> the channel in the radar detection command.

Actually, it seems to me the radar detection command has to set the
channel and be tied to the channel context that way. I'd definitely
prefer that, even if you could use wdev->preset_chan it would be much
better to explicitly pass the channel.

johannes

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Johannes Berg]

On Tue, 2012-11-13 at 17:04 +0200, Victor Goldenshtein wrote:

> > Hmm. Maybe then the channel should be passed to the radar detection
> > command instead? That way, it can be passed through, you can allocate a
> > channel context, etc. Much easier?
> 
> We already pass the frequency in the radar detection command, the set 
> channel operation comes during hostapd init flow regardless DFS 
> implementation.

Ok, but it doesn't seem to be passed through to the driver, nor
considered as a channel that's currently in use. cfg80211 is now
tracking which channels are used for multi-channel purposes, and this
needs to be integrated here.

johannes

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Victor Goldenshtein]

On 14/11/2012 13:19, Johannes Berg wrote:
> On Tue, 2012-11-13 at 17:04 +0200, Victor Goldenshtein wrote:
>
>>> Hmm. Maybe then the channel should be passed to the radar detection
>>> command instead? That way, it can be passed through, you can allocate a
>>> channel context, etc. Much easier?
>>
>> We already pass the frequency in the radar detection command, the set
>> channel operation comes during hostapd init flow regardless DFS
>> implementation.
>
> Ok, but it doesn't seem to be passed through to the driver, nor
> considered as a channel that's currently in use. cfg80211 is now
> tracking which channels are used for multi-channel purposes, and this
> needs to be integrated here.
>

right, we should consider currently used channel and somehow make sure 
that we use a single channel context.


-- 
Thanks,
Victor.

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Michal Kazior]

On 05/11/12 16:21, Johannes Berg wrote:
> On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:
>> On 22/10/2012 14:55, Johannes Berg wrote:
>>>> 2. In __nl80211_set_channel() - to cover the case when the CAC was
>>>> initiated on a "preset_chan" (during AP init phase) and the IF was
>>>> removed before the AP was even started (local->oper_channel wasn't set yet).
>>>
>>> Hmm, I'm not sure I get it. How is "local->oper_channel" (a mac80211
>>> variable) related to this cfg80211 code?
>>
>> It's not, just saying that its not set at this point.
>>
>>> start_ap() isn't expected to be able to succeed until CAC passed
>>> successfully, but OTOH the channel isn't configured until then?
>>
>> right, the initial CAC performed before start_ap(), only by setting the
>> channel with __nl80211_set_channel() + radar detection command.
>
> Hmm. Maybe then the channel should be passed to the radar detection
> command instead? That way, it can be passed through, you can allocate a
> channel context, etc. Much easier?

I was thinking if putting the radar detection parameter in start_ap() is 
an option (and thus removing the explicit radar cmd)? We could defer 
`netif_carrier_on()` to be done after the initial radar detection is done.

This way the whole thing would be atomic and channel accounting would be 
fine.


-- Pozdrawiam / Best regards, Michal Kazior.

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Johannes Berg]

On Wed, 2012-11-14 at 13:32 +0100, Michal Kazior wrote:

> > Hmm. Maybe then the channel should be passed to the radar detection
> > command instead? That way, it can be passed through, you can allocate a
> > channel context, etc. Much easier?
> 
> I was thinking if putting the radar detection parameter in start_ap() is 
> an option (and thus removing the explicit radar cmd)? We could defer 
> `netif_carrier_on()` to be done after the initial radar detection is done.

Yeah, I was actually thinking that too for a bit, not sure if it works
though.

johannes

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Victor Goldenshtein]

On 14/11/2012 14:38, Johannes Berg wrote:
> On Wed, 2012-11-14 at 13:32 +0100, Michal Kazior wrote:
>
>>> Hmm. Maybe then the channel should be passed to the radar detection
>>> command instead? That way, it can be passed through, you can allocate a
>>> channel context, etc. Much easier?
>>
>> I was thinking if putting the radar detection parameter in start_ap() is
>> an option (and thus removing the explicit radar cmd)? We could defer
>> `netif_carrier_on()` to be done after the initial radar detection is done.
>
> Yeah, I was actually thinking that too for a bit, not sure if it works
> though.
>

It won't, well.. at least not with current design. Userspace has the 
responsibility to set the initial channel and reselect the next one if 
necessary (if radar was detected), also it's responsible for the channel 
availability check and its timing.

Moving radar detection to start AP will require additional DFS state 
synchronization between kernel and userspace this why we initially 
agreed to leave the DFS logic (CAC, channel selection etc..) in 
userspace. Anyway, not sure what would we gain here.


-- 
Thanks,
Victor.

Re: [PATCH v4 1/6] nl80211/cfg80211: add radar detection command/event

[Johannes Berg]

On Tue, 2012-11-20 at 17:15 +0200, Victor Goldenshtein wrote:
> On 14/11/2012 14:38, Johannes Berg wrote:
> > On Wed, 2012-11-14 at 13:32 +0100, Michal Kazior wrote:
> >
> >>> Hmm. Maybe then the channel should be passed to the radar detection
> >>> command instead? That way, it can be passed through, you can allocate a
> >>> channel context, etc. Much easier?
> >>
> >> I was thinking if putting the radar detection parameter in start_ap() is
> >> an option (and thus removing the explicit radar cmd)? We could defer
> >> `netif_carrier_on()` to be done after the initial radar detection is done.
> >
> > Yeah, I was actually thinking that too for a bit, not sure if it works
> > though.
> >
> 
> It won't, well.. at least not with current design. Userspace has the 
> responsibility to set the initial channel and reselect the next one if 
> necessary (if radar was detected), also it's responsible for the channel 
> availability check and its timing.
> 
> Moving radar detection to start AP will require additional DFS state 
> synchronization between kernel and userspace this why we initially 
> agreed to leave the DFS logic (CAC, channel selection etc..) in 
> userspace. Anyway, not sure what would we gain here.

Fair enough. I think we can solve it differently, like I outlined in my
first mail today.

johannes

[PATCH v4 2/6] mac80211: add radar detection command/event

[Victor Goldenshtein]

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.

Signed-off-by: Victor Goldenshtein <victorg@ti.com>
---
 include/net/mac80211.h    |   19 ++++++++++++++++++
 net/mac80211/cfg.c        |   14 +++++++++++++
 net/mac80211/driver-ops.h |   14 +++++++++++++
 net/mac80211/iface.c      |    5 ++++
 net/mac80211/mlme.c       |   11 ++++++++++
 net/mac80211/trace.h      |   46 +++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 109 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 82558c8..67f932d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2317,6 +2317,10 @@ enum ieee80211_rate_control_changed {
  *	The callback will be called before each transmission and upon return
  *	mac80211 will transmit the frame right away.
  *	The callback is optional and can (should!) sleep.
+ *
+ * @start_radar_detection: Start radar detection on current operational
+ *	channel, once started it will continuously monitor for radars as long
+ *	as the channel active. This callback can sleep.
  */
 struct ieee80211_ops {
 	void (*tx)(struct ieee80211_hw *hw,
@@ -2461,6 +2465,10 @@ struct ieee80211_ops {
 
 	void	(*mgd_prepare_tx)(struct ieee80211_hw *hw,
 				  struct ieee80211_vif *vif);
+
+	int (*start_radar_detection)(struct ieee80211_hw *hw,
+				     struct ieee80211_vif *vif,
+				     struct ieee80211_channel *chan);
 };
 
 /**
@@ -3633,6 +3641,17 @@ 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.
+ * @chan: radar detected on this channel.
+ * @gfp: context flags.
+ */
+void ieee80211_radar_detected(struct ieee80211_vif *vif,
+			      struct ieee80211_channel *chan, 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 05f3a31..a6c5369 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2446,6 +2446,19 @@ 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 ieee80211_channel *chan)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct ieee80211_local *local = sdata->local;
+
+	if (!local->ops->start_radar_detection)
+		return -EOPNOTSUPP;
+
+	return drv_start_radar_detection(local, sdata, chan);
+}
+
 static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 			     struct ieee80211_channel *chan, bool offchan,
 			     enum nl80211_channel_type channel_type,
@@ -3131,4 +3144,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/driver-ops.h b/net/mac80211/driver-ops.h
index da9003b..dfb992e 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -323,6 +323,20 @@ static inline void drv_cancel_hw_scan(struct ieee80211_local *local,
 	trace_drv_return_void(local);
 }
 
+static inline int drv_start_radar_detection(struct ieee80211_local *local,
+					    struct ieee80211_sub_if_data *sdata,
+					    struct ieee80211_channel *chan)
+{
+	int ret;
+
+	might_sleep();
+
+	trace_drv_start_radar_detection(local, chan);
+	ret = local->ops->start_radar_detection(&local->hw, &sdata->vif, chan);
+	trace_drv_return_int(local, ret);
+	return ret;
+}
+
 static inline int
 drv_sched_scan_start(struct ieee80211_local *local,
 		     struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 6f8a73c..02a8382 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -757,6 +757,11 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
 		/* free all potentially still buffered bcast frames */
 		local->total_ps_buffered -= skb_queue_len(&sdata->u.ap.ps_bc_buf);
 		skb_queue_purge(&sdata->u.ap.ps_bc_buf);
+
+		/* reset DFS channel availability check */
+		if (local->oper_channel)
+			local->oper_channel->cac_started = false;
+
 	} else if (sdata->vif.type == NL80211_IFTYPE_STATION) {
 		ieee80211_mgd_stop(sdata);
 	}
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index e714ed8..208b835 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -3650,3 +3650,14 @@ 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,
+			      struct ieee80211_channel *chan, gfp_t gfp)
+{
+	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+
+	trace_api_radar_detected(sdata, chan->center_freq);
+
+	cfg80211_radar_detected(sdata->dev, chan, gfp);
+}
+EXPORT_SYMBOL(ieee80211_radar_detected);
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index 18d9c8a..cbebd52 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -1645,6 +1645,52 @@ TRACE_EVENT(stop_queue,
 	)
 );
 
+TRACE_EVENT(api_radar_detected,
+	TP_PROTO(struct ieee80211_sub_if_data *sdata,
+		 u16 center_freq),
+
+	TP_ARGS(sdata, center_freq),
+
+	TP_STRUCT__entry(
+		VIF_ENTRY
+		__field(u16, center_freq)
+	),
+
+	TP_fast_assign(
+		VIF_ASSIGN;
+		__entry->center_freq = center_freq;
+	),
+
+	TP_printk(
+		VIF_PR_FMT " radar on freq:%d",
+		VIF_PR_ARG, __entry->center_freq
+	)
+)
+
+TRACE_EVENT(drv_start_radar_detection,
+	TP_PROTO(struct ieee80211_local *local,
+		 struct ieee80211_channel *chan),
+
+	TP_ARGS(local, chan),
+
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		__field(u16, freq)
+		__field(int, cac_type)
+	),
+
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		__entry->freq = chan->center_freq;
+		__entry->cac_type = chan->cac_type;
+	),
+
+	TP_printk(
+		LOCAL_PR_FMT " start radar detection on freq:%u cac_type:%d",
+		LOCAL_PR_ARG, __entry->freq, __entry->cac_type
+	)
+);
+
 #ifdef CONFIG_MAC80211_MESSAGE_TRACING
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM mac80211_msg
-- 
1.7.5.4

[PATCH v4 3/6] nl80211/cfg80211: add ability to enable TX on op-channel

[Victor Goldenshtein]

The dfs master device should monitor radar channels
for potential radar interference for a minimum of
CAC (channel availability check) time, during this
period no tx can occur. If no radar interference
is detected the dfs master may initiate the tx with
new NL80211_CMD_DFS_ENABLE_TX command.

If this command is invoked prior performing a CAC or
the time passed since the beginning of the CAC is
less than min CAC time (60 sec), -EPERM is returned.

Signed-off-by: Victor Goldenshtein <victorg@ti.com>
---
 include/linux/nl80211.h |   11 +++++++++++
 include/net/cfg80211.h  |    5 +++++
 net/wireless/nl80211.c  |   37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 0 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index ba4c697..ee3336c 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -581,6 +581,15 @@
  * @NL80211_CMD_RADAR_DETECT: Start radar detection in the driver/HW. Once
  *	radar detected usermode notified with this event.
  *
+ * @NL80211_CMD_DFS_ENABLE_TX: Initiate tx after verifying radar clearness on
+ *	dfs channel. The dfs master device should monitor radar channels
+ *	for potential radar interference for a minimum of CAC (channel
+ *	availability check) time, during this period no tx can occur. If no
+ *	radar interference is detected during this period the dfs master may
+ *	initiate the tx. If this command is invoked prior performing a CAC or
+ *	the time passed since the beginning of the CAC is less than
+ *	NL80211_DFS_MIN_CAC_TIME_MS, -EPERM is returned.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -731,6 +740,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_RADAR_DETECT,
 
+	NL80211_CMD_DFS_ENABLE_TX,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 238d0e2..f2d4bde 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -1631,6 +1631,8 @@ struct cfg80211_gtk_rekey_data {
  * @stop_p2p_device: Stop the given P2P device.
  *
  * @start_radar_detection: Start radar detection in the driver.
+ *
+ * @dfs_en_tx: Enable tx after radar interference check.
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -1855,6 +1857,9 @@ struct cfg80211_ops {
 	int	(*start_radar_detection)(struct wiphy *wiphy,
 					 struct net_device *dev,
 					 struct ieee80211_channel *chan);
+
+	int	(*dfs_en_tx)(struct wiphy *wiphy, struct net_device *dev,
+			     struct ieee80211_channel *chan);
 };
 
 /*
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4c83c28..be4e160 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4648,6 +4648,35 @@ static int nl80211_start_radar_detection(struct sk_buff *skb,
 	return err;
 }
 
+static int nl80211_dfs_en_tx(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 ieee80211_channel *chan;
+	int freq;
+
+	if (!rdev->ops->dfs_en_tx ||
+	    !(rdev->wiphy.features & NL80211_FEATURE_20MHZ_DFS))
+		return -EOPNOTSUPP;
+
+	if (info->attrs[NL80211_ATTR_WIPHY_FREQ])
+		freq = nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]);
+	else
+		return -EINVAL;
+
+	chan = ieee80211_get_channel(&rdev->wiphy, freq);
+	if (!chan)
+		return -EINVAL;
+
+	if ((((chan->cac_type != NL80211_CHAN_NO_HT) &&
+	      (chan->cac_type != NL80211_CHAN_HT20)) || !chan->cac_started ||
+	     time_is_after_jiffies(chan->radar_detect_timeout)) &&
+	    (chan->flags & IEEE80211_CHAN_RADAR))
+		return -EPERM;
+
+	return rdev->ops->dfs_en_tx(&rdev->wiphy, dev, chan);
+}
+
 static int nl80211_send_bss(struct sk_buff *msg, struct netlink_callback *cb,
 			    u32 seq, int flags,
 			    struct cfg80211_registered_device *rdev,
@@ -7604,6 +7633,14 @@ static struct genl_ops nl80211_ops[] = {
 		.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
 				  NL80211_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL80211_CMD_DFS_ENABLE_TX,
+		.doit = nl80211_dfs_en_tx,
+		.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 = {
-- 
1.7.5.4

[PATCH v4 4/6] mac80211: add ability to enable TX on op-channel

[Victor Goldenshtein]

Prior starting tx on DFS channels, the DFS master
device shall perform a Channel Availability Check
to ensure that there is no radar interference on
those channels. Once CAC done, the tx can be
enabled with ieee80211_dfs_en_tx().

Signed-off-by: Victor Goldenshtein <victorg@ti.com>
---
 include/net/mac80211.h    |    5 +++++
 net/mac80211/cfg.c        |   15 +++++++++++++++
 net/mac80211/driver-ops.h |   14 ++++++++++++++
 net/mac80211/trace.h      |   24 ++++++++++++++++++++++++
 4 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 67f932d..725018f 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2321,6 +2321,9 @@ enum ieee80211_rate_control_changed {
  * @start_radar_detection: Start radar detection on current operational
  *	channel, once started it will continuously monitor for radars as long
  *	as the channel active. This callback can sleep.
+ * @dfs_en_tx: Once channel pass the DFS initial channel availability check,
+ *	initiate the tx on the channel with this command. This callback can
+ *	sleep.
  */
 struct ieee80211_ops {
 	void (*tx)(struct ieee80211_hw *hw,
@@ -2469,6 +2472,8 @@ struct ieee80211_ops {
 	int (*start_radar_detection)(struct ieee80211_hw *hw,
 				     struct ieee80211_vif *vif,
 				     struct ieee80211_channel *chan);
+	int (*dfs_en_tx)(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
+			 struct ieee80211_channel *chan);
 };
 
 /**
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index a6c5369..b706232 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2459,6 +2459,20 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy,
 	return drv_start_radar_detection(local, sdata, chan);
 }
 
+static int ieee80211_dfs_en_tx(struct wiphy *wiphy, struct net_device *dev,
+			       struct ieee80211_channel *chan)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct ieee80211_local *local = sdata->local;
+	int ret = -ENOENT;
+
+	if (!local->ops->dfs_en_tx)
+		return -EOPNOTSUPP;
+
+	ret = drv_dfs_en_tx(local, sdata, chan);
+	return ret;
+}
+
 static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 			     struct ieee80211_channel *chan, bool offchan,
 			     enum nl80211_channel_type channel_type,
@@ -3145,4 +3159,5 @@ struct cfg80211_ops mac80211_config_ops = {
 	.get_et_strings = ieee80211_get_et_strings,
 	.get_channel = ieee80211_cfg_get_channel,
 	.start_radar_detection = ieee80211_start_radar_detection,
+	.dfs_en_tx = ieee80211_dfs_en_tx,
 };
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index dfb992e..bca55a3 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -337,6 +337,20 @@ static inline int drv_start_radar_detection(struct ieee80211_local *local,
 	return ret;
 }
 
+static inline int drv_dfs_en_tx(struct ieee80211_local *local,
+				struct ieee80211_sub_if_data *sdata,
+				struct ieee80211_channel *chan)
+{
+	int ret;
+
+	might_sleep();
+
+	trace_drv_dfs_en_tx(local, chan);
+	ret = local->ops->dfs_en_tx(&local->hw, &sdata->vif, chan);
+	trace_drv_return_int(local, ret);
+	return ret;
+}
+
 static inline int
 drv_sched_scan_start(struct ieee80211_local *local,
 		     struct ieee80211_sub_if_data *sdata,
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index cbebd52..e6e066e 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -1691,6 +1691,30 @@ TRACE_EVENT(drv_start_radar_detection,
 	)
 );
 
+TRACE_EVENT(drv_dfs_en_tx,
+	TP_PROTO(struct ieee80211_local *local,
+		 struct ieee80211_channel *chan),
+
+	TP_ARGS(local, chan),
+
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		__field(u16, freq)
+		__field(int, cac_type)
+	),
+
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		__entry->freq = chan->center_freq;
+		__entry->cac_type = chan->cac_type;
+	),
+
+	TP_printk(
+		LOCAL_PR_FMT " enable transmission on freq:%u cac_type:%d",
+		LOCAL_PR_ARG, __entry->freq, __entry->cac_type
+	)
+);
+
 #ifdef CONFIG_MAC80211_MESSAGE_TRACING
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM mac80211_msg
-- 
1.7.5.4

Re: [PATCH v4 4/6] mac80211: add ability to enable TX on op-channel

[Johannes Berg]

On Sun, 2012-10-14 at 16:48 +0200, Victor Goldenshtein wrote:

> +static int ieee80211_dfs_en_tx(struct wiphy *wiphy, struct net_device *dev,
> +			       struct ieee80211_channel *chan)
> +{
> +	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> +	struct ieee80211_local *local = sdata->local;
> +	int ret = -ENOENT;
> +
> +	if (!local->ops->dfs_en_tx)
> +		return -EOPNOTSUPP;
> +
> +	ret = drv_dfs_en_tx(local, sdata, chan);
> +	return ret;

What's with the "ret" variable here?

johannes

Re: [PATCH v4 4/6] mac80211: add ability to enable TX on op-channel

[Victor Goldenshtein]

On 16/10/2012 21:03, Johannes Berg wrote:
> On Sun, 2012-10-14 at 16:48 +0200, Victor Goldenshtein wrote:
>
>> +static int ieee80211_dfs_en_tx(struct wiphy *wiphy, struct net_device *dev,
>> +			       struct ieee80211_channel *chan)
>> +{
>> +	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
>> +	struct ieee80211_local *local = sdata->local;
>> +	int ret = -ENOENT;
>> +
>> +	if (!local->ops->dfs_en_tx)
>> +		return -EOPNOTSUPP;
>> +
>> +	ret = drv_dfs_en_tx(local, sdata, chan);
>> +	return ret;
>

sorry, missed that one - I can resend v5 ?

-- 
Thanks,
Victor.

[PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Victor Goldenshtein]

Add NL80211_CMD_AP_CH_SWITCH command which
triggers an AP channel switch process.

Usermode notified about channel switch complete
event with NL80211_CMD_CH_SWITCH_NOTIFY.

Usermode (hostapd) is responsible to update the
channel switch announcement IE in the beacon
prior and after the channel switch operation.

Signed-off-by: Victor Goldenshtein <victorg@ti.com>
---
 include/linux/nl80211.h |   27 ++++++++++++++++++++
 include/net/cfg80211.h  |   36 +++++++++++++++++++++++++++
 net/wireless/nl80211.c  |   62 +++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 123 insertions(+), 2 deletions(-)

diff --git a/include/linux/nl80211.h b/include/linux/nl80211.h
index ee3336c..0538300 100644
--- a/include/linux/nl80211.h
+++ b/include/linux/nl80211.h
@@ -590,6 +590,17 @@
  *	the time passed since the beginning of the CAC is less than
  *	NL80211_DFS_MIN_CAC_TIME_MS, -EPERM is returned.
  *
+ * @NL80211_CMD_AP_CH_SWITCH: Perform a channel switch in the driver (for
+ *	AP/GO). Userspace daemon is responsible to update the CSA IE in the
+ *	beacon prior and after the channel switch operation. Userspace notified
+ *	about channel switch complete event with %NL80211_CMD_CH_SWITCH_NOTIFY.
+ *	%NL80211_ATTR_WIPHY_FREQ is used to specify the new channel frequency.
+ *	%NL80211_ATTR_CH_SWITCH_BLOCK_TX is used to specify that tx must be
+ *	blocked on the current channel. %NL80211_ATTR_CH_SWITCH_POST_BLOCK_TX
+ *	is used to specify that tx must be blocked on the target channel.
+ *	%NL80211_FREQ_ATTR_CH_SWITCH_COUNT to specify the number of TBTT's until
+ *	the device switches to the target channel.
+ *
  * @NL80211_CMD_MAX: highest used command number
  * @__NL80211_CMD_AFTER_LAST: internal use
  */
@@ -742,6 +753,8 @@ enum nl80211_commands {
 
 	NL80211_CMD_DFS_ENABLE_TX,
 
+	NL80211_CMD_AP_CH_SWITCH,
+
 	/* add new commands above here */
 
 	/* used to define NL80211_CMD_MAX below */
@@ -1289,6 +1302,15 @@ enum nl80211_commands {
  *	the connection request from a station. nl80211_connect_failed_reason
  *	enum has different reasons of connection failure.
  *
+ * @NL80211_ATTR_CH_SWITCH_COUNT: u32 attribute specifying the number of TBTT's
+ *	until the channel switch event.
+ * @NL80211_ATTR_CH_SWITCH_BLOCK_TX: flag attribute specifying that transmission
+ *	must be blocked on the current channel (before the channel switch
+ *	operation).
+ * @NL80211_ATTR_CH_SWITCH_POST_BLOCK_TX: flag attribute specifying that
+ *	transmission must be blocked on the target channel after the channel
+ *	switch operation, should be set if the target channel is DFS channel.
+ *
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
  */
@@ -1546,6 +1568,10 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_CONN_FAILED_REASON,
 
+	NL80211_ATTR_CH_SWITCH_COUNT,
+	NL80211_ATTR_CH_SWITCH_BLOCK_TX,
+	NL80211_ATTR_CH_SWITCH_POST_BLOCK_TX,
+
 	/* add attributes here, update the policy in nl80211.c */
 
 	__NL80211_ATTR_AFTER_LAST,
@@ -3056,6 +3082,7 @@ enum nl80211_feature_flags {
 	NL80211_FEATURE_CELL_BASE_REG_HINTS		= 1 << 3,
 	NL80211_FEATURE_P2P_DEVICE_NEEDS_CHANNEL	= 1 << 4,
 	NL80211_FEATURE_20MHZ_DFS			= 1 << 5,
+	NL80211_FEATURE_AP_CH_SWITCH			= 1 << 6,
 };
 
 /**
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index f2d4bde..ceff832 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -154,6 +154,36 @@ struct ieee80211_channel {
 };
 
 /**
+ * struct ieee80211_ap_ch_switch - holds the ap channel switch data
+ *
+ * The information provided in this structure is required for the ap channel
+ * switch operation.
+ *
+ * @timestamp: value in microseconds of the 64-bit Time Synchronization
+ *	Function (TSF) timer when the frame containing the channel switch
+ *	announcement was received. This is simply the rx.mactime parameter
+ *	the driver passed into mac80211.
+ * @block_tx: Indicates whether transmission must be blocked before the
+ *	scheduled channel switch, setting this flag will rise Channel Switch
+ *	Mode bit in the AP CSA IE which means that all STAs in a BSS shall
+ *	stop transmitting immediately.
+ * @post_switch_block_tx: Indicates whether the AP transmission must be blocked
+ *	after the scheduled channel switch, this should be set if the target
+ *	channel is DFS channel.
+ * @channel: the new channel to switch to
+ * @channel_type: the type of the new channel
+ * @count: the number of TBTT's until the channel switch event
+ */
+struct ieee80211_ap_ch_switch {
+	u64 timestamp;
+	bool block_tx;
+	bool post_switch_block_tx;
+	struct ieee80211_channel *channel;
+	enum nl80211_channel_type channel_type;
+	u8 count;
+};
+
+/**
  * enum ieee80211_rate_flags - rate flags
  *
  * Hardware/specification flags for rates. These are structured
@@ -1633,6 +1663,8 @@ struct cfg80211_gtk_rekey_data {
  * @start_radar_detection: Start radar detection in the driver.
  *
  * @dfs_en_tx: Enable tx after radar interference check.
+ *
+ * @ap_channel_switch: Perform AP channel switch.
  */
 struct cfg80211_ops {
 	int	(*suspend)(struct wiphy *wiphy, struct cfg80211_wowlan *wow);
@@ -1860,6 +1892,10 @@ struct cfg80211_ops {
 
 	int	(*dfs_en_tx)(struct wiphy *wiphy, struct net_device *dev,
 			     struct ieee80211_channel *chan);
+
+	int	(*ap_channel_switch)(struct wiphy *wiphy,
+				     struct net_device *dev,
+				     struct ieee80211_ap_ch_switch *ap_ch_sw);
 };
 
 /*
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index be4e160..a40e81e 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -355,6 +355,9 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = {
 	[NL80211_ATTR_BG_SCAN_PERIOD] = { .type = NLA_U16 },
 	[NL80211_ATTR_WDEV] = { .type = NLA_U64 },
 	[NL80211_ATTR_USER_REG_HINT_TYPE] = { .type = NLA_U32 },
+	[NL80211_ATTR_CH_SWITCH_COUNT] = { .type = NLA_U32 },
+	[NL80211_ATTR_CH_SWITCH_BLOCK_TX] = { .type = NLA_FLAG },
+	[NL80211_ATTR_CH_SWITCH_POST_BLOCK_TX] = { .type = NLA_FLAG },
 };
 
 /* policy for the key attributes */
@@ -4614,7 +4617,8 @@ static int nl80211_start_radar_detection(struct sk_buff *skb,
 	struct ieee80211_channel *chan;
 	enum nl80211_channel_type cac_type;
 
-	if (!(rdev->wiphy.features & NL80211_FEATURE_20MHZ_DFS))
+	if (!(rdev->wiphy.features & NL80211_FEATURE_20MHZ_DFS) ||
+	    !(rdev->wiphy.features & NL80211_FEATURE_AP_CH_SWITCH))
 		return -EOPNOTSUPP;
 
 	if (!info->attrs[NL80211_ATTR_WIPHY_FREQ] ||
@@ -4656,7 +4660,8 @@ static int nl80211_dfs_en_tx(struct sk_buff *skb, struct genl_info *info)
 	int freq;
 
 	if (!rdev->ops->dfs_en_tx ||
-	    !(rdev->wiphy.features & NL80211_FEATURE_20MHZ_DFS))
+	    !(rdev->wiphy.features & NL80211_FEATURE_20MHZ_DFS) ||
+	    !(rdev->wiphy.features & NL80211_FEATURE_AP_CH_SWITCH))
 		return -EOPNOTSUPP;
 
 	if (info->attrs[NL80211_ATTR_WIPHY_FREQ])
@@ -4677,6 +4682,51 @@ static int nl80211_dfs_en_tx(struct sk_buff *skb, struct genl_info *info)
 	return rdev->ops->dfs_en_tx(&rdev->wiphy, dev, chan);
 }
 
+static int nl80211_ap_channel_switch(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];
+	u32 freq = 0;
+	struct ieee80211_ap_ch_switch ap_ch_sw;
+
+	if (!rdev->ops->ap_channel_switch)
+		return -EOPNOTSUPP;
+
+	if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
+	    dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
+		return -EOPNOTSUPP;
+
+	if (!info->attrs[NL80211_ATTR_CH_SWITCH_COUNT] ||
+	    !info->attrs[NL80211_ATTR_WIPHY_FREQ] ||
+	    !info->attrs[NL80211_ATTR_WIPHY_CHANNEL_TYPE])
+		return -EINVAL;
+
+	memset(&ap_ch_sw, 0, sizeof(ap_ch_sw));
+
+	ap_ch_sw.channel_type =
+		nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_CHANNEL_TYPE]);
+	if ((ap_ch_sw.channel_type != NL80211_CHAN_HT20) &&
+	    (ap_ch_sw.channel_type != NL80211_CHAN_NO_HT))
+		return -EOPNOTSUPP;
+
+	ap_ch_sw.count = nla_get_u32(info->attrs[NL80211_ATTR_CH_SWITCH_COUNT]);
+	freq = nla_get_u32(info->attrs[NL80211_ATTR_WIPHY_FREQ]);
+
+	ap_ch_sw.channel = ieee80211_get_channel(&rdev->wiphy, freq);
+	if (!ap_ch_sw.channel ||
+	    ap_ch_sw.channel->flags & IEEE80211_CHAN_DISABLED)
+		return -EINVAL;
+
+	if (info->attrs[NL80211_ATTR_CH_SWITCH_BLOCK_TX])
+		ap_ch_sw.block_tx = true;
+
+	if (info->attrs[NL80211_ATTR_CH_SWITCH_POST_BLOCK_TX])
+		ap_ch_sw.post_switch_block_tx = true;
+
+	return rdev->ops->ap_channel_switch(&rdev->wiphy, dev, &ap_ch_sw);
+}
+
 static int nl80211_send_bss(struct sk_buff *msg, struct netlink_callback *cb,
 			    u32 seq, int flags,
 			    struct cfg80211_registered_device *rdev,
@@ -7641,6 +7691,14 @@ static struct genl_ops nl80211_ops[] = {
 		.internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
 				  NL80211_FLAG_NEED_RTNL,
 	},
+	{
+		.cmd = NL80211_CMD_AP_CH_SWITCH,
+		.doit = nl80211_ap_channel_switch,
+		.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 = {
-- 
1.7.5.4

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Johannes Berg]

On Sun, 2012-10-14 at 16:48 +0200, Victor Goldenshtein wrote:
> Add NL80211_CMD_AP_CH_SWITCH command which
> triggers an AP channel switch process.
> 
> Usermode notified about channel switch complete
> event with NL80211_CMD_CH_SWITCH_NOTIFY.
> 
> Usermode (hostapd) is responsible to update the
> channel switch announcement IE in the beacon
> prior and after the channel switch operation.

Hmm, does that really work? I mean, it can do the update when the event
comes in, but is that fast enough, or should it give us the new beacon
already when programming the switching?

johannes

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Victor Goldenshtein]

On 16/10/2012 21:06, Johannes Berg wrote:
> Hmm, does that really work? I mean, it can do the update when the event
> comes in, but is that fast enough, or should it give us the new beacon
> already when programming the switching?
>

Yes it works with our 12xx, in the 12xx the FW (upon channel switch 
command) is responsible to decrement the channel switch count in the 
beacon CSA IE and to remove the CSA IE from the beacon once device moves 
to the new channel (+ to updates the DS).

-- 
Thanks,
Victor.

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Johannes Berg]

On Sun, 2012-10-21 at 18:40 +0200, Victor Goldenshtein wrote:
> On 16/10/2012 21:06, Johannes Berg wrote:
> > Hmm, does that really work? I mean, it can do the update when the event
> > comes in, but is that fast enough, or should it give us the new beacon
> > already when programming the switching?
> >
> 
> Yes it works with our 12xx, in the 12xx the FW (upon channel switch 
> command) is responsible to decrement the channel switch count in the 
> beacon CSA IE and to remove the CSA IE from the beacon once device moves 
> to the new channel (+ to updates the DS).

So ... you've made this kind of behaviour a mandatory part of the
userspace API. Where is it documented then? How will it work with other
devices?

johannes

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Victor Goldenshtein]

On 22/10/2012 14:52, Johannes Berg wrote:
> On Sun, 2012-10-21 at 18:40 +0200, Victor Goldenshtein wrote:
>> On 16/10/2012 21:06, Johannes Berg wrote:
>>> Hmm, does that really work? I mean, it can do the update when the event
>>> comes in, but is that fast enough, or should it give us the new beacon
>>> already when programming the switching?
>>>
>>
>> Yes it works with our 12xx, in the 12xx the FW (upon channel switch
>> command) is responsible to decrement the channel switch count in the
>> beacon CSA IE and to remove the CSA IE from the beacon once device moves
>> to the new channel (+ to updates the DS).
>
> So ... you've made this kind of behaviour a mandatory part of the
> userspace API. Where is it documented then? How will it work with other
> devices?

It's should work the same with other devices, each device should 
implement AP channel switch op according the spec, the implementation 
can be hardware/driver depended. I just mentioned how we do it in our 
devices, which I think is the right way without introducing any races 
(as you mentioned), So no behavior enforcement here as long as it 
according the spec.

-- 
Thanks,
Victor.

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Johannes Berg]

On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:

> >> Yes it works with our 12xx, in the 12xx the FW (upon channel switch
> >> command) is responsible to decrement the channel switch count in the
> >> beacon CSA IE and to remove the CSA IE from the beacon once device moves
> >> to the new channel (+ to updates the DS).
> >
> > So ... you've made this kind of behaviour a mandatory part of the
> > userspace API. Where is it documented then? How will it work with other
> > devices?
> 
> It's should work the same with other devices, each device should 
> implement AP channel switch op according the spec, the implementation 
> can be hardware/driver depended. I just mentioned how we do it in our 
> devices, which I think is the right way without introducing any races 
> (as you mentioned), So no behavior enforcement here as long as it 
> according the spec.

I don't think that's how it works ... you should describe what is
expected of the driver in the right API documentation. Even saying
"implement the procedures as described in 802.11-2012 section ..." could
be sufficient, but I think that needs to be there.

johannes

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Victor Goldenshtein]

On 05/11/2012 17:27, Johannes Berg wrote:
> On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:
>
>>>> Yes it works with our 12xx, in the 12xx the FW (upon channel switch
>>>> command) is responsible to decrement the channel switch count in the
>>>> beacon CSA IE and to remove the CSA IE from the beacon once device moves
>>>> to the new channel (+ to updates the DS).
>>>
>>> So ... you've made this kind of behaviour a mandatory part of the
>>> userspace API. Where is it documented then? How will it work with other
>>> devices?
>>
>> It's should work the same with other devices, each device should
>> implement AP channel switch op according the spec, the implementation
>> can be hardware/driver depended. I just mentioned how we do it in our
>> devices, which I think is the right way without introducing any races
>> (as you mentioned), So no behavior enforcement here as long as it
>> according the spec.
>
> I don't think that's how it works ... you should describe what is
> expected of the driver in the right API documentation. Even saying
> "implement the procedures as described in 802.11-2012 section ..." could
> be sufficient, but I think that needs to be there.
>

np, I can add it.

-- 
Thanks,
Victor.

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Michal Kazior]

On 14/10/12 16:48, Victor Goldenshtein wrote:
> +static int nl80211_ap_channel_switch(struct sk_buff *skb,
> +				     struct genl_info *info)
> +{

This function should probably enforce interface combinations with 
`cfg80211_can_use_chan()`. Otherwise we might end up in a state which 
doesn't correspond to any interface combination - and won't be able to 
bring up new interfaces.

This however introduces a new problem. Let's suppose we have 2 APs on 
channel 1. The device doesn't support multi-channel. We won't be able to 
switch channel on these APs at all.

We might want to change the channel switch to resolve around the channel 
itself (not the interface) - so we'd be saying "move all interfaces with 
channel X to channel Y" instead of "move interface X to channel Y".

Or we could let the driver decide what it'll do - e.g. silently switch 
more than one interface to a different channel (which makes sense with 
AP/DFS I guess) and just notify cfg/userspace about it. That would 
require us to provide a way to switch interfaces (atomically possibly) 
between channels while keeping in sync with interface combinations though.

We might try addressing station CSA issue at the same time maybe?


-- Pozdrawiam / Best regards, Michal Kazior.

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Victor Goldenshtein]

On 23/10/2012 08:20, Michal Kazior wrote:
> On 14/10/12 16:48, Victor Goldenshtein wrote:
>> +static int nl80211_ap_channel_switch(struct sk_buff *skb,
>> +                     struct genl_info *info)
>> +{
>
> This function should probably enforce interface combinations with
> `cfg80211_can_use_chan()`. Otherwise we might end up in a state which
> doesn't correspond to any interface combination - and won't be able to
> bring up new interfaces.
>

I guess you're right, we probably should check here whether we can 
switch to the next channel (with cfg80211_can_use_chan) and in the case 
we're not, the AP/GO should be disabled.

> This however introduces a new problem. Let's suppose we have 2 APs on
> channel 1. The device doesn't support multi-channel. We won't be able to
> switch channel on these APs at all.
>
> We might want to change the channel switch to resolve around the channel
> itself (not the interface) - so we'd be saying "move all interfaces with
> channel X to channel Y" instead of "move interface X to channel Y".
>
> Or we could let the driver decide what it'll do - e.g. silently switch
> more than one interface to a different channel (which makes sense with
> AP/DFS I guess) and just notify cfg/userspace about it. That would
> require us to provide a way to switch interfaces (atomically possibly)
> between channels while keeping in sync with interface combinations though.
>

If the driver/device supports MR only on a SC - means it doesn't 
supports channel switch in MR, so basically the radar detection event 
triggers AP channel switch which fails (with this new check) and the AP 
shut down.
Of course there are possible driver specific workarounds (as you 
mentioned above) but these are not part of this series.

Just for interest's sake, what is the use case having two APs on the 
same channel?

-- 
Thanks,
Victor.

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Adrian Chadd]

On 1 November 2012 00:54, Victor Goldenshtein <victorg@ti.com> wrote:

> If the driver/device supports MR only on a SC - means it doesn't supports
> channel switch in MR, so basically the radar detection event triggers AP
> channel switch which fails (with this new check) and the AP shut down.
> Of course there are possible driver specific workarounds (as you mentioned
> above) but these are not part of this series.
>
> Just for interest's sake, what is the use case having two APs on the same
> channel?

One interface _just_ doing DFS/spectral/etc, and not hampered by
trying to do TX/RX.



Adrian

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Johannes Berg]

On Thu, 2012-11-01 at 10:15 -0700, Adrian Chadd wrote:
> On 1 November 2012 00:54, Victor Goldenshtein <victorg@ti.com> wrote:
> 
> > If the driver/device supports MR only on a SC - means it doesn't supports
> > channel switch in MR, so basically the radar detection event triggers AP
> > channel switch which fails (with this new check) and the AP shut down.
> > Of course there are possible driver specific workarounds (as you mentioned
> > above) but these are not part of this series.
> >
> > Just for interest's sake, what is the use case having two APs on the same
> > channel?
> 
> One interface _just_ doing DFS/spectral/etc, and not hampered by
> trying to do TX/RX.

Or maybe just multi-BSSID use cases?!

johannes

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Johannes Berg]

On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:

> > This however introduces a new problem. Let's suppose we have 2 APs on
> > channel 1. The device doesn't support multi-channel. We won't be able to
> > switch channel on these APs at all.
> >
> > We might want to change the channel switch to resolve around the channel
> > itself (not the interface) - so we'd be saying "move all interfaces with
> > channel X to channel Y" instead of "move interface X to channel Y".
> >
> > Or we could let the driver decide what it'll do - e.g. silently switch
> > more than one interface to a different channel (which makes sense with
> > AP/DFS I guess) and just notify cfg/userspace about it. That would
> > require us to provide a way to switch interfaces (atomically possibly)
> > between channels while keeping in sync with interface combinations though.
> >
> 
> If the driver/device supports MR only on a SC - means it doesn't 

What's MR? SC = single channel?

> supports channel switch in MR, so basically the radar detection event 
> triggers AP channel switch which fails (with this new check) and the AP 
> shut down.

But I was speaking of two interfaces on a single channel.

> Of course there are possible driver specific workarounds (as you 
> mentioned above) but these are not part of this series.

yeah but you should probably allow for some strategy of handling this?

> Just for interest's sake, what is the use case having two APs on the 
> same channel?

Umm, lots of things? P2P groups, multiple BSSIDs, ...

johannes

Re: [PATCH v4 5/6] nl80211/cfg80211: add ap channel switch command

[Victor Goldenshtein]

On 05/11/2012 17:29, Johannes Berg wrote:
> On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:
>
>>> This however introduces a new problem. Let's suppose we have 2 APs on
>>> channel 1. The device doesn't support multi-channel. We won't be able to
>>> switch channel on these APs at all.
>>>
>>> We might want to change the channel switch to resolve around the channel
>>> itself (not the interface) - so we'd be saying "move all interfaces with
>>> channel X to channel Y" instead of "move interface X to channel Y".
>>>
>>> Or we could let the driver decide what it'll do - e.g. silently switch
>>> more than one interface to a different channel (which makes sense with
>>> AP/DFS I guess) and just notify cfg/userspace about it. That would
>>> require us to provide a way to switch interfaces (atomically possibly)
>>> between channels while keeping in sync with interface combinations though.
>>>
>>
>> If the driver/device supports MR only on a SC - means it doesn't
>
> What's MR? SC = single channel?
>

MR - Multi-Role, SC - Single Channel.

>> supports channel switch in MR, so basically the radar detection event
>> triggers AP channel switch which fails (with this new check) and the AP
>> shut down.
>
> But I was speaking of two interfaces on a single channel.
>

the radar detection is per channel and for this case we have the 
"if·(chan->cac_started)" check, see 6/6.

>> Of course there are possible driver specific workarounds (as you
>> mentioned above) but these are not part of this series.
>
> yeah but you should probably allow for some strategy of handling this?
>

If we talking about the case of running two APs on the same channel on a 
single channel platform then the strategy should be (at least at first 
stage) to shut down both APs after radar detection event.

Later we can discuss about the idea of "switching interfaces" (switch 
both this APs to a new channel).


-- 
Thanks,
Victor.

[PATCH v4 6/6] mac80211: add ap channel switch command/event

[Victor Goldenshtein]

Add ieee80211_ap_process_chanswitch(), to handle a channel switch
request for AP.

Add ieee80211_ap_ch_switch_done() which updates oper_channel
and notifies upper layers about channel switch complete event.

Signed-off-by: Victor Goldenshtein <victorg@ti.com>
---
 include/net/mac80211.h     |   21 ++++++++++++++++++++
 net/mac80211/cfg.c         |   28 ++++++++++++++++++++++++++
 net/mac80211/driver-ops.h  |   12 +++++++++++
 net/mac80211/ieee80211_i.h |    4 +++
 net/mac80211/iface.c       |    2 +
 net/mac80211/main.c        |    3 ++
 net/mac80211/mlme.c        |   42 ++++++++++++++++++++++++++++++++++++++++
 net/mac80211/trace.h       |   46 ++++++++++++++++++++++++++++++++++++++++++++
 8 files changed, 158 insertions(+), 0 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 725018f..48ea3b5 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2426,6 +2426,8 @@ struct ieee80211_ops {
 	void (*flush)(struct ieee80211_hw *hw, bool drop);
 	void (*channel_switch)(struct ieee80211_hw *hw,
 			       struct ieee80211_channel_switch *ch_switch);
+	int (*ap_channel_switch)(struct ieee80211_hw *hw,
+				 struct ieee80211_ap_ch_switch *ap_ch_switch);
 	int (*napi_poll)(struct ieee80211_hw *hw, int budget);
 	int (*set_antenna)(struct ieee80211_hw *hw, u32 tx_ant, u32 rx_ant);
 	int (*get_antenna)(struct ieee80211_hw *hw, u32 *tx_ant, u32 *rx_ant);
@@ -3657,6 +3659,25 @@ void ieee80211_radar_detected(struct ieee80211_vif *vif,
 			      struct ieee80211_channel *chan, gfp_t gfp);
 
 /**
+ * ieee80211_ap_ch_switch_done_work - ap channel switch complete work.
+ *
+ * @work: the channel switch complete work
+ */
+void ieee80211_ap_ch_switch_work(struct work_struct *work);
+
+/**
+ * ieee80211_ap_ch_switch_done - inform and update a configured connection
+ * that channel switch is complete.
+ *
+ * @vif: &struct ieee80211_vif pointer from the add_interface callback.
+ * @new_channel: the new channel.
+ * @type: new channe ltype.
+ */
+void ieee80211_ap_ch_switch_done(struct ieee80211_vif *vif,
+				 struct ieee80211_channel *new_channel,
+				 enum nl80211_channel_type type);
+
+/**
  * 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 b706232..5fbcdcf 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2473,6 +2473,33 @@ static int ieee80211_dfs_en_tx(struct wiphy *wiphy, struct net_device *dev,
 	return ret;
 }
 
+static int
+ieee80211_ap_process_chanswitch(struct wiphy *wiphy,
+				struct net_device *dev,
+				struct ieee80211_ap_ch_switch *ap_ch_switch)
+{
+	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+	struct ieee80211_local *local = sdata->local;
+
+	if (!local->ops->channel_switch)
+		return -EOPNOTSUPP;
+
+	if (!ap_ch_switch || !ap_ch_switch->channel)
+		return -EINVAL;
+
+	if (local->ap_cs_channel)
+		return -EBUSY;
+
+	local->ap_cs_channel = ap_ch_switch->channel;
+	local->ap_cs_type = ap_ch_switch->channel_type;
+
+	ieee80211_stop_queues_by_reason(&local->hw,
+					IEEE80211_QUEUE_STOP_REASON_CH_SW);
+
+	drv_ap_channel_switch(local, ap_ch_switch);
+	return 0;
+}
+
 static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 			     struct ieee80211_channel *chan, bool offchan,
 			     enum nl80211_channel_type channel_type,
@@ -3160,4 +3187,5 @@ struct cfg80211_ops mac80211_config_ops = {
 	.get_channel = ieee80211_cfg_get_channel,
 	.start_radar_detection = ieee80211_start_radar_detection,
 	.dfs_en_tx = ieee80211_dfs_en_tx,
+	.ap_channel_switch = ieee80211_ap_process_chanswitch,
 };
diff --git a/net/mac80211/driver-ops.h b/net/mac80211/driver-ops.h
index bca55a3..5042a12 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -708,6 +708,18 @@ static inline void drv_channel_switch(struct ieee80211_local *local,
 	trace_drv_return_void(local);
 }
 
+static inline int
+drv_ap_channel_switch(struct ieee80211_local *local,
+		      struct ieee80211_ap_ch_switch *ap_ch_switch)
+{
+	int ret = -EOPNOTSUPP;
+	might_sleep();
+	trace_drv_ap_channel_switch(local, ap_ch_switch);
+	if (local->ops->channel_switch)
+		ret = local->ops->ap_channel_switch(&local->hw, ap_ch_switch);
+	trace_drv_return_int(local, ret);
+	return ret;
+}
 
 static inline int drv_set_antenna(struct ieee80211_local *local,
 				  u32 tx_ant, u32 rx_ant)
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 8c80455..08c12fc 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -295,6 +295,7 @@ struct ieee80211_if_ap {
 	atomic_t num_mcast_sta; /* number of stations receiving multicast */
 	int dtim_count;
 	bool dtim_bc_mc;
+	struct work_struct ap_ch_sw_work;
 };
 
 struct ieee80211_if_wds {
@@ -772,6 +773,7 @@ enum queue_stop_reason {
 	IEEE80211_QUEUE_STOP_REASON_AGGREGATION,
 	IEEE80211_QUEUE_STOP_REASON_SUSPEND,
 	IEEE80211_QUEUE_STOP_REASON_SKB_ADD,
+	IEEE80211_QUEUE_STOP_REASON_CH_SW,
 };
 
 #ifdef CONFIG_MAC80211_LEDS
@@ -982,6 +984,8 @@ struct ieee80211_local {
 	struct ieee80211_sub_if_data __rcu *scan_sdata;
 	enum nl80211_channel_type _oper_channel_type;
 	struct ieee80211_channel *oper_channel, *csa_channel;
+	struct ieee80211_channel *ap_cs_channel;
+	enum nl80211_channel_type ap_cs_type;
 
 	/* Temporary remain-on-channel for off-channel operations */
 	struct ieee80211_channel *tmp_channel;
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 02a8382..f18fad7 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1164,6 +1164,8 @@ static void ieee80211_setup_sdata(struct ieee80211_sub_if_data *sdata,
 	case NL80211_IFTYPE_AP:
 		skb_queue_head_init(&sdata->u.ap.ps_bc_buf);
 		INIT_LIST_HEAD(&sdata->u.ap.vlans);
+		INIT_WORK(&sdata->u.ap.ap_ch_sw_work,
+			ieee80211_ap_ch_switch_work);
 		break;
 	case NL80211_IFTYPE_P2P_CLIENT:
 		type = NL80211_IFTYPE_STATION;
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index c80c449..3b440d8 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -587,6 +587,9 @@ struct ieee80211_hw *ieee80211_alloc_hw(size_t priv_data_len,
 	wiphy->features = NL80211_FEATURE_SK_TX_STATUS |
 			  NL80211_FEATURE_HT_IBSS;
 
+	if (ops->ap_channel_switch)
+		wiphy->features |= NL80211_FEATURE_AP_CH_SWITCH;
+
 	if (!ops->set_key)
 		wiphy->flags |= WIPHY_FLAG_IBSS_RSN;
 
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 208b835..25ec150 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -3661,3 +3661,45 @@ void ieee80211_radar_detected(struct ieee80211_vif *vif,
 	cfg80211_radar_detected(sdata->dev, chan, gfp);
 }
 EXPORT_SYMBOL(ieee80211_radar_detected);
+
+void ieee80211_ap_ch_switch_work(struct work_struct *work)
+{
+	struct ieee80211_sub_if_data *sdata =
+		container_of(work, struct ieee80211_sub_if_data,
+			     u.ap.ap_ch_sw_work);
+	struct ieee80211_local *local = sdata->local;
+
+	/* update the device channel directly */
+	mutex_lock(&local->mtx);
+	if (local->ap_cs_channel) {
+		local->oper_channel =
+			local->hw.conf.channel = local->ap_cs_channel;
+		local->_oper_channel_type =  local->ap_cs_type;
+		local->ap_cs_channel = NULL;
+	}
+
+	ieee80211_wake_queues_by_reason(&local->hw,
+					IEEE80211_QUEUE_STOP_REASON_CH_SW);
+	mutex_unlock(&local->mtx);
+
+	cfg80211_ch_switch_notify(sdata->dev, local->oper_channel->center_freq,
+				  local->_oper_channel_type);
+}
+
+void ieee80211_ap_ch_switch_done(struct ieee80211_vif *vif,
+				 struct ieee80211_channel *new_channel,
+				 enum nl80211_channel_type type)
+{
+	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
+	struct ieee80211_local *local = sdata->local;
+
+	if (WARN_ON(sdata->vif.type != NL80211_IFTYPE_AP))
+		return;
+
+	if (WARN_ON(local->ap_cs_channel != new_channel))
+		return;
+
+	trace_api_ap_ch_switch_done(sdata, new_channel->center_freq);
+	schedule_work(&sdata->u.ap.ap_ch_sw_work);
+}
+EXPORT_SYMBOL(ieee80211_ap_ch_switch_done);
diff --git a/net/mac80211/trace.h b/net/mac80211/trace.h
index e6e066e..e12f243 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -1715,6 +1715,52 @@ TRACE_EVENT(drv_dfs_en_tx,
 	)
 );
 
+TRACE_EVENT(drv_ap_channel_switch,
+	TP_PROTO(struct ieee80211_local *local,
+		 struct ieee80211_ap_ch_switch *ap_ch_switch),
+
+	TP_ARGS(local, ap_ch_switch),
+
+	TP_STRUCT__entry(
+		LOCAL_ENTRY
+		__field(u16, freq)
+		__field(u8, count)
+	),
+
+	TP_fast_assign(
+		LOCAL_ASSIGN;
+		__entry->freq = ap_ch_switch->channel->center_freq;
+		__entry->count = ap_ch_switch->count;
+	),
+
+	TP_printk(
+		LOCAL_PR_FMT " switching to freq:%u count:%d",
+		LOCAL_PR_ARG, __entry->freq, __entry->count
+	)
+);
+
+TRACE_EVENT(api_ap_ch_switch_done,
+	TP_PROTO(struct ieee80211_sub_if_data *sdata,
+		 u16 center_freq),
+
+	TP_ARGS(sdata, center_freq),
+
+	TP_STRUCT__entry(
+		VIF_ENTRY
+		__field(u16, center_freq)
+	),
+
+	TP_fast_assign(
+		VIF_ASSIGN;
+		__entry->center_freq = center_freq;
+	),
+
+	TP_printk(
+		VIF_PR_FMT " switched to freq:%d",
+		VIF_PR_ARG, __entry->center_freq
+	)
+)
+
 #ifdef CONFIG_MAC80211_MESSAGE_TRACING
 #undef TRACE_SYSTEM
 #define TRACE_SYSTEM mac80211_msg
-- 
1.7.5.4

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Johannes Berg]

On Sun, 2012-10-14 at 16:48 +0200, Victor Goldenshtein wrote:
> Add ieee80211_ap_process_chanswitch(), to handle a channel switch
> request for AP.
> 
> Add ieee80211_ap_ch_switch_done() which updates oper_channel
> and notifies upper layers about channel switch complete event.

Given channel contexts, how does this patch need to change?

johannes

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Victor Goldenshtein]

On 16/10/2012 21:07, Johannes Berg wrote:
> On Sun, 2012-10-14 at 16:48 +0200, Victor Goldenshtein wrote:
>> Add ieee80211_ap_process_chanswitch(), to handle a channel switch
>> request for AP.
>>
>> Add ieee80211_ap_ch_switch_done() which updates oper_channel
>> and notifies upper layers about channel switch complete event.
>
> Given channel contexts, how does this patch need to change?
>

I guess most of the changes would be in the channel switch complete 
area, we will need to update the correct vif channel 
(ieee80211_assign_vif_chanctx) instead of the local->oper_channel.

-- 
Thanks,
Victor.

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Johannes Berg]

On Sun, 2012-10-21 at 18:40 +0200, Victor Goldenshtein wrote:
> On 16/10/2012 21:07, Johannes Berg wrote:
> > On Sun, 2012-10-14 at 16:48 +0200, Victor Goldenshtein wrote:
> >> Add ieee80211_ap_process_chanswitch(), to handle a channel switch
> >> request for AP.
> >>
> >> Add ieee80211_ap_ch_switch_done() which updates oper_channel
> >> and notifies upper layers about channel switch complete event.
> >
> > Given channel contexts, how does this patch need to change?
> >
> 
> I guess most of the changes would be in the channel switch complete 
> area, we will need to update the correct vif channel 
> (ieee80211_assign_vif_chanctx) instead of the local->oper_channel.

Yes, maybe, not sure.

Also wrt. channel contexts, how will CAC/radar check work? The AP isn't
bound to a channel until start_ap(), so somehow you'll have to bind it
(and keep it on that channel!) while the DFS check is running? And also
prohibit using multiple channels at this time...?

johannes

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Victor Goldenshtein]

On 22/10/2012 14:56, Johannes Berg wrote:
> On Sun, 2012-10-21 at 18:40 +0200, Victor Goldenshtein wrote:
>> On 16/10/2012 21:07, Johannes Berg wrote:
>>> On Sun, 2012-10-14 at 16:48 +0200, Victor Goldenshtein wrote:
>>>> Add ieee80211_ap_process_chanswitch(), to handle a channel switch
>>>> request for AP.
>>>>
>>>> Add ieee80211_ap_ch_switch_done() which updates oper_channel
>>>> and notifies upper layers about channel switch complete event.
>>>
>>> Given channel contexts, how does this patch need to change?
>>>
>>
>> I guess most of the changes would be in the channel switch complete
>> area, we will need to update the correct vif channel
>> (ieee80211_assign_vif_chanctx) instead of the local->oper_channel.
>
> Yes, maybe, not sure.
>
> Also wrt. channel contexts, how will CAC/radar check work? The AP isn't
> bound to a channel until start_ap(), so somehow you'll have to bind it

As I mentioned in my previous email we do it with __nl80211_set_channel().

> (and keep it on that channel!) while the DFS check is running? And also
> prohibit using multiple channels at this time...?

Why we need to prohibit multiple channels during CAC?
I don't see any problems to start CAC on different channels (once we"ll 
add this support), also lets not forget that CAC is relevant only for 
the AP role. Today for the MR single channel scenarios we have this 
check in the nl80211_start_radar_detection():

+» if·(chan->cac_started)
+» » return·-EBUSY;

which should block concurrent CAC executions on the same channel.

Anyway, the "DFS channel contexts" are not part of this patch series, 
and can be added later.

-- 
Thanks,
Victor.

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Johannes Berg]

On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:

> > Also wrt. channel contexts, how will CAC/radar check work? The AP isn't
> > bound to a channel until start_ap(), so somehow you'll have to bind it
> 
> As I mentioned in my previous email we do it with __nl80211_set_channel().

Yeah but that probably should change anyway, for channel contexts?

> > (and keep it on that channel!) while the DFS check is running? And also
> > prohibit using multiple channels at this time...?
> 
> Why we need to prohibit multiple channels during CAC?

Because you can't do CAC properly while you're channel-hopping?

> I don't see any problems to start CAC on different channels (once we"ll 
> add this support)

I do see issues with that, but let's not worry about it for now.

> also lets not forget that CAC is relevant only for 
> the AP role. Today for the MR single channel scenarios we have this 
> check in the nl80211_start_radar_detection():

MR?

> +» if·(chan->cac_started)
> +» » return·-EBUSY;
> 
> which should block concurrent CAC executions on the same channel.

Yes.

> Anyway, the "DFS channel contexts" are not part of this patch series, 
> and can be added later.

No ... channel contexts are in the kernel now, so you do have to think
about it now.

johannes

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Victor Goldenshtein]

On 05/11/2012 17:23, Johannes Berg wrote:
> On Thu, 2012-11-01 at 09:54 +0200, Victor Goldenshtein wrote:
>
>>> Also wrt. channel contexts, how will CAC/radar check work? The AP isn't
>>> bound to a channel until start_ap(), so somehow you'll have to bind it
>>
>> As I mentioned in my previous email we do it with __nl80211_set_channel().
>
> Yeah but that probably should change anyway, for channel contexts?
>
>>> (and keep it on that channel!) while the DFS check is running? And also
>>> prohibit using multiple channels at this time...?
>>
>> Why we need to prohibit multiple channels during CAC?
>
> Because you can't do CAC properly while you're channel-hopping?
>
>> I don't see any problems to start CAC on different channels (once we"ll
>> add this support)
>
> I do see issues with that, but let's not worry about it for now.
>
>> also lets not forget that CAC is relevant only for
>> the AP role. Today for the MR single channel scenarios we have this
>> check in the nl80211_start_radar_detection():
>
> MR?
>
>> +» if·(chan->cac_started)
>> +» » return·-EBUSY;
>>
>> which should block concurrent CAC executions on the same channel.
>
> Yes.
>
>> Anyway, the "DFS channel contexts" are not part of this patch series,
>> and can be added later.
>
> No ... channel contexts are in the kernel now, so you do have to think
> about it now.
>

Well thinking is one thing implementing is another ;)

This whole DFS implementation initially intended for a single channel 
mac. I don't might to deal with the channel context stuff but not sure 
how much available time I"ll have for it, so it might take awhile.
I know there are people waiting for this, so I'm thinking would you 
consider to go first with this single channel DFS support and later to 
extend it to work with channel context stuff ?

-- 
Thanks,
Victor.

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Johannes Berg]

On Tue, 2012-11-13 at 17:04 +0200, Victor Goldenshtein wrote:

> > No ... channel contexts are in the kernel now, so you do have to think
> > about it now.
> 
> Well thinking is one thing implementing is another ;)

Well ... :)

> This whole DFS implementation initially intended for a single channel 
> mac. I don't might to deal with the channel context stuff but not sure 
> how much available time I"ll have for it, so it might take awhile.
> I know there are people waiting for this, so I'm thinking would you 
> consider to go first with this single channel DFS support and later to 
> extend it to work with channel context stuff ?

Fair enough, but like I've been telling you, the current code doesn't
even match the current APIs.

Initially, I thought that for radar detection, you need to reserve the
channel context (in mac80211), make sure it's the only channel context
and prohibit other channel contexts from being added, until radar
detection is done.

However, then I realised that that still doesn't work -- once initial
radar detection is done, it needs to continue while the AP is active. If
the channel context was going to be relinquished, or even just the
channel changed for a few seconds, it would be unsafe. So as a result,
the radar detect operation has to somehow be coupled to the start AP
operation and prohibit channel changes and additional channel contexts
during the entire operation time.

I definitely think this needs some work, because the APIs and cfg80211
code have to be correct for multi-channel operation even if you don't
support it.

johannes

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Michal Kazior]

On 14/11/12 12:23, Johannes Berg wrote:
> On Tue, 2012-11-13 at 17:04 +0200, Victor Goldenshtein wrote:
>
>>> No ... channel contexts are in the kernel now, so you do have to think
>>> about it now.
>>
>> Well thinking is one thing implementing is another ;)
>
> Well ... :)
>
>> This whole DFS implementation initially intended for a single channel
>> mac. I don't might to deal with the channel context stuff but not sure
>> how much available time I"ll have for it, so it might take awhile.
>> I know there are people waiting for this, so I'm thinking would you
>> consider to go first with this single channel DFS support and later to
>> extend it to work with channel context stuff ?
>
> Fair enough, but like I've been telling you, the current code doesn't
> even match the current APIs.
>
> Initially, I thought that for radar detection, you need to reserve the
> channel context (in mac80211), make sure it's the only channel context
> and prohibit other channel contexts from being added, until radar
> detection is done.
>
> However, then I realised that that still doesn't work -- once initial
> radar detection is done, it needs to continue while the AP is active. If
> the channel context was going to be relinquished, or even just the
> channel changed for a few seconds, it would be unsafe. So as a result,
> the radar detect operation has to somehow be coupled to the start AP
> operation and prohibit channel changes and additional channel contexts
> during the entire operation time.

Hmm.. cfg80211 doesn't really know about channel contexts.

The problem I see is that cfg80211 may be in a combination with 
`num_different_channels = 1` and mac80211 can have 2 channel contexts 
due to channel type incompatibilities.

We'd need to tell cfg80211 that multi-interface is not possible when DFS 
is active if we want to at least consider single-channel only DFS.


-- Pozdrawiam / Best regards, Michal Kazior.

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Johannes Berg]

On Wed, 2012-11-14 at 13:19 +0100, Michal Kazior wrote:

> > However, then I realised that that still doesn't work -- once initial
> > radar detection is done, it needs to continue while the AP is active. If
> > the channel context was going to be relinquished, or even just the
> > channel changed for a few seconds, it would be unsafe. So as a result,
> > the radar detect operation has to somehow be coupled to the start AP
> > operation and prohibit channel changes and additional channel contexts
> > during the entire operation time.
> 
> Hmm.. cfg80211 doesn't really know about channel contexts.
>
> The problem I see is that cfg80211 may be in a combination with 
> `num_different_channels = 1` and mac80211 can have 2 channel contexts 
> due to channel type incompatibilities.

Yes, this is a concern, we need to fix that anyway though, otherwise it
may assume that some operations are possible when they really aren't
(e.g. starting an HT40+ and HT40- AP on the same channel, which
shouldn't be done anyway, but still)

> We'd need to tell cfg80211 that multi-interface is not possible when DFS 
> is active if we want to at least consider single-channel only DFS.

Not sure about multi-interface, but multi-channel at least.

johannes

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Victor Goldenshtein]

On 14/11/2012 14:38, Johannes Berg wrote:
> On Wed, 2012-11-14 at 13:19 +0100, Michal Kazior wrote:
>
>>> However, then I realised that that still doesn't work -- once initial
>>> radar detection is done, it needs to continue while the AP is active. If
>>> the channel context was going to be relinquished, or even just the
>>> channel changed for a few seconds, it would be unsafe. So as a result,
>>> the radar detect operation has to somehow be coupled to the start AP
>>> operation and prohibit channel changes and additional channel contexts
>>> during the entire operation time.
>>
>> Hmm.. cfg80211 doesn't really know about channel contexts.
>>
>> The problem I see is that cfg80211 may be in a combination with
>> `num_different_channels = 1` and mac80211 can have 2 channel contexts
>> due to channel type incompatibilities.
>
> Yes, this is a concern, we need to fix that anyway though, otherwise it
> may assume that some operations are possible when they really aren't
> (e.g. starting an HT40+ and HT40- AP on the same channel, which
> shouldn't be done anyway, but still)
>

not sure how it can happen if have this check:

if ((cac_type != NL80211_CHAN_HT20) && (cac_type != NL80211_CHAN_NO_HT))
		return -EOPNOTSUPP;

in both start_radar_detection() & enable_tx() ?

>> We'd need to tell cfg80211 that multi-interface is not possible when DFS
>> is active if we want to at least consider single-channel only DFS.
>
> Not sure about multi-interface, but multi-channel at least.
>

Agree, maybe you have an idea how we should do this ?


-- 
Thanks,
Victor.

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Johannes Berg]

On Tue, 2012-11-20 at 17:15 +0200, Victor Goldenshtein wrote:
> On 14/11/2012 14:38, Johannes Berg wrote:
> > On Wed, 2012-11-14 at 13:19 +0100, Michal Kazior wrote:
> >
> >>> However, then I realised that that still doesn't work -- once initial
> >>> radar detection is done, it needs to continue while the AP is active. If
> >>> the channel context was going to be relinquished, or even just the
> >>> channel changed for a few seconds, it would be unsafe. So as a result,
> >>> the radar detect operation has to somehow be coupled to the start AP
> >>> operation and prohibit channel changes and additional channel contexts
> >>> during the entire operation time.
> >>
> >> Hmm.. cfg80211 doesn't really know about channel contexts.
> >>
> >> The problem I see is that cfg80211 may be in a combination with
> >> `num_different_channels = 1` and mac80211 can have 2 channel contexts
> >> due to channel type incompatibilities.
> >
> > Yes, this is a concern, we need to fix that anyway though, otherwise it
> > may assume that some operations are possible when they really aren't
> > (e.g. starting an HT40+ and HT40- AP on the same channel, which
> > shouldn't be done anyway, but still)
> >
> 
> not sure how it can happen if have this check:
> 
> if ((cac_type != NL80211_CHAN_HT20) && (cac_type != NL80211_CHAN_NO_HT))
> 		return -EOPNOTSUPP;
> 
> in both start_radar_detection() & enable_tx() ?

The discussion between Michal and me above was unrelated to DFS :-)

johannes

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Victor Goldenshtein]

On 14/11/2012 13:23, Johannes Berg wrote:
> On Tue, 2012-11-13 at 17:04 +0200, Victor Goldenshtein wrote:
>> This whole DFS implementation initially intended for a single channel
>> mac. I don't might to deal with the channel context stuff but not sure
>> how much available time I"ll have for it, so it might take awhile.
>> I know there are people waiting for this, so I'm thinking would you
>> consider to go first with this single channel DFS support and later to
>> extend it to work with channel context stuff ?
>
> Fair enough, but like I've been telling you, the current code doesn't
> even match the current APIs.
>
> Initially, I thought that for radar detection, you need to reserve the
> channel context (in mac80211), make sure it's the only channel context
> and prohibit other channel contexts from being added, until radar
> detection is done.
>

I thought the same, but to prohibit other channel contexts from being 
added as long as we on DFS channel (not just until the end of the CAC).

> However, then I realised that that still doesn't work -- once initial
> radar detection is done, it needs to continue while the AP is active. If
> the channel context was going to be relinquished, or even just the
> channel changed for a few seconds, it would be unsafe. So as a result,
> the radar detect operation has to somehow be coupled to the start AP
> operation and prohibit channel changes and additional channel contexts
> during the entire operation time.
>

What would be "unsafe" here if from the beginning we will allow only one 
channel context ?

> I definitely think this needs some work, because the APIs and cfg80211
> code have to be correct for multi-channel operation even if you don't
> support it.
>

Agree.

-- 
Thanks,
Victor.

Re: [PATCH v4 6/6] mac80211: add ap channel switch command/event

[Johannes Berg]

On Tue, 2012-11-20 at 17:15 +0200, Victor Goldenshtein wrote:

> > Initially, I thought that for radar detection, you need to reserve the
> > channel context (in mac80211), make sure it's the only channel context
> > and prohibit other channel contexts from being added, until radar
> > detection is done.
> >
> 
> I thought the same, but to prohibit other channel contexts from being 
> added as long as we on DFS channel (not just until the end of the CAC).

Yes, that's a good plan, however, you're missing the fact that you need
to define "as long as we['re] on [the] DFS channel". You forgot that
this isn't a well-defined sentence with channel contexts. As soon as the
CAC operation stops, there's no way to know that we're still on the same
channel, nor do we know if we event *want to*.

So it seems you need different operations:

 1) start CAC (as today)
 2) event - CAC complete - but don't relinquish channel lock here!
 3a) abort CAC - relinquish channel lock, in case userspace no longer
     cares
 3b) start AP - which moves the channel from being CAC owned to AP owned

Note that you'd also implicitly have to call the CAC abort case (3a) in
case userspace crashes inbetween, so the netlink socket has to own this
similar to the management frame RX handling etc.

> > However, then I realised that that still doesn't work -- once initial
> > radar detection is done, it needs to continue while the AP is active. If
> > the channel context was going to be relinquished, or even just the
> > channel changed for a few seconds, it would be unsafe. So as a result,
> > the radar detect operation has to somehow be coupled to the start AP
> > operation and prohibit channel changes and additional channel contexts
> > during the entire operation time.
> >
> 
> What would be "unsafe" here if from the beginning we will allow only one 
> channel context ?

Well it seems I was a step ahead of you, I was thinking of the gap
between "CAC ended" and "start AP", see above.

johannes