diff mbox

[v2] cfg80211/nl80211: add DFS offload flag

Message ID 20180222091513.25456-1-sergey.matyukevich.os@quantenna.com (mailing list archive)
State Superseded
Delegated to: Johannes Berg
Headers show

Commit Message

Sergey Matyukevich Feb. 22, 2018, 9:15 a.m. UTC
From: Dmitry Lebed <dlebed@quantenna.com>

Add wiphy EXT_FEATURE flag to indicate that HW or driver does
all DFS actions by itself.
User-space functionality already implemented in hostapd using
vendor-specific (QCA) OUI to advertise DFS offload support.
Need to introduce generic flag to inform about DFS offload support.
For deivces with DFS_OFFLOAD flag set user-space will no longer
need to issue CAC or do any actions in response to
"radar detected" events. HW will do everything by itself and send
events to user-space to indicate that CAC was started/finished, etc.

Signed-off-by: Dmitrii Lebed <dlebed@quantenna.com>
---
V1->V2 changes:
- moved from wiphy_flags to ext_features flags usage

include/uapi/linux/nl80211.h |  7 +++++++
 net/wireless/nl80211.c       | 12 ++++++++----
 2 files changed, 15 insertions(+), 4 deletions(-)

Comments

Johannes Berg Feb. 23, 2018, 11:32 a.m. UTC | #1
On Thu, 2018-02-22 at 12:15 +0300, Sergey Matyukevich wrote:
> From: Dmitry Lebed <dlebed@quantenna.com>
> 
> Add wiphy EXT_FEATURE flag to indicate that HW or driver does
> all DFS actions by itself.
> User-space functionality already implemented in hostapd using
> vendor-specific (QCA) OUI to advertise DFS offload support.
> Need to introduce generic flag to inform about DFS offload support.
> For deivces with DFS_OFFLOAD flag set user-space will no longer
> need to issue CAC or do any actions in response to
> "radar detected" events. HW will do everything by itself and send
> events to user-space to indicate that CAC was started/finished, etc.
> 
> + * @NL80211_EXT_FEATURE_DFS_OFFLOAD: HW/driver will offload DFS actions.
> + *	Device or driver will do all DFS-related actions by itself,
> + *	informing user-space about CAC progress, radar detection event,
> + *	channel change triggered by radar detection event.
> + *	No need to start CAC from user-space, no need to react to
> + *	"radar detected" event.

The "channel change" part bothers me a bit - isn't normally hostapd
very much in control of the channel? How does it even get notified of
the new channel? How can you configure the parameters like how many
beacons to include the (E)CSA IE for, and whether to add ECSA or not
etc?

johannes
Dmitry Lebed Feb. 23, 2018, 4:23 p.m. UTC | #2
23.02.2018 14:32, Johannes Berg wrote:
> On Thu, 2018-02-22 at 12:15 +0300, Sergey Matyukevich wrote:
>> + * @NL80211_EXT_FEATURE_DFS_OFFLOAD: HW/driver will offload DFS actions.
>> + *	Device or driver will do all DFS-related actions by itself,
>> + *	informing user-space about CAC progress, radar detection event,
>> + *	channel change triggered by radar detection event.
>> + *	No need to start CAC from user-space, no need to react to
>> + *	"radar detected" event.
> The "channel change" part bothers me a bit - isn't normally hostapd
> very much in control of the channel? How does it even get notified of
> the new channel? How can you configure the parameters like how many
> beacons to include the (E)CSA IE for, and whether to add ECSA or not
> etc?
>
> johannes

hostapd will be notified with NL80211_CMD_CH_SWITCH_NOTIFY event
and it already has a handler for this event and looks like it works as 
expected.
In case of radar detection, userspace (hostapd) should receive two 
events (with DFS offload enabled):
1. RADAR_DETECT - and in case of DFS_OFFLOAD flag turned on don't do any CSA
2. CH_SWITCH_NOTIFY - at the moment when HW will switch channel as a 
reaction
     to radar event

Currently, all CSA will be sent with default parameters. If it's crucial 
to tune e.g. TBTT
count, then let's add it with a separate commit later, when DFS offload 
will be
implemented in the whole SW stack (driver/kernel/userspace).

DFS offload should be quite useful and should fix some issues in a 
repeater configuration.
There are a number of issues which are hard to fix without DFS offload 
feature:
e.g. if repeater has detected a radar and sent radar report frame to 
master AP,
master AP will reply with CSA, but hostapd will try to change channel to 
different channel.

Regards,
Dmitry
Sergey Matyukevich March 1, 2018, 8:46 a.m. UTC | #3
Hello Johannes,

