This is patchs series no 6 of the DFS master ability, based on Victor
Goldenshteins original work. See changelog in the various patches for
details. The main changes to v5 are:
* channel switch announcement patch has been dropped from this patchset.
Although CSA is required for 802.11h, DFS can also be implemented
by just disassociating all stations and open up AP on another channel
later when a radar is detected (to meet ETSI/FCC requirements).
I've choosen this approach to reduce the patchset size, CSA can (and should)
still be added later.
* SINGLE_ONLY patch from last time was dropped as well
* interface combination are now used to define and check DFS capabilities:
* one AP only/single channel is supported for now, more can be added later
* channel bandwidths are checked within a bitfield now
* Johannes Berg helped me a lot here, thanks! :)
* some more obsolete things have been removed (dfs_nlportid, cac_chantype, etc)
* rebased on latest master, of course
Again, the implementation works for me[tm] with a dummy dfs-implementation
in ath9k and a patched hostapd - I'll provide the hostapd patchset later
after cleaning it up.
Any comment/review/etc are very much appreciated!
Thanks,
Simon
Simon Wunderlich (4):
nl80211: check if channel can be used in join_ibss
cfg80211: check radar interface combinations
mac80211: check radar interaction with scan and roc
nl80211: allow DFS in start_ap
Victor Goldenshtein (2):
nl80211/cfg80211: add radar detection command/event
mac80211: add radar detection command/event
include/net/cfg80211.h | 26 +++++++++
include/net/mac80211.h | 19 +++++++
include/uapi/linux/nl80211.h | 12 ++++
net/mac80211/cfg.c | 37 ++++++++++++-
net/mac80211/driver-ops.h | 15 +++++
net/mac80211/ieee80211_i.h | 3 +
net/mac80211/iface.c | 4 ++
net/mac80211/mlme.c | 11 ++++
net/mac80211/scan.c | 3 +
net/mac80211/trace.h | 76 +++++++++++++++++++++++++
net/wireless/chan.c | 14 +++--
net/wireless/core.c | 11 +++-
net/wireless/core.h | 7 ++-
net/wireless/mlme.c | 14 +++++
net/wireless/nl80211.c | 126 +++++++++++++++++++++++++++++++++++++++++-
net/wireless/nl80211.h | 6 ++
net/wireless/util.c | 32 ++++++++++-
17 files changed, 400 insertions(+), 16 deletions(-)
--
1.7.10.4
On 01/18/2013 11:08 PM, Johannes Berg wrote:
> Hi,
>
>> As stated before: "available until ..." time is not neccessary as far
>> as I know. Also the channel must stay available when AP stops operation,
>> I'll change that.
>
> Yeah, I checked the documents myself now ... strange, but hey. Radars
> are fixed installations and mostly active continually, so I guess they
> figured it was no big deal.
>
We were also unsure if this makes sense and contacted regulatory for
clarification. Indeed, with the current document a scenario where "I switched to a
DFS channel and skipped CAC because I passed one half a year ago" is fully
compliant. Practically, when you set up a new AP and do once a CAC for each DFS
channel during installation time, you don't need to do a CAC ever again (as long
as you do not see radars).
Why then bother with CAC at all, one might think. In fact, exploiting given
requirements to such extent is mandatory to make operation on DFS channels usable
(which btw. is not the scope of the patches discussed here).
> [...]
>
> johannes
>
On 01/23/2013 01:52 PM, Simon Wunderlich wrote:
> On Mon, Jan 21, 2013 at 11:46:20AM +0100, Zefir Kurtisi wrote:
> [...]
>
>> Why then bother with CAC at all, one might think. In fact, exploiting given
>> requirements to such extent is mandatory to make operation on DFS channels usable
>> (which btw. is not the scope of the patches discussed here).
>>
>
> I would actually like to make operation on DFS channels usable by using the implementation
> from these patches. :) Do you imply that we couldn't practically use it? Any suggestions
> what can be improved?
>
> Cheers,
> Simon
>
Hi Simon,
I realize that my last statement is misleading, given that there are different
levels for being 'usable'. The current DFS master support will allow to certify,
but how usable will those devices will operate? I'll try to explain my concerns
based on what is needed for the products I am currently working on. It got
longish, so here is the short version.
tl;dr: we can't integrate DFS support for managed master devices in Linux
wireless, since that requires breaking the existing regulatory statement.
The long version bases on the fact that in any setup we will face false radar
events that are degrading usability. The inevitability of false detections is
given by the existence of interferences not distinguishable from radar pulses and
the regulatory requirement to detect incomplete patterns with a given probability.
Set up as radar monitor, we measured false detections between once a day and
several per hour. This is the best case, since in master mode the numbers
significantly increase (traffic type dependent). These observations were done with
Atheros hardware, but given their general cause, lets deal with regular false
events we need to handle.
So, who is going to use DFS channels and how? It is maybe not the home user, who
will disable them as soon as he looses connectivity for minutes again - he can do
just fine with non-DFS channels (that's maybe why there are so few consumer
products with working DFS support out there).
Industrial applications however depend on using them for their higher tx power
allowed and to escape the crowded non-DFS channels. At the same time, they do not
tolerate service interruption: our devices are controlling trains over WiFi - and
you do not want them to stop on every CAC ;) Obviously, the only way to achieve
continuous connectivity on DFS channels is to go the managed approach combining
master and sensor nodes. There, on top of what you and Johannes already discussed
(re-use wiphy0's CAC results for wiphy1), you need to be able to re-use the
channel information of nearby nodes.
And that's finally my point: to be able to skip CAC for a DFS channel based on
external information, you need to provide means to override your own channel
states - which in turn allows to bypass regulatory requirements and therefore
breaks the Linux wireless regulatory statement.
I don't see an easy way to expose those mechanisms to manufacturers but prevent
mis-use by the 'common user'. Are there?
Cheers,
Zefir
Hi,
On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
> To ease further DFS development regarding interface combinations, use
> the interface combinations structure to test for radar capabilities.
> Drivers can specify which channel widths they support, and in which
> modes. Drivers should first allow AP mode only, but can later allow
> MultiSSID APs, AP+Ad-Hoc, etc.
>
> Signed-off-by: Simon Wunderlich <[email protected]>
> ---
[...]
> diff --git a/net/wireless/util.c b/net/wireless/util.c
> index 16d76a8..72476e8 100644
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
[...]
> @@ -1195,14 +1196,36 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
> enum cfg80211_chan_mode chmode;
> int num_different_channels = 0;
> int total = 1;
> + bool radar_required;
> int i, j;
>
> ASSERT_RTNL();
> lockdep_assert_held(&rdev->devlist_mtx);
>
> + if (WARN_ON(hweight32(radar_detect) > 1))
> + return -EINVAL;
> +
> + switch (iftype) {
> + case NL80211_IFTYPE_ADHOC:
> + case NL80211_IFTYPE_AP:
> + case NL80211_IFTYPE_AP_VLAN:
> + case NL80211_IFTYPE_MESH_POINT:
> + case NL80211_IFTYPE_P2P_GO:
> + radar_required = !!(chan->flags & IEEE80211_CHAN_RADAR);
> + break;
This code is causing an oops with the wl18xx driver in AP mode. The
problem is that cfg80211_can_change_interface() calls
cfg80211_can_use_iftype_chan() with chan == NULL. This code doesn't
check if chan is NULL, so this dereference causes the oops.
I don't have the time right now to fix this, but I'll look into it
tomorrow (unless someone comes with a fix before that :P).
This code is currently in wireless-next as commit
11c4a075db2f8774d37544342c8cb9752b4db9e1.
Here's the full oops report:
[ 1869.594970] Unable to handle kernel NULL pointer dereference at virtual address 00000008
[ 1869.604675] pgd = ebc0c000
[ 1869.608886] [00000008] *pgd=abd73831, *pte=00000000, *ppte=00000000
[ 1869.621276] Internal error: Oops: 17 [#1] SMP ARM
[ 1869.627532] Modules linked in: wl18xx wlcore mac80211 cfg80211 rfkill wlcore_sdio
[ 1869.635467] CPU: 0 Not tainted (3.8.0-rc4-wl+ #990)
[ 1869.641387] PC is at cfg80211_can_use_iftype_chan+0xb0/0x598 [cfg80211]
[ 1869.648468] LR is at cfg80211_can_use_iftype_chan+0x58/0x598 [cfg80211]
[ 1869.655426] pc : [<bf01ac10>] lr : [<bf01abb8>] psr: 80000113
[ 1869.655426] sp : ebe09ca8 ip : 00000000 fp : ebe09d4c
[ 1869.667480] r10: 0000000d r9 : c0c6ba4a r8 : 0000000c
[ 1869.672973] r7 : 00000000 r6 : 00000000 r5 : 00000003 r4 : 00000000
[ 1869.679840] r3 : ea000000 r2 : 5d400000 r1 : 00000000 r0 : 00000000
[ 1869.686706] Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 1869.694213] Control: 10c53c7d Table: abc0c04a DAC: 00000015
[ 1869.700256] Process hostapd (pid: 4627, stack limit = 0xebe08240)
[ 1869.706665] Stack: (0xebe09ca8 to 0xebe0a000)
[ 1869.711273] 9ca0: 00000000 ebd5db80 ebe09cdc 00000000 c00b0870 eb944000
[ 1869.719879] 9cc0: eb9440a8 c0c7f8d4 ebe08000 eb9440ac ebe09cec c00ac864 c0bc6de8 eb9440a8
[ 1869.728485] 9ce0: ebe09d4c ebe09cf0 c071fe04 c00ac834 00000002 00000000 bf017628 edf60000
[ 1869.737091] 9d00: ebe09d1c eb9440d4 bf017628 eb9440e8 22222222 22222222 22222222 22222222
[ 1869.745697] 9d20: 00000000 eb9440a8 eb944000 edf60648 00000000 0000000c c0c6ba4a 0000000d
[ 1869.754302] 9d40: ebe09d8c ebe09d50 bf017644 bf01ab6c 00000000 00000000 ebe09d7c ebe09d68
[ 1869.762908] 9d60: c00c3f8c edf60000 fffffff2 00000000 bf01722c bf135ed4 c0c6ba4a 0000000d
[ 1869.771484] 9d80: ebe09dc4 ebe09d90 c0725514 bf017238 c00b0870 bf06b398 ebe08000 edf60000
[ 1869.780090] 9da0: 0000000d 00001002 00000001 ebe08000 00000000 ecca400c ebe09ddc ebe09dc8
[ 1869.788696] 9dc0: c00823c0 c0725470 00000000 c05411c8 ebe09df4 ebe09de0 c05411e0 c00823a4
[ 1869.797302] 9de0: edf60000 bf1254c8 ebe09e14 ebe09df8 c05451e8 c05411b4 edf60000 00001003
[ 1869.805908] 9e00: 00001002 00000001 ebe09e34 ebe09e18 c05454c4 c05451b0 edf60000 00001002
[ 1869.814514] 9e20: 00000001 00008914 ebe09e54 ebe09e38 c0545614 c0545448 ebd5db80 00000000
[ 1869.823120] 9e40: 00000000 00000001 ebe09ebc ebe09e58 c06483b0 c0545600 c0526d24 c01319a8
[ 1869.831726] 9e60: edf60000 00000014 ecca4000 bea7fa90 6e616c77 00000030 00000000 00000000
[ 1869.840332] 9e80: 00001003 00000000 00000000 00000000 00000000 00008914 bea7fa90 c0649e88
[ 1869.848937] 9ea0: bea7fa90 0000c000 ebe08000 00000000 ebe09ecc ebe09ec0 c064a044 c0647d30
[ 1869.857543] 9ec0: ebe09eec ebe09ed0 c0527ddc c0649e94 c0527d70 ee3fcd20 ebc76000 00000006
[ 1869.866149] 9ee0: ebe09f74 ebe09ef0 c0169954 c0527d7c c0c6bc60 ec5a8380 ebe09f3c ebe09f08
[ 1869.874755] 9f00: ebd5db80 ebe08000 00000000 60000113 ebe09f3c ebe09f20 c00b1770 c00b16bc
[ 1869.883361] 9f20: c0c6bc60 ec0b5340 c0c6bc60 ebc76000 ebe09f74 ebe09f40 c0175bb8 c00b172c
[ 1869.891967] 9f40: c00155a4 ebd5db80 00000001 60000110 00000000 ebc76000 bea7fa90 00008914
[ 1869.900573] 9f60: 00000006 ebe08000 ebe09fa4 ebe09f78 c0169f60 c01698d0 ebe09f94 00000000
[ 1869.909179] 9f80: bea7fb38 bea7fa90 00000006 00000036 c0015648 00000000 00000000 ebe09fa8
[ 1869.917785] 9fa0: c00153e0 c0169eec bea7fb38 bea7fa90 00000006 00008914 bea7fa90 00001003
[ 1869.926391] 9fc0: bea7fb38 bea7fa90 00000006 00000036 00000001 00000001 00000000 00000000
[ 1869.934997] 9fe0: 0008e4d8 bea7fa88 000454fc b6ca6bcc 60000110 00000006 726f7720 72702064
[ 1869.943847] [<bf01ac10>] (cfg80211_can_use_iftype_chan+0xb0/0x598 [cfg80211]) from [<bf017644>] (cfg80211_netdev_notifier_call+0x418/0x84c [cfg80211])
[ 1869.958160] [<bf017644>] (cfg80211_netdev_notifier_call+0x418/0x84c [cfg80211]) from [<c0725514>] (notifier_call_chain+0xb0/0x184)
[ 1869.970520] [<c0725514>] (notifier_call_chain+0xb0/0x184) from [<c00823c0>] (raw_notifier_call_chain+0x28/0x30)
[ 1869.981170] [<c00823c0>] (raw_notifier_call_chain+0x28/0x30) from [<c05411e0>] (call_netdevice_notifiers+0x38/0x64)
[ 1869.992156] [<c05411e0>] (call_netdevice_notifiers+0x38/0x64) from [<c05451e8>] (__dev_open+0x44/0x110)
[ 1870.002044] [<c05451e8>] (__dev_open+0x44/0x110) from [<c05454c4>] (__dev_change_flags+0x88/0x14c)
[ 1870.011474] [<c05454c4>] (__dev_change_flags+0x88/0x14c) from [<c0545614>] (dev_change_flags+0x20/0x58)
[ 1870.021362] [<c0545614>] (dev_change_flags+0x20/0x58) from [<c06483b0>] (devinet_ioctl+0x68c/0x79c)
[ 1870.030883] [<c06483b0>] (devinet_ioctl+0x68c/0x79c) from [<c064a044>] (inet_ioctl+0x1bc/0x1d0)
[ 1870.040069] [<c064a044>] (inet_ioctl+0x1bc/0x1d0) from [<c0527ddc>] (sock_ioctl+0x6c/0x2bc)
[ 1870.048858] [<c0527ddc>] (sock_ioctl+0x6c/0x2bc) from [<c0169954>] (do_vfs_ioctl+0x90/0x61c)
[ 1870.057739] [<c0169954>] (do_vfs_ioctl+0x90/0x61c) from [<c0169f60>] (sys_ioctl+0x80/0x88)
[ 1870.066436] [<c0169f60>] (sys_ioctl+0x80/0x88) from [<c00153e0>] (ret_fast_syscall+0x0/0x3c)
[ 1870.075317] Code: e1a00003 13e00015 e24bd028 e89daff0 (e5963008)
[ 1870.081909] ---[ end trace 10620d4073c27977 ]---
--
Cheers,
Luca.
On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
> To ease further DFS development regarding interface combinations, use
> the interface combinations structure to test for radar capabilities.
> Drivers can specify which channel widths they support, and in which
> modes. Drivers should first allow AP mode only, but can later allow
> MultiSSID APs, AP+Ad-Hoc, etc.
Applied.
johannes
This patch enables DFS within nl80211/cfg80211. start_ap now checks if
DFS is allowed on the channel with the selected channel width.
Signed-off-by: Simon Wunderlich <[email protected]>
---
Changes to PATCHv5:
* check radar detection width
---
net/wireless/mlme.c | 1 +
net/wireless/nl80211.c | 23 +++++++++++++++++++++--
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 6b63ffb..153c4ed 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -709,6 +709,7 @@ int cfg80211_mlme_register_mgmt(struct wireless_dev *wdev, u32 snd_portid,
void cfg80211_mlme_unregister_socket(struct wireless_dev *wdev, u32 nlportid)
{
struct wiphy *wiphy = wdev->wiphy;
+ struct net_device *dev = wdev->netdev;
struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
struct cfg80211_mgmt_registration *reg, *tmp;
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d98c1d3..2bb9889 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2600,7 +2600,9 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
struct net_device *dev = info->user_ptr[1];
struct wireless_dev *wdev = dev->ieee80211_ptr;
struct cfg80211_ap_settings params;
+ struct ieee80211_channel *chan;
int err;
+ u8 radar_detect_width = 0;
if (dev->ieee80211_ptr->iftype != NL80211_IFTYPE_AP &&
dev->ieee80211_ptr->iftype != NL80211_IFTYPE_P2P_GO)
@@ -2719,9 +2721,26 @@ static int nl80211_start_ap(struct sk_buff *skb, struct genl_info *info)
if (!cfg80211_reg_can_beacon(&rdev->wiphy, ¶ms.chandef))
return -EINVAL;
+ chan = params.chandef.chan;
+
+ if (chan->flags & IEEE80211_CHAN_RADAR) {
+ if (!(rdev->wiphy.features & NL80211_FEATURE_DFS))
+ return -EINVAL;
+
+ if (!chan->cac_started)
+ return -EPERM;
+
+ if (time_is_after_jiffies(chan->radar_detect_timeout))
+ return -EPERM;
+
+ radar_detect_width = BIT(params.chandef.width);
+ }
+
mutex_lock(&rdev->devlist_mtx);
- err = cfg80211_can_use_chan(rdev, wdev, params.chandef.chan,
- CHAN_MODE_SHARED);
+ err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
+ params.chandef.chan,
+ CHAN_MODE_SHARED,
+ radar_detect_width);
mutex_unlock(&rdev->devlist_mtx);
if (err)
--
1.7.10.4
On Fri, Jan 18, 2013 at 11:08:53PM +0100, Johannes Berg wrote:
> Hi,
>
> > As stated before: "available until ..." time is not neccessary as far
> > as I know. Also the channel must stay available when AP stops operation,
> > I'll change that.
>
> Yeah, I checked the documents myself now ... strange, but hey. Radars
> are fixed installations and mostly active continually, so I guess they
> figured it was no big deal.
>
Yup. There was this "24 hours disconnect" problem many users complained about.
Some vendors ignored it, some actually implemented to be quiet for 1 minute
after using a channel for 24 hours. It's a good thing that this requirement
was removed. :)
> > > * radar detection finished:
> > > - driver: call mac80211 with result (radar detected or not)
> > > - mac80211: free up channel context (probably needs workqueue!)
> > > pass result to cfg80211
> > > - cfg80211: clear wdev radar detect chandef
> > > if OK to use: store availability in channel struct
> > > send result to userspace
> >
> > Right now we only have the event for radar detect (i.e. detection
> > failed).
> >
> > We could add a timer to mark the channel available after the CAC time,
> > if no radar has been detected in this time. Don't know if mac80211 or
> > cfg80211 would be the best place for this, but I think the driver is not
> > - this timer must be shared over various drivers after all.
>
> Fair point. It seems likely though that full-MAC chips, if they
> implement this at all, would have the timer in firmware. Therefore,
> providing API for "radar detected" and "channel is usable" would make
> more sense? The timer would then go into mac80211.
>
> Later, if we modify the regdb format/API, we could also pass the minimum
> time from there. That way, if it ever gets changed and we want to rely
> on regdb info, we could.
>
OK, will look into that.
> > > * start AP:
> > > - cfg80211: check channel availability
> > > - cfg80211/mac80211/driver? - enable radar detection while operating
> >
> > mac80211 can instruct the driver to use radar detection while operating.
> > E.g. pass it in sdata->vis.bss_conf ?
>
> I would say in the chanctx, i.e. struct ieee80211_chanctx_conf. Since
> not all drivers implement that yet, also struct ieee80211_conf. See how
> local->_oper_channel gets set in mac80211/chan.c. This information would
> be similar, the "master" information would be in the chandef, copied to
> local/local->hw.conf.
>
OK.
> > > * AP channel switch:
> > > - if due to radar, mark channel as not clear
> > OK - in AP case, we would have got the radar event anyway so the channel
> > would already be marked "not clear". In IBSS however, that is possible
> > (another station may have asked us to switch channel because of radar).
>
> Oh, I wasn't even considering that -- don't care what you do there,
> although it makes sense. OTOH, we would detect the radar ourselves, so
> it doesn't really matter?
>
It might happen that a radar is detected on one device but not on the other.
Think of a longshot operated with IBSS or a big mesh network. The other participants
of the IBSS will have to switch channels as well - and the same channel would
be good. :)
Anyway, the IBSS case is special and we can handle it later.
Cheers,
Simon
To ease further DFS development regarding interface combinations, use
the interface combinations structure to test for radar capabilities.
Drivers can specify which channel widths they support, and in which
modes. Drivers should first allow AP mode only, but can later allow
MultiSSID APs, AP+Ad-Hoc, etc.
Signed-off-by: Simon Wunderlich <[email protected]>
---
include/net/cfg80211.h | 2 ++
include/uapi/linux/nl80211.h | 3 +++
net/wireless/core.c | 11 +++++++++--
net/wireless/core.h | 7 ++++---
net/wireless/nl80211.c | 3 +++
net/wireless/util.c | 32 +++++++++++++++++++++++++++++---
6 files changed, 50 insertions(+), 8 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index e5f085c..8671feb 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2092,6 +2092,7 @@ struct ieee80211_iface_limit {
* @beacon_int_infra_match: In this combination, the beacon intervals
* between infrastructure and AP types must match. This is required
* only in special cases.
+ * @radar_detect_widths: bitmap of channel widths supported for radar detection
*
* These examples can be expressed as follows:
*
@@ -2144,6 +2145,7 @@ struct ieee80211_iface_combination {
u16 max_interfaces;
u8 n_limits;
bool beacon_int_infra_match;
+ u8 radar_detect_widths;
};
struct mac_address {
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 5470171..c8730d2 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2937,6 +2937,8 @@ enum nl80211_iface_limit_attrs {
* the infrastructure network's beacon interval.
* @NL80211_IFACE_COMB_NUM_CHANNELS: u32 attribute specifying how many
* different channels may be used within this group.
+ * @NL80211_IFACE_COMB_RADAR_DETECT_WIDTHS: u32 attribute containing the bitmap
+ * of supported channel widths for radar detection.
* @NUM_NL80211_IFACE_COMB: number of attributes
* @MAX_NL80211_IFACE_COMB: highest attribute number
*
@@ -2969,6 +2971,7 @@ enum nl80211_if_combination_attrs {
NL80211_IFACE_COMB_MAXNUM,
NL80211_IFACE_COMB_STA_AP_BI_MATCH,
NL80211_IFACE_COMB_NUM_CHANNELS,
+ NL80211_IFACE_COMB_RADAR_DETECT_WIDTHS,
/* keep last */
NUM_NL80211_IFACE_COMB,
diff --git a/net/wireless/core.c b/net/wireless/core.c
index 747dd93..7978d0e 100644
--- a/net/wireless/core.c
+++ b/net/wireless/core.c
@@ -382,8 +382,10 @@ static int wiphy_verify_combinations(struct wiphy *wiphy)
c = &wiphy->iface_combinations[i];
- /* Combinations with just one interface aren't real */
- if (WARN_ON(c->max_interfaces < 2))
+ /* Combinations with just one interface aren't real,
+ * however we make an exception for DFS.
+ */
+ if (WARN_ON((c->max_interfaces < 2) && !c->radar_detect_widths))
return -EINVAL;
/* Need at least one channel */
@@ -398,6 +400,11 @@ static int wiphy_verify_combinations(struct wiphy *wiphy)
CFG80211_MAX_NUM_DIFFERENT_CHANNELS))
return -EINVAL;
+ /* DFS only works on one channel. */
+ if (WARN_ON(c->radar_detect_widths &&
+ (c->num_different_channels > 1)))
+ return -EINVAL;
+
if (WARN_ON(!c->n_limits))
return -EINVAL;
diff --git a/net/wireless/core.h b/net/wireless/core.h
index f342267..8396f76 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -425,7 +425,8 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
struct wireless_dev *wdev,
enum nl80211_iftype iftype,
struct ieee80211_channel *chan,
- enum cfg80211_chan_mode chanmode);
+ enum cfg80211_chan_mode chanmode,
+ u8 radar_detect);
static inline int
cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
@@ -433,7 +434,7 @@ cfg80211_can_change_interface(struct cfg80211_registered_device *rdev,
enum nl80211_iftype iftype)
{
return cfg80211_can_use_iftype_chan(rdev, wdev, iftype, NULL,
- CHAN_MODE_UNDEFINED);
+ CHAN_MODE_UNDEFINED, 0);
}
static inline int
@@ -450,7 +451,7 @@ cfg80211_can_use_chan(struct cfg80211_registered_device *rdev,
enum cfg80211_chan_mode chanmode)
{
return cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
- chan, chanmode);
+ chan, chanmode, 0);
}
void
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 62e98f5..ea7d492 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -856,6 +856,9 @@ static int nl80211_put_iface_combinations(struct wiphy *wiphy,
nla_put_u32(msg, NL80211_IFACE_COMB_MAXNUM,
c->max_interfaces))
goto nla_put_failure;
+ if (nla_put_u32(msg, NL80211_IFACE_COMB_RADAR_DETECT_WIDTHS,
+ c->radar_detect_widths))
+ goto nla_put_failure;
nla_nest_end(msg, nl_combi);
}
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 16d76a8..72476e8 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -1184,7 +1184,8 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
struct wireless_dev *wdev,
enum nl80211_iftype iftype,
struct ieee80211_channel *chan,
- enum cfg80211_chan_mode chanmode)
+ enum cfg80211_chan_mode chanmode,
+ u8 radar_detect)
{
struct wireless_dev *wdev_iter;
u32 used_iftypes = BIT(iftype);
@@ -1195,14 +1196,36 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
enum cfg80211_chan_mode chmode;
int num_different_channels = 0;
int total = 1;
+ bool radar_required;
int i, j;
ASSERT_RTNL();
lockdep_assert_held(&rdev->devlist_mtx);
+ if (WARN_ON(hweight32(radar_detect) > 1))
+ return -EINVAL;
+
+ switch (iftype) {
+ case NL80211_IFTYPE_ADHOC:
+ case NL80211_IFTYPE_AP:
+ case NL80211_IFTYPE_AP_VLAN:
+ case NL80211_IFTYPE_MESH_POINT:
+ case NL80211_IFTYPE_P2P_GO:
+ radar_required = !!(chan->flags & IEEE80211_CHAN_RADAR);
+ break;
+ default:
+ radar_required = false;
+ }
+
+ if (radar_required && !radar_detect)
+ return -EINVAL;
+
/* Always allow software iftypes */
- if (rdev->wiphy.software_iftypes & BIT(iftype))
+ if (rdev->wiphy.software_iftypes & BIT(iftype)) {
+ if (radar_detect)
+ return -EINVAL;
return 0;
+ }
memset(num, 0, sizeof(num));
memset(used_channels, 0, sizeof(used_channels));
@@ -1275,7 +1298,7 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
used_iftypes |= BIT(wdev_iter->iftype);
}
- if (total == 1)
+ if (total == 1 && !radar_detect)
return 0;
for (i = 0; i < rdev->wiphy.n_iface_combinations; i++) {
@@ -1308,6 +1331,9 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
}
}
+ if (radar_detect && !(c->radar_detect_widths & radar_detect))
+ goto cont;
+
/*
* Finally check that all iftypes that we're currently
* using are actually part of this combination. If they
--
1.7.10.4
On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
> Any comment/review/etc are very much appreciated!
I'm still confused about what the desired behaviour should be.
>From what we discussed the following are valid use cases:
1) do CAC on all channels, select one that is usable later
2) do CAC with one NIC, use it with another NIC (hot standby)
We can address 2) by storing the "channel is clear" evaluation in the
channel struct -- although then it'll only work with the same driver
etc. This can be resolved later anyway.
Case 1) certainly implies that we need something like "channel
considered usable until ..." timer. You don't have that at all. Instead,
you seem to consider it clear until an AP interface is started & stopped
on that channel, or otherwise forever. That seems wrong.
So I think that in the channel struct/globally we really only need the
"channel_usable_until" value, which is X seconds (? I don't know what
the reg rules say) after the radar detection completed.
Actually with your current patch 2) cannot be implemented at all because
the initial CAC never really finishes?
I think the operations here should be like this:
* start radar detection
- cfg80211: store radar detect chandef in wdev struct to use it in
cfg80211_get_chan_state()
- mac80211: configure chanctx, call driver
- driver: start radar detection
* radar detection finished:
- driver: call mac80211 with result (radar detected or not)
- mac80211: free up channel context (probably needs workqueue!)
pass result to cfg80211
- cfg80211: clear wdev radar detect chandef
if OK to use: store availability in channel struct
send result to userspace
* start AP:
- cfg80211: check channel availability
- cfg80211/mac80211/driver? - enable radar detection while operating
* AP channel switch:
- if due to radar, mark channel as not clear
- if not due to radar, mark channel as clear until X seconds in the
future
* stop AP:
- cfg80211: mark channel as clear until X seconds in the future,
unless it was marked not clear by a radar detect event
* radar detected event:
- cfg80211: mark channel as not clear
Note how I'm also letting the drivers determine the duration of the
radar check. Maybe the duration should be passed to the driver, or maybe
not, but I think for full-MAC chips it would be more likely that they
get an event for both cases from the device, not just the timeout you
have in software ...
Actually, now that I think about it, I have my doubts about the mac80211
"start_radar_detect()" API. It seems that it really just starts the
detection, and has no way to stop it etc. So maybe that timer should be
in *mac80211* (for the current drivers), and should also be called after
adding a chanctx when starting the AP vif. Or maybe it should actually
be tied to "add_chanctx()" (or "config()" for backward compat) -- when a
channel requiring radar detection is configured, detection should be
enabled?
johannes
On Thu, 2013-01-17 at 14:52 +0100, Simon Wunderlich wrote:
> > > + res = drv_start_radar_detection(local, sdata, chandef);
> >
> > If the vif is assigned the channel, why also pass it to the
> > start_radar_detection command? That seems pointless, they can't be
> > different?
> >
> In the initial phase, __nl80211_set_channel() should not be able to set
> the channel (no CAC yet), and will fail for IBSS (to be implemented later)
> generally, at least as it's implemented now.
>
> But what we can do is set the channel from mac80211 and call the driver
> function without the channel argument, as it'll be pointless in this case.
> Is this what you mean?
Hmm. Why are you talking about __nl80211_set_channel() now? We're now
talking about mac80211, which can and does set the channel (chanctx,
which may fall back to drv_config()) before calling
drv_start_radar_detection(), so I think the latter needs no chandef
argument?
> > Actually that raises another question: If we have "external" radar
> > detection, say by a different NIC, then shouldn't we still ask the
> > driver to start radar detection when using the channel? Or is that
> > implicit, does the driver have to check?
>
> That is a good question, I didn't consider that. In the "simple process"
> we first start radar detection and start the ap on the same channel.
> For external CAC that won't work, of course. What about mac80211 checking
> the channel in start_ap(), and if the channel requires DFS, pass some flag
> to the driver that radar detection should be enabled?
Yes, that makes sense. But then it would also make sense to remove the
start_radar_detection() callback entirely, and encode all that
information in the channel context/drv_config call?
If mac80211 gets to be responsible for it, this should totally be
documented in the cfg80211 API though so if a full-mac driver wants to
implement it they know what to do. I do think this is the reasonable way
of doing it though.
Note that I'm not advocating removing the start_radar_detection() or its
chandef from the *cfg80211* API. That is clearly needed. But in mac80211
it seems "set this chandef with radar detection" is a better API?
> > > +#define CHAN_DEF_ENTRY __field(enum ieee80211_band, band) \
> >
> > I'll apply my "mac80211: split out chandef tracing macros" patch
> > instead.
>
> OK.
Done, though it seems you don't need it if you remove the chandef
argument from start_radar_detection, or even remove the callback
entirely.
johannes
On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich 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_started: indicates that channel availability check is started for this
> + * channel type.
So you're enforcing a certain CAC time, but not the time we are allowed
to treat the channel as clear? Shouldn't *that* be in each channel
struct, rather than the other stuff?
It also seems to me that "cac_started" isn't really all that relevant in
the channel struct either. What seems relevant is the *result* of the
CAC, and how long it's still valid, no?
> +++ b/net/wireless/chan.c
> @@ -287,14 +287,18 @@ bool cfg80211_reg_can_beacon(struct wiphy *wiphy,
> struct cfg80211_chan_def *chandef)
> {
> bool res;
> + u32 prohibited_flags;
>
> trace_cfg80211_reg_can_beacon(wiphy, chandef);
>
> - res = cfg80211_chandef_usable(wiphy, chandef,
> - IEEE80211_CHAN_DISABLED |
> - IEEE80211_CHAN_PASSIVE_SCAN |
> - IEEE80211_CHAN_NO_IBSS |
> - IEEE80211_CHAN_RADAR);
> + prohibited_flags = IEEE80211_CHAN_DISABLED;
> +
> + if (!(wiphy->features & NL80211_FEATURE_DFS))
> + prohibited_flags |= IEEE80211_CHAN_PASSIVE_SCAN |
> + IEEE80211_CHAN_NO_IBSS |
> + IEEE80211_CHAN_RADAR;
I have a feeling this change should take into account the channel width,
and whether CAC completed successfully?
> +static int nl80211_start_radar_detection(struct sk_buff *skb,
> + struct genl_info *info)
> +{
> + struct cfg80211_registered_device *rdev = info->user_ptr[0];
> + struct net_device *dev = info->user_ptr[1];
> + struct wireless_dev *wdev = dev->ieee80211_ptr;
> + struct cfg80211_chan_def chandef;
> + int err;
> +
> + if (!(rdev->wiphy.features & NL80211_FEATURE_DFS))
> + return -EOPNOTSUPP;
> +
> + err = nl80211_parse_chandef(rdev, info, &chandef);
> + if (err)
> + return err;
> +
> + if (!(chandef.chan->flags & IEEE80211_CHAN_RADAR))
> + return -EINVAL;
> +
> + if (chandef.chan->cac_started)
> + return -EBUSY;
> +
> + if (!rdev->ops->start_radar_detection)
> + return -EOPNOTSUPP;
> +
> + mutex_lock(&rdev->devlist_mtx);
> + err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
> + chandef.chan, CHAN_MODE_SHARED,
> + BIT(chandef.width));
> + mutex_unlock(&rdev->devlist_mtx);
> +
> + if (err)
> + return err;
> +
> + err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, &chandef);
> + if (!err) {
> + wdev->preset_chandef = chandef;
> + chandef.chan->cac_started = true;
> + chandef.chan->radar_detect_timeout = jiffies +
> + msecs_to_jiffies(NL80211_DFS_MIN_CAC_TIME_MS);
> + }
> +
> + return err;
> +}
This still seems somewhat wrong. For the duration of the CAC, the
channel should be "locked" in some way, no? As it stands now, nothing
prevents userspace from adding another vif and using it for something
entirely different, while cfg80211 thinks the CAC is actually running.
> + 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;
That should be the entire chandef info, and possibly the WDEV_ID too.
johannes
On Wed, Jan 16, 2013 at 11:44:47PM +0100, Johannes Berg wrote:
> On Wed, 2013-01-16 at 23:42 +0100, Johannes Berg wrote:
> > On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
> > > To ease further DFS development regarding interface combinations, use
> > > the interface combinations structure to test for radar capabilities.
> > > Drivers can specify which channel widths they support, and in which
> > > modes. Drivers should first allow AP mode only, but can later allow
> > > MultiSSID APs, AP+Ad-Hoc, etc.
> >
> > Applied.
>
> Btw, an iw patch would be nice.
OK, will put that on my TODO.
Hey Luca,
On Wed, Jan 30, 2013 at 06:34:48PM +0200, Luciano Coelho wrote:
> [...]
>
> > @@ -1195,14 +1196,36 @@ int cfg80211_can_use_iftype_chan(struct cfg80211_registered_device *rdev,
> > enum cfg80211_chan_mode chmode;
> > int num_different_channels = 0;
> > int total = 1;
> > + bool radar_required;
> > int i, j;
> >
> > ASSERT_RTNL();
> > lockdep_assert_held(&rdev->devlist_mtx);
> >
> > + if (WARN_ON(hweight32(radar_detect) > 1))
> > + return -EINVAL;
> > +
> > + switch (iftype) {
> > + case NL80211_IFTYPE_ADHOC:
> > + case NL80211_IFTYPE_AP:
> > + case NL80211_IFTYPE_AP_VLAN:
> > + case NL80211_IFTYPE_MESH_POINT:
> > + case NL80211_IFTYPE_P2P_GO:
> > + radar_required = !!(chan->flags & IEEE80211_CHAN_RADAR);
> > + break;
>
> This code is causing an oops with the wl18xx driver in AP mode. The
> problem is that cfg80211_can_change_interface() calls
> cfg80211_can_use_iftype_chan() with chan == NULL. This code doesn't
> check if chan is NULL, so this dereference causes the oops.
Sorry about that - I believe you've found the same bug I've posted a patch
for a few days ago. Johannes already has this in mac80211-next, but it is
not yet in wireless-testing:
http://article.gmane.org/gmane.linux.kernel.wireless.general/102836/match=simon
http://git.kernel.org/?p=linux/kernel/git/jberg/mac80211-next.git;a=commit;h=683d41ae6755e6ae297ec09603c229795ab9566e
>
> I don't have the time right now to fix this, but I'll look into it
> tomorrow (unless someone comes with a fix before that :P).
Please have a look at the patch posted above (both links for the same patch).
Cheers,
Simon
> [...]
Hey Zefir,
On Mon, Jan 21, 2013 at 11:44:22AM +0100, Zefir Kurtisi wrote:
> On 01/18/2013 10:54 PM, Johannes Berg wrote:
> > On Thu, 2013-01-17 at 14:40 +0100, Simon Wunderlich wrote:
> >
> >> Actually there is no limit how long a channel is considered "available", at
> >> least in ETSI. ETSI EN 301-893 v1.4.1 had a limit of 24 hours for that,
> >> but that was removed in v1.5.1 and didn't re-appear since then (current is
> >> v1.7.1).
> >
> > Huh indeed, I would have expected that to be there. It does have a
> > non-occupancy time though (30 minutes), maybe we should implement that?
> >
> No, why be more restrictive than regulatory demands?
>
> With the recent incremental updates, a general note: Victor's initial approach was
> to keep all logic in hostapd and minimize the modifications in mac by only
> ensuring CAC times there. If NOP handling is also added to mac, we'd have
> everything needed to handle DFS channel states available. With hostapd only left
> to do the selection of the channel to switch to after a radar detection, it might
> make sense to move everything down to mac. I understand that questioning the
> design that late is not helpful, at the same time and since the initial path was
> left, it might be worth considering.
Actually questioning the design NOW is a good idea, we are already questioning it
through the last patches and better bring up the issues now than later. We have started from the
simple hostap-handles-everything approach and have seen some points are missing when
we think about multi interfaces etc for the future (and implementation issues of course).
If we move channel states (available/unavailable) already into the kernel space, we might
as well check for other states (already doing CAC, not valid until). Maybe it's better like
this than splitting the management over hostapd and cfg/mac80211?
> > I'm also thinking with the next regdb format update we should allow
> > specifying these timeouts etc. there.
> >
> > Does anyone have the relevant FCC rules? I can't find anything with
> > google ...
> >
> The FCC 06-96 document (freely available, e.g.
> http://hraunfoss.fcc.gov/edocs_public/attachmatch/FCC-06-96A1.pdf) seems to be the
> most recent one. Skimming over I did not find a requirement for the validity
> period after CAC.
>
Thanks for pointing us to that doc. Couldn't find anything either, so I'd just
skip any "validity" for now.
> >> But we can move the CAC/timeout in the wdev and have keep a flag field in
> >> the channel struct instead, marking the channel as available, unavailable, etc.
> >>
> >> What do you think?
> >
> > I think that would make sense. Probably available/unavailable and
> > "non-occupancy until"?
> >
> At that stage, we would have half of all potential states (UNKNOWN, AVAILABLE,
> OPERATING, UNAVAILABLE, USABLE, SCANNING) covered, so a current state per channel
> and the time it was entered would give everything required for the complete state
> machine in mac.
Sounds good! Will look into this ...
Thanks,
Simon
On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
> Scanning and remain on channel functionality must be disabled while
> doing radar detection/scanning, and vice versa.
This really belongs into the previous patch, I think?
johannes
On 01/18/2013 10:54 PM, Johannes Berg wrote:
> On Thu, 2013-01-17 at 14:40 +0100, Simon Wunderlich wrote:
>
>> Actually there is no limit how long a channel is considered "available", at
>> least in ETSI. ETSI EN 301-893 v1.4.1 had a limit of 24 hours for that,
>> but that was removed in v1.5.1 and didn't re-appear since then (current is
>> v1.7.1).
>
> Huh indeed, I would have expected that to be there. It does have a
> non-occupancy time though (30 minutes), maybe we should implement that?
>
No, why be more restrictive than regulatory demands?
With the recent incremental updates, a general note: Victor's initial approach was
to keep all logic in hostapd and minimize the modifications in mac by only
ensuring CAC times there. If NOP handling is also added to mac, we'd have
everything needed to handle DFS channel states available. With hostapd only left
to do the selection of the channel to switch to after a radar detection, it might
make sense to move everything down to mac. I understand that questioning the
design that late is not helpful, at the same time and since the initial path was
left, it might be worth considering.
> I'm also thinking with the next regdb format update we should allow
> specifying these timeouts etc. there.
>
> Does anyone have the relevant FCC rules? I can't find anything with
> google ...
>
The FCC 06-96 document (freely available, e.g.
http://hraunfoss.fcc.gov/edocs_public/attachmatch/FCC-06-96A1.pdf) seems to be the
most recent one. Skimming over I did not find a requirement for the validity
period after CAC.
>> But we can move the CAC/timeout in the wdev and have keep a flag field in
>> the channel struct instead, marking the channel as available, unavailable, etc.
>>
>> What do you think?
>
> I think that would make sense. Probably available/unavailable and
> "non-occupancy until"?
>
At that stage, we would have half of all potential states (UNKNOWN, AVAILABLE,
OPERATING, UNAVAILABLE, USABLE, SCANNING) covered, so a current state per channel
and the time it was entered would give everything required for the complete state
machine in mac.
>
> [...]
From: Victor Goldenshtein <[email protected]>
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 <[email protected]>
[rebase, change to chandef, minor fixes]
Signed-off-by: Simon Wunderlich <[email protected]>
---
Changes to PATCHv5:
* use local instead of sdata->local
* change API to chandef
---
include/net/mac80211.h | 19 ++++++++++++
net/mac80211/cfg.c | 27 ++++++++++++++++
net/mac80211/driver-ops.h | 15 +++++++++
net/mac80211/iface.c | 4 +++
net/mac80211/mlme.c | 11 +++++++
net/mac80211/trace.h | 76 +++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 152 insertions(+)
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 23daed3..ab9748d 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -2487,6 +2487,10 @@ enum ieee80211_rate_control_changed {
* @restart_complete: Called after a call to ieee80211_restart_hw(), when the
* reconfiguration has completed. This can help the driver implement the
* reconfiguration step. This callback may 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,
@@ -2660,6 +2664,10 @@ struct ieee80211_ops {
struct ieee80211_chanctx_conf *ctx);
void (*restart_complete)(struct ieee80211_hw *hw);
+
+ int (*start_radar_detection)(struct ieee80211_hw *hw,
+ struct ieee80211_vif *vif,
+ struct cfg80211_chan_def *chandef);
};
/**
@@ -3889,6 +3897,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 7d290bc..bd300e4 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2588,6 +2588,32 @@ static int ieee80211_cancel_remain_on_channel(struct wiphy *wiphy,
return ieee80211_cancel_roc(local, cookie, false);
}
+static int ieee80211_start_radar_detection(struct wiphy *wiphy,
+ struct net_device *dev,
+ struct cfg80211_chan_def *chandef)
+{
+ struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
+ struct ieee80211_local *local = sdata->local;
+ int res;
+
+ if (!local->ops->start_radar_detection)
+ return -EOPNOTSUPP;
+
+ /* whatever, but channel contexts should not complain about that one */
+ sdata->smps_mode = IEEE80211_SMPS_OFF;
+ sdata->needed_rx_chains = local->rx_chains;
+
+ if (ieee80211_vif_use_channel(sdata, chandef,
+ IEEE80211_CHANCTX_SHARED))
+ return -EBUSY;
+
+ res = drv_start_radar_detection(local, sdata, chandef);
+ if (res)
+ ieee80211_vif_release_channel(sdata);
+
+ return res;
+}
+
static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
struct ieee80211_channel *chan, bool offchan,
unsigned int wait, const u8 *buf, size_t len,
@@ -3292,4 +3318,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 0c07f94..2b3bdd9 100644
--- a/net/mac80211/driver-ops.h
+++ b/net/mac80211/driver-ops.h
@@ -331,6 +331,21 @@ 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 cfg80211_chan_def *chandef)
+{
+ int ret;
+
+ might_sleep();
+
+ trace_drv_start_radar_detection(local, sdata, chandef);
+ ret = local->ops->start_radar_detection(&local->hw, &sdata->vif,
+ chandef);
+ 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 06fac29..3dbd81b 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -826,6 +826,10 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
u.vlan.list)
dev_close(vlan->dev);
WARN_ON(!list_empty(&sdata->u.ap.vlans));
+
+ /* 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 d90c07b1..ce06771 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -4131,3 +4131,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 a8270b4..553d25b 100644
--- a/net/mac80211/trace.h
+++ b/net/mac80211/trace.h
@@ -45,6 +45,35 @@
__entry->center_freq1, __entry->center_freq2, \
__entry->rx_chains_static, __entry->rx_chains_dynamic
+#define CHAN_DEF_ENTRY __field(enum ieee80211_band, band) \
+ __field(u32, control_freq) \
+ __field(u32, width) \
+ __field(u32, center_freq1) \
+ __field(u32, center_freq2)
+#define CHAN_DEF_ASSIGN(chandef) \
+ do { \
+ if ((chandef) && (chandef)->chan) { \
+ __entry->band = (chandef)->chan->band; \
+ __entry->control_freq = \
+ (chandef)->chan->center_freq; \
+ __entry->width = (chandef)->width; \
+ __entry->center_freq1 = (chandef)->center_freq1;\
+ __entry->center_freq2 = (chandef)->center_freq2;\
+ } else { \
+ __entry->band = 0; \
+ __entry->control_freq = 0; \
+ __entry->width = 0; \
+ __entry->center_freq1 = 0; \
+ __entry->center_freq2 = 0; \
+ } \
+ } while (0)
+#define CHAN_DEF_PR_FMT \
+ "band: %d, control freq: %u, width: %d, cf1: %u, cf2: %u"
+#define CHAN_DEF_PR_ARG __entry->band, __entry->control_freq, \
+ __entry->width, __entry->center_freq1, \
+ __entry->center_freq2
+
+
/*
@@ -1815,6 +1844,53 @@ 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_sub_if_data *sdata,
+ struct cfg80211_chan_def *chandef),
+
+ TP_ARGS(local, sdata, chandef),
+
+ TP_STRUCT__entry(
+ LOCAL_ENTRY
+ VIF_ENTRY
+ CHAN_DEF_ENTRY
+ ),
+
+ TP_fast_assign(
+ LOCAL_ASSIGN;
+ VIF_ASSIGN;
+ CHAN_DEF_ASSIGN(chandef);
+ ),
+
+ TP_printk(
+ LOCAL_PR_FMT " start radar detection on: " CHAN_DEF_PR_FMT,
+ LOCAL_PR_ARG, CHAN_DEF_PR_ARG
+ )
+);
+
#ifdef CONFIG_MAC80211_MESSAGE_TRACING
#undef TRACE_SYSTEM
#define TRACE_SYSTEM mac80211_msg
--
1.7.10.4
Johannes,
as always, thanks for the detailed review!
On Thu, Jan 17, 2013 at 12:22:54AM +0100, Johannes Berg wrote:
> On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
>
> > Any comment/review/etc are very much appreciated!
>
> I'm still confused about what the desired behaviour should be.
>
> >From what we discussed the following are valid use cases:
>
> 1) do CAC on all channels, select one that is usable later
> 2) do CAC with one NIC, use it with another NIC (hot standby)
>
> We can address 2) by storing the "channel is clear" evaluation in the
> channel struct -- although then it'll only work with the same driver
> etc. This can be resolved later anyway.
>
> Case 1) certainly implies that we need something like "channel
> considered usable until ..." timer. You don't have that at all. Instead,
> you seem to consider it clear until an AP interface is started & stopped
> on that channel, or otherwise forever. That seems wrong.
>
> So I think that in the channel struct/globally we really only need the
> "channel_usable_until" value, which is X seconds (? I don't know what
> the reg rules say) after the radar detection completed.
As stated before: "available until ..." time is not neccessary as far
as I know. Also the channel must stay available when AP stops operation,
I'll change that.
> Actually with your current patch 2) cannot be implemented at all because
> the initial CAC never really finishes?
Yeah, for this case it is flawed - one could start radar detection on
different channels one after another in short time, and we would just
set "cac_started" and the timeout - which eventually might mark all channels
available without checking for the required time on it. That seems wrong.
>
> I think the operations here should be like this:
>
> * start radar detection
> - cfg80211: store radar detect chandef in wdev struct to use it in
> cfg80211_get_chan_state()
> - mac80211: configure chanctx, call driver
> - driver: start radar detection
OK.
> * radar detection finished:
> - driver: call mac80211 with result (radar detected or not)
> - mac80211: free up channel context (probably needs workqueue!)
> pass result to cfg80211
> - cfg80211: clear wdev radar detect chandef
> if OK to use: store availability in channel struct
> send result to userspace
Right now we only have the event for radar detect (i.e. detection
failed).
We could add a timer to mark the channel available after the CAC time,
if no radar has been detected in this time. Don't know if mac80211 or
cfg80211 would be the best place for this, but I think the driver is not
- this timer must be shared over various drivers after all.
> * start AP:
> - cfg80211: check channel availability
> - cfg80211/mac80211/driver? - enable radar detection while operating
mac80211 can instruct the driver to use radar detection while operating.
E.g. pass it in sdata->vis.bss_conf ?
> * AP channel switch:
> - if due to radar, mark channel as not clear
OK - in AP case, we would have got the radar event anyway so the channel
would already be marked "not clear". In IBSS however, that is possible
(another station may have asked us to switch channel because of radar).
> - if not due to radar, mark channel as clear until X seconds in the
> future
Timeout not needed.
> * stop AP:
> - cfg80211: mark channel as clear until X seconds in the future,
> unless it was marked not clear by a radar detect event
Timeout not needed, just keep it clear.
> * radar detected event:
> - cfg80211: mark channel as not clear
OK
>
>
> Note how I'm also letting the drivers determine the duration of the
> radar check. Maybe the duration should be passed to the driver, or maybe
> not, but I think for full-MAC chips it would be more likely that they
> get an event for both cases from the device, not just the timeout you
> have in software ...
Hm, I don't know. CAC times are subject to regulation changes. In ETSI
it's one minute, for weather channels (5600-5650 MHz) 10 minutes (but we
better don't support them anyway as they require incredible radar
detection rates).
> Actually, now that I think about it, I have my doubts about the mac80211
> "start_radar_detect()" API. It seems that it really just starts the
> detection, and has no way to stop it etc. So maybe that timer should be
> in *mac80211* (for the current drivers), and should also be called after
> adding a chanctx when starting the AP vif. Or maybe it should actually
> be tied to "add_chanctx()" (or "config()" for backward compat) -- when a
> channel requiring radar detection is configured, detection should be
> enabled?
The radar detect command was originally planned to just start the detection,
yes. Userspace should just "try" to use the channel after CAC time - if the
channel should be used earlier, start_ap or whatever will just fail. So yes,
it just starts the radar detection, stopping it didn't seem neccesary yet.
If we change the concept to explicitly handle the end of CAC in the kernel,
the timer approach is imho the best.
Cheers,
Simon
On Wed, Jan 16, 2013 at 11:35:59PM +0100, Johannes Berg wrote:
> On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
> > IBSS should also consider interface combinations.
> >
> > Signed-off-by: Simon Wunderlich <[email protected]>
> > ---
> > net/wireless/nl80211.c | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> > index 9bd8340..62e98f5 100644
> > --- a/net/wireless/nl80211.c
> > +++ b/net/wireless/nl80211.c
> > @@ -5463,6 +5463,7 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
> > {
> > struct cfg80211_registered_device *rdev = info->user_ptr[0];
> > struct net_device *dev = info->user_ptr[1];
> > + struct wireless_dev *wdev = dev->ieee80211_ptr;
> > struct cfg80211_ibss_params ibss;
> > struct wiphy *wiphy;
> > struct cfg80211_cached_keys *connkeys = NULL;
> > @@ -5524,6 +5525,15 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
> > ibss.channel_fixed = !!info->attrs[NL80211_ATTR_FREQ_FIXED];
> > ibss.privacy = !!info->attrs[NL80211_ATTR_PRIVACY];
> >
> > + mutex_lock(&rdev->devlist_mtx);
> > + err = cfg80211_can_use_chan(rdev, wdev, ibss.chandef.chan,
> > + ibss.channel_fixed ? CHAN_MODE_SHARED :
> > + CHAN_MODE_EXCLUSIVE);
> > + mutex_unlock(&rdev->devlist_mtx);
> > +
> > + if (err)
> > + return -EINVAL;
> > +
>
> This is already in __cfg80211_join_ibss()
Whoops, missed that, sorry. Then we can skip this patch.
Cheers,
Simon
On 01/23/2013 01:49 PM, Simon Wunderlich wrote:
> Hey Zefir,
>
> On Mon, Jan 21, 2013 at 11:44:22AM +0100, Zefir Kurtisi wrote:
>> On 01/18/2013 10:54 PM, Johannes Berg wrote:
>>> On Thu, 2013-01-17 at 14:40 +0100, Simon Wunderlich wrote:
>>>
>>>> Actually there is no limit how long a channel is considered "available", at
>>>> least in ETSI. ETSI EN 301-893 v1.4.1 had a limit of 24 hours for that,
>>>> but that was removed in v1.5.1 and didn't re-appear since then (current is
>>>> v1.7.1).
>>>
>>> Huh indeed, I would have expected that to be there. It does have a
>>> non-occupancy time though (30 minutes), maybe we should implement that?
>>>
>> No, why be more restrictive than regulatory demands?
>>
>> With the recent incremental updates, a general note: Victor's initial approach was
>> to keep all logic in hostapd and minimize the modifications in mac by only
>> ensuring CAC times there. If NOP handling is also added to mac, we'd have
>> everything needed to handle DFS channel states available. With hostapd only left
>> to do the selection of the channel to switch to after a radar detection, it might
>> make sense to move everything down to mac. I understand that questioning the
>> design that late is not helpful, at the same time and since the initial path was
>> left, it might be worth considering.
>
> Actually questioning the design NOW is a good idea, we are already questioning it
> through the last patches and better bring up the issues now than later. We have started from the
> simple hostap-handles-everything approach and have seen some points are missing when
> we think about multi interfaces etc for the future (and implementation issues of course).
>
> If we move channel states (available/unavailable) already into the kernel space, we might
> as well check for other states (already doing CAC, not valid until). Maybe it's better like
> this than splitting the management over hostapd and cfg/mac80211?
>
As written in my other post today, managed master mode can't be supported (as long
as we can't override channel states), so for our needs we need a different approach.
Remarkably, the proof-of-concept I proposed as 'poor-man's solution' in Vancouver
two years ago turned out to be quite well usable. It goes like this:
*) hostapd: patched to allow DFS channels
*) ath9k: patched to enable radar detection on DFS-channels
*) channel states handled by a daemon in user space
Switching to AVAILABLE channel:
*) set up master immediately
Switching to non AVAILABLE channel:
*) set up monitor
*) wait CAC
*) set up master if no radar
*) else select different channel
Since I need to patch / bypass the master functionality to enable managed DFS
mode, I am naturally biased. Nevertheless, ideally everything should be in
cfg/mac, as long as there is no compelling reason to split it with hostapd.
>>> I'm also thinking with the next regdb format update we should allow
>>> specifying these timeouts etc. there.
>>>
>>> Does anyone have the relevant FCC rules? I can't find anything with
>>> google ...
>>>
>> The FCC 06-96 document (freely available, e.g.
>> http://hraunfoss.fcc.gov/edocs_public/attachmatch/FCC-06-96A1.pdf) seems to be the
>> most recent one. Skimming over I did not find a requirement for the validity
>> period after CAC.
>>
>
> Thanks for pointing us to that doc. Couldn't find anything either, so I'd just
> skip any "validity" for now.
>
>>>> But we can move the CAC/timeout in the wdev and have keep a flag field in
>>>> the channel struct instead, marking the channel as available, unavailable, etc.
>>>>
>>>> What do you think?
>>>
>>> I think that would make sense. Probably available/unavailable and
>>> "non-occupancy until"?
>>>
>> At that stage, we would have half of all potential states (UNKNOWN, AVAILABLE,
>> OPERATING, UNAVAILABLE, USABLE, SCANNING) covered, so a current state per channel
>> and the time it was entered would give everything required for the complete state
>> machine in mac.
>
> Sounds good! Will look into this ...
>
State machine is trivial. If you need a reference implementation and are not
scared of C++ code, I can provide you mine.
> Thanks,
> Simon
>
Thanks for handling this,
Zefir
From: Victor Goldenshtein <[email protected]>
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_DFS attribute which
indicates that driver/HW supports radar detection.
Signed-off-by: Victor Goldenshtein <[email protected]>
[changes:
* change to chandef
* rebase on current master]
Signed-off-by: Simon Wunderlich <[email protected]>
---
Changes to PATCHv5:
* remove unused cac_type
* fix doc for cac_started
* remove dfs_nlportid
---
include/net/cfg80211.h | 24 +++++++++++
include/uapi/linux/nl80211.h | 9 +++++
net/wireless/chan.c | 14 ++++---
net/wireless/mlme.c | 13 ++++++
net/wireless/nl80211.c | 90 ++++++++++++++++++++++++++++++++++++++++++
net/wireless/nl80211.h | 6 +++
6 files changed, 151 insertions(+), 5 deletions(-)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8671feb..00d8f2c 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -133,6 +133,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_started: indicates that channel availability check is started for this
+ * channel type.
*/
struct ieee80211_channel {
enum ieee80211_band band;
@@ -145,6 +150,8 @@ struct ieee80211_channel {
bool beacon_found;
u32 orig_flags;
int orig_mag, orig_mpwr;
+ unsigned long radar_detect_timeout;
+ bool cac_started;
};
/**
@@ -1763,6 +1770,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);
@@ -1983,6 +1992,10 @@ struct cfg80211_ops {
struct wireless_dev *wdev);
void (*stop_p2p_device)(struct wiphy *wiphy,
struct wireless_dev *wdev);
+
+ int (*start_radar_detection)(struct wiphy *wiphy,
+ struct net_device *dev,
+ struct cfg80211_chan_def *chandef);
};
/*
@@ -3571,6 +3584,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/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index c8730d2..3a14310 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -586,6 +586,9 @@
* @NL80211_CMD_SET_MCAST_RATE: Change the rate used to send multicast frames
* for IBSS or MESH vif.
*
+ * @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
*/
@@ -736,6 +739,8 @@ enum nl80211_commands {
NL80211_CMD_SET_MCAST_RATE,
+ NL80211_CMD_RADAR_DETECT,
+
/* add new commands above here */
/* used to define NL80211_CMD_MAX below */
@@ -1635,6 +1640,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
*
@@ -3158,6 +3165,7 @@ enum nl80211_ap_sme_features {
* Note that even for drivers that support this, the default is to add
* stations in authenticated/associated state, so to add unauthenticated
* stations the authenticated/associated bits have to be set in the mask.
+ * @NL80211_FEATURE_DFS: Radar detection is supported in the HW/driver.
*/
enum nl80211_feature_flags {
NL80211_FEATURE_SK_TX_STATUS = 1 << 0,
@@ -3174,6 +3182,7 @@ enum nl80211_feature_flags {
NL80211_FEATURE_P2P_GO_CTWIN = 1 << 11,
NL80211_FEATURE_P2P_GO_OPPPS = 1 << 12,
NL80211_FEATURE_FULL_AP_CLIENT_STATE = 1 << 13,
+ NL80211_FEATURE_DFS = 1 << 14,
};
/**
diff --git a/net/wireless/chan.c b/net/wireless/chan.c
index 396373f..eefb1f2 100644
--- a/net/wireless/chan.c
+++ b/net/wireless/chan.c
@@ -287,14 +287,18 @@ bool cfg80211_reg_can_beacon(struct wiphy *wiphy,
struct cfg80211_chan_def *chandef)
{
bool res;
+ u32 prohibited_flags;
trace_cfg80211_reg_can_beacon(wiphy, chandef);
- res = cfg80211_chandef_usable(wiphy, chandef,
- IEEE80211_CHAN_DISABLED |
- IEEE80211_CHAN_PASSIVE_SCAN |
- IEEE80211_CHAN_NO_IBSS |
- IEEE80211_CHAN_RADAR);
+ prohibited_flags = IEEE80211_CHAN_DISABLED;
+
+ if (!(wiphy->features & NL80211_FEATURE_DFS))
+ prohibited_flags |= IEEE80211_CHAN_PASSIVE_SCAN |
+ IEEE80211_CHAN_NO_IBSS |
+ IEEE80211_CHAN_RADAR;
+
+ res = cfg80211_chandef_usable(wiphy, chandef, prohibited_flags);
trace_cfg80211_return_bool(res);
return res;
diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c
index 461e692..6b63ffb 100644
--- a/net/wireless/mlme.c
+++ b/net/wireless/mlme.c
@@ -987,3 +987,16 @@ void cfg80211_pmksa_candidate_notify(struct net_device *dev, int index,
nl80211_pmksa_candidate_notify(rdev, dev, index, bssid, preauth, gfp);
}
EXPORT_SYMBOL(cfg80211_pmksa_candidate_notify);
+
+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 ea7d492..d98c1d3 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -4812,6 +4812,51 @@ static int nl80211_stop_sched_scan(struct sk_buff *skb,
return err;
}
+static int nl80211_start_radar_detection(struct sk_buff *skb,
+ struct genl_info *info)
+{
+ struct cfg80211_registered_device *rdev = info->user_ptr[0];
+ struct net_device *dev = info->user_ptr[1];
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
+ struct cfg80211_chan_def chandef;
+ int err;
+
+ if (!(rdev->wiphy.features & NL80211_FEATURE_DFS))
+ return -EOPNOTSUPP;
+
+ err = nl80211_parse_chandef(rdev, info, &chandef);
+ if (err)
+ return err;
+
+ if (!(chandef.chan->flags & IEEE80211_CHAN_RADAR))
+ return -EINVAL;
+
+ if (chandef.chan->cac_started)
+ return -EBUSY;
+
+ if (!rdev->ops->start_radar_detection)
+ return -EOPNOTSUPP;
+
+ mutex_lock(&rdev->devlist_mtx);
+ err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
+ chandef.chan, CHAN_MODE_SHARED,
+ BIT(chandef.width));
+ mutex_unlock(&rdev->devlist_mtx);
+
+ if (err)
+ return err;
+
+ err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, &chandef);
+ if (!err) {
+ wdev->preset_chandef = chandef;
+ chandef.chan->cac_started = true;
+ chandef.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,
@@ -7805,6 +7850,14 @@ static struct genl_ops nl80211_ops[] = {
.internal_flags = NL80211_FLAG_NEED_NETDEV |
NL80211_FLAG_NEED_RTNL,
},
+ {
+ .cmd = NL80211_CMD_RADAR_DETECT,
+ .doit = nl80211_start_radar_detection,
+ .policy = nl80211_policy,
+ .flags = GENL_ADMIN_PERM,
+ .internal_flags = NL80211_FLAG_NEED_NETDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+ },
};
static struct genl_multicast_group nl80211_mlme_mcgrp = {
@@ -9002,6 +9055,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 2acba84..28b9a34 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -108,6 +108,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.10.4
On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
> IBSS should also consider interface combinations.
>
> Signed-off-by: Simon Wunderlich <[email protected]>
> ---
> net/wireless/nl80211.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 9bd8340..62e98f5 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -5463,6 +5463,7 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
> {
> struct cfg80211_registered_device *rdev = info->user_ptr[0];
> struct net_device *dev = info->user_ptr[1];
> + struct wireless_dev *wdev = dev->ieee80211_ptr;
> struct cfg80211_ibss_params ibss;
> struct wiphy *wiphy;
> struct cfg80211_cached_keys *connkeys = NULL;
> @@ -5524,6 +5525,15 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
> ibss.channel_fixed = !!info->attrs[NL80211_ATTR_FREQ_FIXED];
> ibss.privacy = !!info->attrs[NL80211_ATTR_PRIVACY];
>
> + mutex_lock(&rdev->devlist_mtx);
> + err = cfg80211_can_use_chan(rdev, wdev, ibss.chandef.chan,
> + ibss.channel_fixed ? CHAN_MODE_SHARED :
> + CHAN_MODE_EXCLUSIVE);
> + mutex_unlock(&rdev->devlist_mtx);
> +
> + if (err)
> + return -EINVAL;
> +
This is already in __cfg80211_join_ibss()
johannes
On Thu, 2013-01-17 at 14:40 +0100, Simon Wunderlich wrote:
> Actually there is no limit how long a channel is considered "available", at
> least in ETSI. ETSI EN 301-893 v1.4.1 had a limit of 24 hours for that,
> but that was removed in v1.5.1 and didn't re-appear since then (current is
> v1.7.1).
Huh indeed, I would have expected that to be there. It does have a
non-occupancy time though (30 minutes), maybe we should implement that?
I'm also thinking with the next regdb format update we should allow
specifying these timeouts etc. there.
Does anyone have the relevant FCC rules? I can't find anything with
google ...
> But we can move the CAC/timeout in the wdev and have keep a flag field in
> the channel struct instead, marking the channel as available, unavailable, etc.
>
> What do you think?
I think that would make sense. Probably available/unavailable and
"non-occupancy until"?
> > I have a feeling this change should take into account the channel width,
> > and whether CAC completed successfully?
> >
>
> All channels used for operation are checked already in cfg80211_chandef_usable()
> for the flags.
>
> If the channel width is supported at all is checked with cfg80211_can_use_iftype_chan()
> before/after.
>
> So I don't see the neccesity for further checking width, or am I missing something?
Hmm, cfg80211_reg_can_beacon() is exported and usable by other modules,
so it should probably do more checking?
> > > + err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, &chandef);
> > > + if (!err) {
> > > + wdev->preset_chandef = chandef;
> > > + chandef.chan->cac_started = true;
> > > + chandef.chan->radar_detect_timeout = jiffies +
> > > + msecs_to_jiffies(NL80211_DFS_MIN_CAC_TIME_MS);
> > > + }
> > > +
> > > + return err;
> > > +}
> >
> > This still seems somewhat wrong. For the duration of the CAC, the
> > channel should be "locked" in some way, no? As it stands now, nothing
> > prevents userspace from adding another vif and using it for something
> > entirely different, while cfg80211 thinks the CAC is actually running.
> >
>
> Hmm, we can put the "CAC state" in the wdev then, and use it for locking?
I think it may need to be the CAC chandef? Not sure, but we definitely
need something to use in cfg80211_get_chan_state().
johannes
On Wed, Jan 16, 2013 at 11:59:31PM +0100, Johannes Berg wrote:
> On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
>
> > +static int ieee80211_start_radar_detection(struct wiphy *wiphy,
> > + struct net_device *dev,
> > + struct cfg80211_chan_def *chandef)
> > +{
> > + struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> > + struct ieee80211_local *local = sdata->local;
> > + int res;
> > +
> > + if (!local->ops->start_radar_detection)
> > + return -EOPNOTSUPP;
> > +
> > + /* whatever, but channel contexts should not complain about that one */
> > + sdata->smps_mode = IEEE80211_SMPS_OFF;
> > + sdata->needed_rx_chains = local->rx_chains;
> > +
> > + if (ieee80211_vif_use_channel(sdata, chandef,
> > + IEEE80211_CHANCTX_SHARED))
> > + return -EBUSY;
>
> Should probably return whatever error vif_use_channel() returned?
>
OK
> > + res = drv_start_radar_detection(local, sdata, chandef);
>
> If the vif is assigned the channel, why also pass it to the
> start_radar_detection command? That seems pointless, they can't be
> different?
>
In the initial phase, __nl80211_set_channel() should not be able to set
the channel (no CAC yet), and will fail for IBSS (to be implemented later)
generally, at least as it's implemented now.
But what we can do is set the channel from mac80211 and call the driver
function without the channel argument, as it'll be pointless in this case.
Is this what you mean?
> > +++ b/net/mac80211/iface.c
> > @@ -826,6 +826,10 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
> > u.vlan.list)
> > dev_close(vlan->dev);
> > WARN_ON(!list_empty(&sdata->u.ap.vlans));
> > +
> > + /* reset DFS channel availability check */
> > + if (local->_oper_channel)
> > + local->_oper_channel->cac_started = false;
>
> That doesn't really belong here, does it? Seems like it should be in
> cfg80211. Actually, should it be there at all?
Actually, no, you are right. After stopping an AP and no radar is detected,
the channel should go from "operational" back to "available" state. Quoting
from ETSI EN 3001-893 v1.7.1
4.7.1.3/Master Devices/
c) Once the RLAN has started operations on an Available Channel, then that channel becomes an Operating Channel. During normal operation, the master device shall monitor all Operating Channels (In-Service Monitoring) to ensure that there is no radar operating within these channel(s). If no radar was detected on an Operating Channel but the RLAN stops operating on that channel, then the channel becomes an Available Channel.
I'll change that.
>
> If we allow using the channel for some amount of time after doing the
> CAC, even if we haven't been on the channel continuously, then we should
> also allow using the channel after stopping the AP for that much time.
>
> IOW -- deactivating should cause the timeout to start running.
As stated in a mail earlier from this patchset, a timeout ist not neccessary.
I'd skip this patch entirely.
>
> Actually that raises another question: If we have "external" radar
> detection, say by a different NIC, then shouldn't we still ask the
> driver to start radar detection when using the channel? Or is that
> implicit, does the driver have to check?
That is a good question, I didn't consider that. In the "simple process"
we first start radar detection and start the ap on the same channel.
For external CAC that won't work, of course. What about mac80211 checking
the channel in start_ap(), and if the channel requires DFS, pass some flag
to the driver that radar detection should be enabled?
>
> It seems the driver API needs a lot more documentation :-)
Agreed, but I guess we still need to discuss some corner cases to get the
neccesary input for documentation. ;)
>
>
> > +++ b/net/mac80211/trace.h
> > @@ -45,6 +45,35 @@
> > __entry->center_freq1, __entry->center_freq2, \
> > __entry->rx_chains_static, __entry->rx_chains_dynamic
> >
> > +#define CHAN_DEF_ENTRY __field(enum ieee80211_band, band) \
>
> I'll apply my "mac80211: split out chandef tracing macros" patch
> instead.
OK.
>
>
> Also: once the CAC is completed, shouldn't the channel context be freed
> up in mac80211, or something like that?
Currently we don't handle the event "CAC finished" explicitly - we only have
the cac_started/timeout fields. More on that later ...
Cheers,
Simon
Scanning and remain on channel functionality must be disabled while
doing radar detection/scanning, and vice versa.
Signed-off-by: Simon Wunderlich <[email protected]>
---
net/mac80211/cfg.c | 10 +++++++++-
net/mac80211/ieee80211_i.h | 3 +++
net/mac80211/scan.c | 3 +++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index bd300e4..2a5b7a4 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1032,6 +1032,7 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev)
skb_queue_purge(&sdata->u.ap.ps.bc_buf);
ieee80211_vif_release_channel(sdata);
+ local->radar_detect_enabled = false;
return 0;
}
@@ -2338,7 +2339,9 @@ static int ieee80211_start_roc_work(struct ieee80211_local *local,
INIT_LIST_HEAD(&roc->dependents);
/* if there's one pending or we're scanning, queue this one */
- if (!list_empty(&local->roc_list) || local->scanning)
+ if (!list_empty(&local->roc_list) ||
+ local->scanning ||
+ local->radar_detect_enabled)
goto out_check_combine;
/* if not HW assist, just queue & schedule work */
@@ -2599,6 +2602,9 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy,
if (!local->ops->start_radar_detection)
return -EOPNOTSUPP;
+ if (!list_empty(&local->roc_list) || local->scanning)
+ return -EBUSY;
+
/* whatever, but channel contexts should not complain about that one */
sdata->smps_mode = IEEE80211_SMPS_OFF;
sdata->needed_rx_chains = local->rx_chains;
@@ -2610,6 +2616,8 @@ static int ieee80211_start_radar_detection(struct wiphy *wiphy,
res = drv_start_radar_detection(local, sdata, chandef);
if (res)
ieee80211_vif_release_channel(sdata);
+ else
+ local->radar_detect_enabled = true;
return res;
}
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 0fa44a9..ff1c001 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -974,6 +974,9 @@ struct ieee80211_local {
/* wowlan is enabled -- don't reconfig on resume */
bool wowlan;
+ /* DFS/radar detection is enabled */
+ bool radar_detect_enabled;
+
/* number of RX chains the hardware has */
u8 rx_chains;
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 06cbe26..ceaeb32 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -356,6 +356,9 @@ static int ieee80211_start_sw_scan(struct ieee80211_local *local)
static bool ieee80211_can_scan(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata)
{
+ if (local->radar_detect_enabled)
+ return false;
+
if (!list_empty(&local->roc_list))
return false;
--
1.7.10.4
On Wed, Jan 16, 2013 at 11:51:12PM +0100, Johannes Berg wrote:
> On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich 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_started: indicates that channel availability check is started for this
> > + * channel type.
>
> So you're enforcing a certain CAC time, but not the time we are allowed
> to treat the channel as clear? Shouldn't *that* be in each channel
> struct, rather than the other stuff?
>
> It also seems to me that "cac_started" isn't really all that relevant in
> the channel struct either. What seems relevant is the *result* of the
> CAC, and how long it's still valid, no?
>
Actually there is no limit how long a channel is considered "available", at
least in ETSI. ETSI EN 301-893 v1.4.1 had a limit of 24 hours for that,
but that was removed in v1.5.1 and didn't re-appear since then (current is
v1.7.1).
But we can move the CAC/timeout in the wdev and have keep a flag field in
the channel struct instead, marking the channel as available, unavailable, etc.
What do you think?
> > +++ b/net/wireless/chan.c
> > @@ -287,14 +287,18 @@ bool cfg80211_reg_can_beacon(struct wiphy *wiphy,
> > struct cfg80211_chan_def *chandef)
> > {
> > bool res;
> > + u32 prohibited_flags;
> >
> > trace_cfg80211_reg_can_beacon(wiphy, chandef);
> >
> > - res = cfg80211_chandef_usable(wiphy, chandef,
> > - IEEE80211_CHAN_DISABLED |
> > - IEEE80211_CHAN_PASSIVE_SCAN |
> > - IEEE80211_CHAN_NO_IBSS |
> > - IEEE80211_CHAN_RADAR);
> > + prohibited_flags = IEEE80211_CHAN_DISABLED;
> > +
> > + if (!(wiphy->features & NL80211_FEATURE_DFS))
> > + prohibited_flags |= IEEE80211_CHAN_PASSIVE_SCAN |
> > + IEEE80211_CHAN_NO_IBSS |
> > + IEEE80211_CHAN_RADAR;
>
> I have a feeling this change should take into account the channel width,
> and whether CAC completed successfully?
>
All channels used for operation are checked already in cfg80211_chandef_usable()
for the flags.
If the channel width is supported at all is checked with cfg80211_can_use_iftype_chan()
before/after.
So I don't see the neccesity for further checking width, or am I missing something?
The CAC completed check is performed outside right now, but when we introduce
the available/unavailable flags as suggested above we can as well move this check
into cfg80211_reg_can_beacon().
> > +static int nl80211_start_radar_detection(struct sk_buff *skb,
> > + struct genl_info *info)
> > +{
> > + struct cfg80211_registered_device *rdev = info->user_ptr[0];
> > + struct net_device *dev = info->user_ptr[1];
> > + struct wireless_dev *wdev = dev->ieee80211_ptr;
> > + struct cfg80211_chan_def chandef;
> > + int err;
> > +
> > + if (!(rdev->wiphy.features & NL80211_FEATURE_DFS))
> > + return -EOPNOTSUPP;
> > +
> > + err = nl80211_parse_chandef(rdev, info, &chandef);
> > + if (err)
> > + return err;
> > +
> > + if (!(chandef.chan->flags & IEEE80211_CHAN_RADAR))
> > + return -EINVAL;
> > +
> > + if (chandef.chan->cac_started)
> > + return -EBUSY;
> > +
> > + if (!rdev->ops->start_radar_detection)
> > + return -EOPNOTSUPP;
> > +
> > + mutex_lock(&rdev->devlist_mtx);
> > + err = cfg80211_can_use_iftype_chan(rdev, wdev, wdev->iftype,
> > + chandef.chan, CHAN_MODE_SHARED,
> > + BIT(chandef.width));
> > + mutex_unlock(&rdev->devlist_mtx);
> > +
> > + if (err)
> > + return err;
> > +
> > + err = rdev->ops->start_radar_detection(&rdev->wiphy, dev, &chandef);
> > + if (!err) {
> > + wdev->preset_chandef = chandef;
> > + chandef.chan->cac_started = true;
> > + chandef.chan->radar_detect_timeout = jiffies +
> > + msecs_to_jiffies(NL80211_DFS_MIN_CAC_TIME_MS);
> > + }
> > +
> > + return err;
> > +}
>
> This still seems somewhat wrong. For the duration of the CAC, the
> channel should be "locked" in some way, no? As it stands now, nothing
> prevents userspace from adding another vif and using it for something
> entirely different, while cfg80211 thinks the CAC is actually running.
>
Hmm, we can put the "CAC state" in the wdev then, and use it for locking?
> > + 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;
>
> That should be the entire chandef info, and possibly the WDEV_ID too.
OK
On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
> +static int ieee80211_start_radar_detection(struct wiphy *wiphy,
> + struct net_device *dev,
> + struct cfg80211_chan_def *chandef)
> +{
> + struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
> + struct ieee80211_local *local = sdata->local;
> + int res;
> +
> + if (!local->ops->start_radar_detection)
> + return -EOPNOTSUPP;
> +
> + /* whatever, but channel contexts should not complain about that one */
> + sdata->smps_mode = IEEE80211_SMPS_OFF;
> + sdata->needed_rx_chains = local->rx_chains;
> +
> + if (ieee80211_vif_use_channel(sdata, chandef,
> + IEEE80211_CHANCTX_SHARED))
> + return -EBUSY;
Should probably return whatever error vif_use_channel() returned?
> + res = drv_start_radar_detection(local, sdata, chandef);
If the vif is assigned the channel, why also pass it to the
start_radar_detection command? That seems pointless, they can't be
different?
> +++ b/net/mac80211/iface.c
> @@ -826,6 +826,10 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
> u.vlan.list)
> dev_close(vlan->dev);
> WARN_ON(!list_empty(&sdata->u.ap.vlans));
> +
> + /* reset DFS channel availability check */
> + if (local->_oper_channel)
> + local->_oper_channel->cac_started = false;
That doesn't really belong here, does it? Seems like it should be in
cfg80211. Actually, should it be there at all?
If we allow using the channel for some amount of time after doing the
CAC, even if we haven't been on the channel continuously, then we should
also allow using the channel after stopping the AP for that much time.
IOW -- deactivating should cause the timeout to start running.
Actually that raises another question: If we have "external" radar
detection, say by a different NIC, then shouldn't we still ask the
driver to start radar detection when using the channel? Or is that
implicit, does the driver have to check?
It seems the driver API needs a lot more documentation :-)
> +++ b/net/mac80211/trace.h
> @@ -45,6 +45,35 @@
> __entry->center_freq1, __entry->center_freq2, \
> __entry->rx_chains_static, __entry->rx_chains_dynamic
>
> +#define CHAN_DEF_ENTRY __field(enum ieee80211_band, band) \
I'll apply my "mac80211: split out chandef tracing macros" patch
instead.
Also: once the CAC is completed, shouldn't the channel context be freed
up in mac80211, or something like that?
johannes
On Mon, Jan 21, 2013 at 11:46:20AM +0100, Zefir Kurtisi wrote:
> On 01/18/2013 11:08 PM, Johannes Berg wrote:
> > Hi,
> >
> >> As stated before: "available until ..." time is not neccessary as far
> >> as I know. Also the channel must stay available when AP stops operation,
> >> I'll change that.
> >
> > Yeah, I checked the documents myself now ... strange, but hey. Radars
> > are fixed installations and mostly active continually, so I guess they
> > figured it was no big deal.
> >
> We were also unsure if this makes sense and contacted regulatory for
> clarification. Indeed, with the current document a scenario where "I switched to a
> DFS channel and skipped CAC because I passed one half a year ago" is fully
> compliant. Practically, when you set up a new AP and do once a CAC for each DFS
> channel during installation time, you don't need to do a CAC ever again (as long
> as you do not see radars).
>
Thanks for clarification. :)
> Why then bother with CAC at all, one might think. In fact, exploiting given
> requirements to such extent is mandatory to make operation on DFS channels usable
> (which btw. is not the scope of the patches discussed here).
>
I would actually like to make operation on DFS channels usable by using the implementation
from these patches. :) Do you imply that we couldn't practically use it? Any suggestions
what can be improved?
Cheers,
Simon
On Fri, Jan 18, 2013 at 11:00:11PM +0100, Johannes Berg wrote:
> On Thu, 2013-01-17 at 14:52 +0100, Simon Wunderlich wrote:
>
> > > > + res = drv_start_radar_detection(local, sdata, chandef);
> > >
> > > If the vif is assigned the channel, why also pass it to the
> > > start_radar_detection command? That seems pointless, they can't be
> > > different?
> > >
> > In the initial phase, __nl80211_set_channel() should not be able to set
> > the channel (no CAC yet), and will fail for IBSS (to be implemented later)
> > generally, at least as it's implemented now.
> >
> > But what we can do is set the channel from mac80211 and call the driver
> > function without the channel argument, as it'll be pointless in this case.
> > Is this what you mean?
>
> Hmm. Why are you talking about __nl80211_set_channel() now? We're now
> talking about mac80211, which can and does set the channel (chanctx,
> which may fall back to drv_config()) before calling
> drv_start_radar_detection(), so I think the latter needs no chandef
> argument?
>
Sorry, I was confused at the point, you are right nl80211_set_channel() has
nothing to do with that. ieee80211_vif_use_channel() already sets the channel
implicitly (e.g. through ieee80211_hw_config()).
> > > Actually that raises another question: If we have "external" radar
> > > detection, say by a different NIC, then shouldn't we still ask the
> > > driver to start radar detection when using the channel? Or is that
> > > implicit, does the driver have to check?
> >
> > That is a good question, I didn't consider that. In the "simple process"
> > we first start radar detection and start the ap on the same channel.
> > For external CAC that won't work, of course. What about mac80211 checking
> > the channel in start_ap(), and if the channel requires DFS, pass some flag
> > to the driver that radar detection should be enabled?
>
> Yes, that makes sense. But then it would also make sense to remove the
> start_radar_detection() callback entirely, and encode all that
> information in the channel context/drv_config call?
>
> If mac80211 gets to be responsible for it, this should totally be
> documented in the cfg80211 API though so if a full-mac driver wants to
> implement it they know what to do. I do think this is the reasonable way
> of doing it though.
>
> Note that I'm not advocating removing the start_radar_detection() or its
> chandef from the *cfg80211* API. That is clearly needed. But in mac80211
> it seems "set this chandef with radar detection" is a better API?
>
OK, so we keep the cfg80211 start_radar_detection() and replace the mac80211
part with respective information that radar is required. That could work,
I'll look into this.
Thanks,
Simon
On Wed, 2013-01-16 at 23:42 +0100, Johannes Berg wrote:
> On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
> > To ease further DFS development regarding interface combinations, use
> > the interface combinations structure to test for radar capabilities.
> > Drivers can specify which channel widths they support, and in which
> > modes. Drivers should first allow AP mode only, but can later allow
> > MultiSSID APs, AP+Ad-Hoc, etc.
>
> Applied.
Btw, an iw patch would be nice.
johannes
IBSS should also consider interface combinations.
Signed-off-by: Simon Wunderlich <[email protected]>
---
net/wireless/nl80211.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 9bd8340..62e98f5 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5463,6 +5463,7 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
{
struct cfg80211_registered_device *rdev = info->user_ptr[0];
struct net_device *dev = info->user_ptr[1];
+ struct wireless_dev *wdev = dev->ieee80211_ptr;
struct cfg80211_ibss_params ibss;
struct wiphy *wiphy;
struct cfg80211_cached_keys *connkeys = NULL;
@@ -5524,6 +5525,15 @@ static int nl80211_join_ibss(struct sk_buff *skb, struct genl_info *info)
ibss.channel_fixed = !!info->attrs[NL80211_ATTR_FREQ_FIXED];
ibss.privacy = !!info->attrs[NL80211_ATTR_PRIVACY];
+ mutex_lock(&rdev->devlist_mtx);
+ err = cfg80211_can_use_chan(rdev, wdev, ibss.chandef.chan,
+ ibss.channel_fixed ? CHAN_MODE_SHARED :
+ CHAN_MODE_EXCLUSIVE);
+ mutex_unlock(&rdev->devlist_mtx);
+
+ if (err)
+ return -EINVAL;
+
if (info->attrs[NL80211_ATTR_BSS_BASIC_RATES]) {
u8 *rates =
nla_data(info->attrs[NL80211_ATTR_BSS_BASIC_RATES]);
--
1.7.10.4
Hi,
> As stated before: "available until ..." time is not neccessary as far
> as I know. Also the channel must stay available when AP stops operation,
> I'll change that.
Yeah, I checked the documents myself now ... strange, but hey. Radars
are fixed installations and mostly active continually, so I guess they
figured it was no big deal.
> > Actually with your current patch 2) cannot be implemented at all because
> > the initial CAC never really finishes?
>
> Yeah, for this case it is flawed - one could start radar detection on
> different channels one after another in short time, and we would just
> set "cac_started" and the timeout - which eventually might mark all channels
> available without checking for the required time on it. That seems wrong.
Indeed. But once you add the "locking", it will be that you can start
radar detection only on a single channel, and then it stays locked to
doing that until you start and stop an AP on that channel. That's also
undesirable.
> > * radar detection finished:
> > - driver: call mac80211 with result (radar detected or not)
> > - mac80211: free up channel context (probably needs workqueue!)
> > pass result to cfg80211
> > - cfg80211: clear wdev radar detect chandef
> > if OK to use: store availability in channel struct
> > send result to userspace
>
> Right now we only have the event for radar detect (i.e. detection
> failed).
>
> We could add a timer to mark the channel available after the CAC time,
> if no radar has been detected in this time. Don't know if mac80211 or
> cfg80211 would be the best place for this, but I think the driver is not
> - this timer must be shared over various drivers after all.
Fair point. It seems likely though that full-MAC chips, if they
implement this at all, would have the timer in firmware. Therefore,
providing API for "radar detected" and "channel is usable" would make
more sense? The timer would then go into mac80211.
Later, if we modify the regdb format/API, we could also pass the minimum
time from there. That way, if it ever gets changed and we want to rely
on regdb info, we could.
> > * start AP:
> > - cfg80211: check channel availability
> > - cfg80211/mac80211/driver? - enable radar detection while operating
>
> mac80211 can instruct the driver to use radar detection while operating.
> E.g. pass it in sdata->vis.bss_conf ?
I would say in the chanctx, i.e. struct ieee80211_chanctx_conf. Since
not all drivers implement that yet, also struct ieee80211_conf. See how
local->_oper_channel gets set in mac80211/chan.c. This information would
be similar, the "master" information would be in the chandef, copied to
local/local->hw.conf.
> > * AP channel switch:
> > - if due to radar, mark channel as not clear
> OK - in AP case, we would have got the radar event anyway so the channel
> would already be marked "not clear". In IBSS however, that is possible
> (another station may have asked us to switch channel because of radar).
Oh, I wasn't even considering that -- don't care what you do there,
although it makes sense. OTOH, we would detect the radar ourselves, so
it doesn't really matter?
> > Note how I'm also letting the drivers determine the duration of the
> > radar check. Maybe the duration should be passed to the driver, or maybe
> > not, but I think for full-MAC chips it would be more likely that they
> > get an event for both cases from the device, not just the timeout you
> > have in software ...
>
> Hm, I don't know. CAC times are subject to regulation changes. In ETSI
> it's one minute, for weather channels (5600-5650 MHz) 10 minutes (but we
> better don't support them anyway as they require incredible radar
> detection rates).
See above. I have a feeling that if full-MAC chips implement this they'd
enforce a minimum time in firmware.
johannes
On Wed, 2013-01-30 at 17:56 +0100, Simon Wunderlich wrote:
> On Wed, Jan 30, 2013 at 06:34:48PM +0200, Luciano Coelho wrote:
> > > + switch (iftype) {
> > > + case NL80211_IFTYPE_ADHOC:
> > > + case NL80211_IFTYPE_AP:
> > > + case NL80211_IFTYPE_AP_VLAN:
> > > + case NL80211_IFTYPE_MESH_POINT:
> > > + case NL80211_IFTYPE_P2P_GO:
> > > + radar_required = !!(chan->flags & IEEE80211_CHAN_RADAR);
> > > + break;
> >
> > This code is causing an oops with the wl18xx driver in AP mode. The
> > problem is that cfg80211_can_change_interface() calls
> > cfg80211_can_use_iftype_chan() with chan == NULL. This code doesn't
> > check if chan is NULL, so this dereference causes the oops.
>
> Sorry about that - I believe you've found the same bug I've posted a patch
> for a few days ago. Johannes already has this in mac80211-next, but it is
> not yet in wireless-testing:
>
> http://article.gmane.org/gmane.linux.kernel.wireless.general/102836/match=simon
> http://git.kernel.org/?p=linux/kernel/git/jberg/mac80211-next.git;a=commit;h=683d41ae6755e6ae297ec09603c229795ab9566e
Ah, thanks. I should have read the list more carefully. :)
> > I don't have the time right now to fix this, but I'll look into it
> > tomorrow (unless someone comes with a fix before that :P).
>
> Please have a look at the patch posted above (both links for the same patch).
I'll apply that and try it out. Thanks for pointing it out so promptly!
--
Cheers,
Luca.
On Thu, Jan 17, 2013 at 12:00:27AM +0100, Johannes Berg wrote:
> On Tue, 2013-01-08 at 14:04 +0100, Simon Wunderlich wrote:
> > Scanning and remain on channel functionality must be disabled while
> > doing radar detection/scanning, and vice versa.
>
> This really belongs into the previous patch, I think?
OK, I can squash it.
Cheers,
Simon