Re: [PATCH] New functionality for aborting ongoing CAC.

2018-10-01 Thread Enrique Giraldo
You are right, it is possible to do it as you comment
(NL80211_CMD_STOP_AP and then NL80211_CMD_START_AP). It is therefore
not necessary a specific command to abort the CAC.
Thank you very much for the clarification and the time spent.

PS: If you use OpenWRT you have to control the STOP and the START, the
command "wifi" does not work properly.
El mié., 26 sept. 2018 a las 11:26, Johannes Berg
() escribió:
>
> On Tue, 2018-09-25 at 10:19 +0200, Enrique Giraldo wrote:
> > Add NL80211_CMD_ABORT_CAC to the nl80211 interface.
>
> As Arend pointed out, this really needs a much better commit message.
>
> Please also adjust the subject to have a proper prefix etc. See
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
> for some help with that.
>
> > @@ -3227,6 +3228,7 @@ struct cfg80211_ops {
> >   int (*scan)(struct wiphy *wiphy,
> >   struct cfg80211_scan_request *request);
> >   void(*abort_scan)(struct wiphy *wiphy, struct wireless_dev *wdev);
> > + void(*abort_cac)(struct wiphy *wiphy, struct wireless_dev *wdev);
>
> This is fine, you can add in the middle of the struct.
>
> >
> > @@ -1220,6 +1223,7 @@ enum nl80211_commands {
> >   NL80211_CMD_WIPHY_REG_CHANGE,
> >
> >   NL80211_CMD_ABORT_SCAN,
> > + NL80211_CMD_ABORT_CAC,
> >
> >   NL80211_CMD_START_NAN,
>
> This is *not* OK, as the comments here indicate, we must never add
> anything in the middle of this enum, doing so breaks userspace ABI.
>
> johannes


Re: [PATCH] New functionality for aborting ongoing CAC.

2018-09-26 Thread Johannes Berg
On Tue, 2018-09-25 at 10:19 +0200, Enrique Giraldo wrote:
> Add NL80211_CMD_ABORT_CAC to the nl80211 interface.

As Arend pointed out, this really needs a much better commit message.

Please also adjust the subject to have a proper prefix etc. See
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
for some help with that.

> @@ -3227,6 +3228,7 @@ struct cfg80211_ops {
>   int (*scan)(struct wiphy *wiphy,
>   struct cfg80211_scan_request *request);
>   void(*abort_scan)(struct wiphy *wiphy, struct wireless_dev *wdev);
> + void(*abort_cac)(struct wiphy *wiphy, struct wireless_dev *wdev);

This is fine, you can add in the middle of the struct.

> 
> @@ -1220,6 +1223,7 @@ enum nl80211_commands {
>   NL80211_CMD_WIPHY_REG_CHANGE,
>  
>   NL80211_CMD_ABORT_SCAN,
> + NL80211_CMD_ABORT_CAC,
>  
>   NL80211_CMD_START_NAN,

This is *not* OK, as the comments here indicate, we must never add
anything in the middle of this enum, doing so breaks userspace ABI.

johannes


Re: [PATCH] New functionality for aborting ongoing CAC.

2018-09-25 Thread Enrique Giraldo
What you commented was the first thing that I tried, but doing it, the
radio stays in an inconsistent state and the hostapd is not able to
raise an instance again.
Attached is the log of what is happening:

Tue Sep 25 09:58:10 2018 daemon.notice hostapd: wlan0: DFS-CAC-START
freq=5500 chan=100 sec_chan=1, width=1, seg0=106, seg1=0, cac_time=60s
Tue Sep 25 09:58:29 2018 daemon.notice hostapd: wlan0:
DFS-CAC-COMPLETED freq=5500 success=0 ht_enabled=0 chan_offset=0
chan_width=3 cf1=5530 cf2=0
Tue Sep 25 09:58:29 2018 daemon.notice hostapd: wlan0: INTERFACE-DISABLED
Tue Sep 25 09:58:29 2018 kern.info kernel: [   70.650485] br-lan: port
3(wlan0) entered disabled state
Tue Sep 25 09:58:29 2018 kern.info kernel: [   70.665228] device wlan0
left promiscuous mode
Tue Sep 25 09:58:29 2018 kern.info kernel: [   70.669794] br-lan: port
3(wlan0) entered disabled state
Tue Sep 25 09:58:30 2018 daemon.err hostapd: Configuration file:
/var/run/hostapd-phy0.conf
Tue Sep 25 09:58:31 2018 kern.warn kernel: [   72.295668] ath10k_pci
:00:00.0: 10.1 wmi init: vdevs: 16  peers: 127  tid: 256
Tue Sep 25 09:58:31 2018 kern.info kernel: [   72.312713] ath10k_pci
:00:00.0: wmi print 'P 128 V 8 T 410'
Tue Sep 25 09:58:31 2018 kern.info kernel: [   72.318911] ath10k_pci
:00:00.0: wmi print 'msdu-desc: 912  sw-crypt: 0'
Tue Sep 25 09:58:31 2018 kern.info kernel: [   72.326063] ath10k_pci
:00:00.0: wmi print 'alloc rem: 38688 iram: 27140'
Tue Sep 25 09:58:31 2018 daemon.notice hostapd: wlan0: INTERFACE-ENABLED
Tue Sep 25 09:58:31 2018 kern.info kernel: [   72.402994] IPv6:
ADDRCONF(NETDEV_UP): wlan0: link is not ready
Tue Sep 25 09:58:31 2018 daemon.notice hostapd: wlan0: INTERFACE-DISABLED
Tue Sep 25 09:58:31 2018 daemon.err hostapd: nl80211: Could not
configure driver mode
Tue Sep 25 09:58:31 2018 daemon.notice hostapd: nl80211: deinit
ifname=wlan0 disabled_11b_rates=0
Tue Sep 25 09:58:31 2018 daemon.err hostapd: nl80211 driver
initialization failed.
Tue Sep 25 09:58:31 2018 daemon.notice hostapd: wlan0: interface state
UNINITIALIZED->DISABLED
Tue Sep 25 09:58:31 2018 daemon.notice hostapd: wlan0: AP-DISABLED
Tue Sep 25 09:58:31 2018 daemon.notice hostapd: wlan0:
CTRL-EVENT-TERMINATING
Tue Sep 25 09:58:31 2018 daemon.err hostapd: hostapd_free_hapd_data:
Interface wlan0 wasn't started
Tue Sep 25 09:58:31 2018 daemon.notice netifd: radio0 (1809): WARNING
(wireless_add_process): executable path /usr/sbin/hostapd does not
match process 1640 path ()
Tue Sep 25 09:58:31 2018 daemon.notice netifd: radio0 (1809): Device
setup failed: HOSTAPD_START_FAILED

Regards,
EG

El mar., 25 sept. 2018 a las 11:53, Arend van Spriel
() escribió:
>
> On 9/25/2018 11:28 AM, Enrique Giraldo wrote:
> > The main reason is to be able to stop the CAC when you want to make a
> > channel switch and the CAC is ongoing. It's true that the radio would
> > not pass to the next phase, the behavior is the same as when during
> > the CAC a radar event is detected. In the case of aborting, a later
> > action is expected, for example, switch to the desired new channel.
>
> So for mac80211 the function ieee80211_dfs_cancel_cac() indeed seems to
> just abort. So what is the scenario here? If user-space sends
> NL80211_CMD_START_AP with DFS channel, a CAC period is started. So you
> are saying you want to do a channel switch? Does it not make sense to
> issue a NL80211_CMD_STOP_AP and then NL80211_CMD_START_AP for the
> desired channel? Maybe the use-case you are looking at is not AP mode,
> but there is no context whatsoever in the commit message so I can only
> guess.
>
> Regards,
> Arend
>


Re: [PATCH] New functionality for aborting ongoing CAC.

2018-09-25 Thread Arend van Spriel

On 9/25/2018 11:28 AM, Enrique Giraldo wrote:

The main reason is to be able to stop the CAC when you want to make a
channel switch and the CAC is ongoing. It's true that the radio would
not pass to the next phase, the behavior is the same as when during
the CAC a radar event is detected. In the case of aborting, a later
action is expected, for example, switch to the desired new channel.


So for mac80211 the function ieee80211_dfs_cancel_cac() indeed seems to 
just abort. So what is the scenario here? If user-space sends 
NL80211_CMD_START_AP with DFS channel, a CAC period is started. So you 
are saying you want to do a channel switch? Does it not make sense to 
issue a NL80211_CMD_STOP_AP and then NL80211_CMD_START_AP for the 
desired channel? Maybe the use-case you are looking at is not AP mode, 
but there is no context whatsoever in the commit message so I can only 
guess.


Regards,
Arend



Re: [PATCH] New functionality for aborting ongoing CAC.

2018-09-25 Thread Enrique Giraldo
The main reason is to be able to stop the CAC when you want to make a
channel switch and the CAC is ongoing. It's true that the radio would
not pass to the next phase, the behavior is the same as when during
the CAC a radar event is detected. In the case of aborting, a later
action is expected, for example, switch to the desired new channel.

Regards,
EG
El mar., 25 sept. 2018 a las 10:52, Arend van Spriel
() escribió:
>
> On 9/25/2018 10:19 AM, Enrique Giraldo wrote:
> > Add NL80211_CMD_ABORT_CAC to the nl80211 interface.
>
> This one really needs a good motivation. The CAC duration is a hard
> requirement so aborting it means that the radio can not proceed to the
> next phase. You really need to describe your reasoning for wanting this
> in the commit message.
>
> Regards,
> Arend


Re: [PATCH] New functionality for aborting ongoing CAC.

2018-09-25 Thread Arend van Spriel

On 9/25/2018 10:19 AM, Enrique Giraldo wrote:

Add NL80211_CMD_ABORT_CAC to the nl80211 interface.


This one really needs a good motivation. The CAC duration is a hard 
requirement so aborting it means that the radio can not proceed to the 
next phase. You really need to describe your reasoning for wanting this 
in the commit message.


Regards,
Arend


[PATCH] New functionality for aborting ongoing CAC.

2018-09-25 Thread Enrique Giraldo
Add NL80211_CMD_ABORT_CAC to the nl80211 interface.

Signed-off-by: Enrique Giraldo 
---
 include/net/cfg80211.h   |  2 ++
 include/uapi/linux/nl80211.h |  4 
 net/mac80211/cfg.c   |  6 ++
 net/wireless/nl80211.c   | 23 +++
 net/wireless/rdev-ops.h  |  8 
 net/wireless/trace.h |  5 +
 6 files changed, 48 insertions(+)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 8ebabc9873d1..60b60fbdce32 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -2896,6 +2896,7 @@ struct cfg80211_external_auth_params {
  * the scan/scan_done bracket too.
  * @abort_scan: Tell the driver to abort an ongoing scan. The driver shall
  * indicate the status of the scan through cfg80211_scan_done().
+ * @abort_cac: Tell the driver to abort an ongoing cac.
  *
  * @auth: Request to authenticate with the specified peer
  * (invoked with the wireless_dev mutex held)
@@ -3227,6 +3228,7 @@ struct cfg80211_ops {
int (*scan)(struct wiphy *wiphy,
struct cfg80211_scan_request *request);
void(*abort_scan)(struct wiphy *wiphy, struct wireless_dev *wdev);
+   void(*abort_cac)(struct wiphy *wiphy, struct wireless_dev *wdev);
 
int (*auth)(struct wiphy *wiphy, struct net_device *dev,
struct cfg80211_auth_request *req);
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 7acc16f34942..4ddcb93f370b 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -932,6 +932,9 @@
  * not running. The driver indicates the status of the scan through
  * cfg80211_scan_done().
  *
+ * @NL80211_CMD_ABORT_CAC: Stop an ongoing CAC. Returns -ENOENT if a CAC
+ * process is not running.
+ *
  * @NL80211_CMD_START_NAN: Start NAN operation, identified by its
  * %NL80211_ATTR_WDEV interface. This interface must have been
  * previously created with %NL80211_CMD_NEW_INTERFACE. After it
@@ -1220,6 +1223,7 @@ enum nl80211_commands {
NL80211_CMD_WIPHY_REG_CHANGE,
 
NL80211_CMD_ABORT_SCAN,
+   NL80211_CMD_ABORT_CAC,
 
NL80211_CMD_START_NAN,
NL80211_CMD_STOP_NAN,
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index d25da0e66da1..2036c91a6e51 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2248,6 +2248,11 @@ static void ieee80211_abort_scan(struct wiphy *wiphy, 
struct wireless_dev *wdev)
ieee80211_scan_cancel(wiphy_priv(wiphy));
 }
 
+static void ieee80211_abort_cac(struct wiphy *wiphy, struct wireless_dev *wdev)
+{
+   ieee80211_dfs_cac_cancel(wiphy_priv(wiphy));
+}
+
 static int
 ieee80211_sched_scan_start(struct wiphy *wiphy,
   struct net_device *dev,
@@ -3850,6 +3855,7 @@ const struct cfg80211_ops mac80211_config_ops = {
.resume = ieee80211_resume,
.scan = ieee80211_scan,
.abort_scan = ieee80211_abort_scan,
+   .abort_cac = ieee80211_abort_cac,
.sched_scan_start = ieee80211_sched_scan_start,
.sched_scan_stop = ieee80211_sched_scan_stop,
.auth = ieee80211_auth,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 4b8ec659e797..ee80904e098a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -7306,6 +7306,21 @@ static int nl80211_abort_scan(struct sk_buff *skb, 
struct genl_info *info)
return 0;
 }
 
+static int nl80211_abort_cac(struct sk_buff *skb, struct genl_info *info)
+{
+   struct cfg80211_registered_device *rdev = info->user_ptr[0];
+   struct wireless_dev *wdev = info->user_ptr[1];
+
+   if (!rdev->ops->abort_cac)
+   return -EOPNOTSUPP;
+
+   if (!wdev->cac_started)
+   return -ENOENT;
+
+   rdev_abort_cac(rdev, wdev);
+   return 0;
+}
+
 static int
 nl80211_parse_sched_scan_plans(struct wiphy *wiphy, int n_plans,
   struct cfg80211_sched_scan_request *request,
@@ -13344,6 +13359,14 @@ static const struct genl_ops nl80211_ops[] = {
.internal_flags = NL80211_FLAG_NEED_WDEV_UP |
  NL80211_FLAG_NEED_RTNL,
},
+   {
+   .cmd = NL80211_CMD_ABORT_CAC,
+   .doit = nl80211_abort_cac,
+   .policy = nl80211_policy,
+   .flags = GENL_UNS_ADMIN_PERM,
+   .internal_flags = NL80211_FLAG_NEED_WDEV_UP |
+ NL80211_FLAG_NEED_RTNL,
+   },
{
.cmd = NL80211_CMD_GET_SCAN,
.policy = nl80211_policy,
diff --git a/net/wireless/rdev-ops.h b/net/wireless/rdev-ops.h
index 364f5d67f05b..edbb11251c03 100644
--- a/net/wireless/rdev-ops.h
+++ b/net/wireless/rdev-ops.h
@@ -435,6 +435,14 @@ static inline void rdev_abort_scan(struct 
cfg80211_registered_device *rdev,
trace_rdev_return_void(>wiphy);
 }
 
+static inline void rdev_abort_cac(struct cfg80211_registered_device