> > Add wiphy EXT_FEATURE flag to indicate that HW or driver does
> > all DFS actions by itself.
> > User-space functionality already implemented in hostapd using
> > vendor-specific (QCA) OUI to advertise DFS offload support.
> > Need to introduce generic flag to inform about DFS offload support.
> > For deivces with DFS_OFFLOAD flag set user-space will no longer
> > need to issue CAC or do any actions in response to
> > "radar detected" events. HW will do everything by itself and send
> > events to user-space to indicate that CAC was started/finished, etc.
> >
> > + * @NL80211_EXT_FEATURE_DFS_OFFLOAD: HW/driver will offload DFS actions.
> > + *   Device or driver will do all DFS-related actions by itself,
> > + *   informing user-space about CAC progress, radar detection event,
> > + *   channel change triggered by radar detection event.
> > + *   No need to start CAC from user-space, no need to react to
> > + *   "radar detected" event.
> 
> The "channel change" part bothers me a bit - isn't normally hostapd
> very much in control of the channel? How does it even get notified of
> the new channel? How can you configure the parameters like how many
> beacons to include the (E)CSA IE for, and whether to add ECSA or not
> etc?

We are about to send v3 for this feature. Could you please clarify
your preferences in terms of patchwork procedure ? Should we mark
previous patch versions as superseded for nl80211/cfg80211 patches ?

Regards,
Sergey
Johannes Berg March 1, 2018, 8:52 a.m. UTC | #4
Hi,

Sorry I haven't answered/applied this. I will eventually, you've
convinced me with the answer.

> We are about to send v3 for this feature. Could you please clarify
> your preferences in terms of patchwork procedure ? Should we mark
> previous patch versions as superseded for nl80211/cfg80211 patches ?

If you do that it saves me the "work", but it doesn't bother me to do
it myself.

johannes
diff mbox

Patch

diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 22cca373161d..811928ce4e1c 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -4948,6 +4948,12 @@  enum nl80211_feature_flags {
  *	probe request tx deferral and suppression
  * @NL80211_EXT_FEATURE_MFP_OPTIONAL: Driver supports the %NL80211_MFP_OPTIONAL
  *	value in %NL80211_ATTR_USE_MFP.
+ * @NL80211_EXT_FEATURE_DFS_OFFLOAD: HW/driver will offload DFS actions.
+ *	Device or driver will do all DFS-related actions by itself,
+ *	informing user-space about CAC progress, radar detection event,
+ *	channel change triggered by radar detection event.
+ *	No need to start CAC from user-space, no need to react to
+ *	"radar detected" event.
  *
  * @NUM_NL80211_EXT_FEATURES: number of extended features.
  * @MAX_NL80211_EXT_FEATURES: highest extended feature index.
@@ -4975,6 +4981,7 @@  enum nl80211_ext_feature_index {
 	NL80211_EXT_FEATURE_OCE_PROBE_REQ_HIGH_TX_RATE,
 	NL80211_EXT_FEATURE_OCE_PROBE_REQ_DEFERRAL_SUPPRESSION,
 	NL80211_EXT_FEATURE_MFP_OPTIONAL,
+	NL80211_EXT_FEATURE_DFS_OFFLOAD,
 
 	/* add new features before the definition below */
 	NUM_NL80211_EXT_FEATURES,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b5e613d14d6a..d57aba58a418 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7786,12 +7786,13 @@  static int nl80211_start_radar_detection(struct sk_buff *skb,
 	struct cfg80211_registered_device *rdev = info->user_ptr[0];
 	struct net_device *dev = info->user_ptr[1];
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
+	struct wiphy *wiphy = wdev->wiphy;
 	struct cfg80211_chan_def chandef;
 	enum nl80211_dfs_regions dfs_region;
 	unsigned int cac_time_ms;
 	int err;
 
-	dfs_region = reg_get_dfs_region(wdev->wiphy);
+	dfs_region = reg_get_dfs_region(wiphy);
 	if (dfs_region == NL80211_DFS_UNSET)
 		return -EINVAL;
 
@@ -7805,17 +7806,20 @@  static int nl80211_start_radar_detection(struct sk_buff *skb,
 	if (wdev->cac_started)
 		return -EBUSY;
 
-	err = cfg80211_chandef_dfs_required(wdev->wiphy, &chandef,
-					    wdev->iftype);
+	err = cfg80211_chandef_dfs_required(wiphy, &chandef, wdev->iftype);
 	if (err < 0)
 		return err;
 
 	if (err == 0)
 		return -EINVAL;
 
-	if (!cfg80211_chandef_dfs_usable(wdev->wiphy, &chandef))
+	if (!cfg80211_chandef_dfs_usable(wiphy, &chandef))
 		return -EINVAL;
 
+	/* CAC start is offloaded to HW and can't be started manually */
+	if (wiphy_ext_feature_isset(wiphy, NL80211_EXT_FEATURE_DFS_OFFLOAD))
+		return -EOPNOTSUPP;
+
 	if (!rdev->ops->start_radar_detection)
 		return -EOPNOTSUPP;