diff mbox series

[v2] DFS: restart CAC in case of CAC is aborted

Message ID 20180219154930.7011-1-dlebed@quantenna.com
State Changes Requested
Headers show
Series [v2] DFS: restart CAC in case of CAC is aborted | expand

Commit Message

Dmitrii Lebed Feb. 19, 2018, 3:49 p.m. UTC 
From: Dmitry Lebed <dlebed@quantenna.com>

If CAC is aborted, hostapd will continue wait for
CAC completion and will stuck in this state forever.
Adding CAC restart action on CAC aborted event,
considering "CAC aborted" state as recoverable.
CAC aborted can be generated in some complex configs,
e.g. in repeater config, when wpa_supplicant is
doing something on another virtual interface that
can lead to generation of "CAC aborted" event.

Signed-off-by: Dmitrii Lebed <dlebed@quantenna.com>
---
V1->V2 changes:
1. Move CAC restart to eloop with 1 sec delay
2. Added CAC start error handling in CAC restart handler
3. Added DFS offload handling for CAC abort

 src/ap/dfs.c           | 97 ++++++++++++++++++++++++++++++++++++--------------
 src/ap/dfs.h           |  2 +-
 src/ap/drv_callbacks.c |  6 ++--
 3 files changed, 74 insertions(+), 31 deletions(-)

--
2.16.1



This email, including its contents and any attachment(s), may contain confidential information of Quantenna Communications, Inc. and is solely for the intended recipient(s). If you may have received this in error, please contact the sender and permanently delete this email, its contents and any attachment(s).

Comments

Jouni Malinen Jan. 5, 2019, 11:26 p.m. UTC | #1 
On Mon, Feb 19, 2018 at 06:49:30PM +0300, Dmitrii Lebed wrote:
> If CAC is aborted, hostapd will continue wait for
> CAC completion and will stuck in this state forever.
> Adding CAC restart action on CAC aborted event,
> considering "CAC aborted" state as recoverable.
> CAC aborted can be generated in some complex configs,
> e.g. in repeater config, when wpa_supplicant is
> doing something on another virtual interface that
> can lead to generation of "CAC aborted" event.

Please note that nl80211 drivers will indicate CAC aborted if a radar is
detected during the CAC. It does not sound correct to try to restart CAC
in such cases since the channel can clearly not be usable.

> diff --git a/src/ap/dfs.c b/src/ap/dfs.c
> @@ -636,6 +637,34 @@ static unsigned int dfs_get_cac_time(struct hostapd_iface *iface,
>         return cac_time_ms;
>  }
> 
> +static int dfs_start_cac(struct hostapd_iface *iface)
> +{
> +       int res;

This is whitespace damaged (tabs seemed to be converted to spaces) which
makes it quite inconvenient to try to apply this.

> @@ -800,9 +831,21 @@ int hostapd_dfs_complete_cac(struct hostapd_iface *iface, int success, int freq,

> +       } else {

So this is the !success case which is also entered if a radar is
detected during CAC.

> +               if (dfs_offload) {
> +                       if (iface->state == HAPD_IFACE_ENABLED)
> +                               iface->cac_started = 0;
> +               } else if (iface->state == HAPD_IFACE_DFS) {
> +                       /* Schedule CAC restart in 1 second */
> +                       eloop_register_timeout(1, 0, hostapd_dfs_cac_restart,
> +                                              iface, NULL);

That eloop timeout was not canceled anywhere. There needs to be some
protection against the interface being removed during the wait since
otherwise there would be a callback coming in with a pointer to freed
memory.

Where does this one second time come from? Why not immediately? Or
should the extra time be used to wait to see if NL80211_CMD_RADAR_DETECT
is received and if so, cancel this restarting attempt of CAC?

> -int hostapd_dfs_start_cac(struct hostapd_iface *iface, int freq,
> +int hostapd_dfs_cac_started(struct hostapd_iface *iface, int freq,

That renaming broke wpa_supplicant build. I'd leave this renaming to a
separate patch (also covering the needed wpa_supplicant/ap.c change) or
maybe just drop the renaming completely to avoid unnecessary changes.
diff mbox series

Patch

diff --git a/src/ap/dfs.c b/src/ap/dfs.c
index 5a0d7814b..f1476eee0 100644
--- a/src/ap/dfs.c
+++ b/src/ap/dfs.c
@@ -10,6 +10,7 @@ 
 #include "utils/includes.h"

 #include "utils/common.h"
+#include "utils/eloop.h"
 #include "common/ieee802_11_defs.h"
 #include "common/hw_features_common.h"
 #include "common/wpa_ctrl.h"
@@ -636,6 +637,34 @@  static unsigned int dfs_get_cac_time(struct hostapd_iface *iface,
        return cac_time_ms;
 }

+static int dfs_start_cac(struct hostapd_iface *iface)
+{
+       int res;
+
+       wpa_msg(iface->bss[0]->msg_ctx, MSG_INFO, DFS_EVENT_CAC_START
+               "freq=%d chan=%d sec_chan=%d, width=%d, seg0=%d, seg1=%d, cac_time=%ds",
+               iface->freq,
+               iface->conf->channel, iface->conf->secondary_channel,
+               iface->conf->vht_oper_chwidth,
+               iface->conf->vht_oper_centr_freq_seg0_idx,
+               iface->conf->vht_oper_centr_freq_seg1_idx,
+               iface->dfs_cac_ms / 1000);
+
+       res = hostapd_start_dfs_cac(iface, iface->conf->hw_mode,
+                                   iface->freq,
+                                   iface->conf->channel,
+                                   iface->conf->ieee80211n,
+                                   iface->conf->ieee80211ac,
+                                   iface->conf->secondary_channel,
+                                   iface->conf->vht_oper_chwidth,
+                                   iface->conf->vht_oper_centr_freq_seg0_idx,
+                                   iface->conf->vht_oper_centr_freq_seg1_idx);
+
+       if (res)
+               wpa_printf(MSG_ERROR, "DFS start_dfs_cac() failed, %d", res);
+
+       return res;
+}

 /*
  * Main DFS handler
@@ -719,31 +748,8 @@  int hostapd_handle_dfs(struct hostapd_iface *iface)
        /* Finally start CAC */
        hostapd_set_state(iface, HAPD_IFACE_DFS);
        wpa_printf(MSG_DEBUG, "DFS start CAC on %d MHz", iface->freq);
-       wpa_msg(iface->bss[0]->msg_ctx, MSG_INFO, DFS_EVENT_CAC_START
-               "freq=%d chan=%d sec_chan=%d, width=%d, seg0=%d, seg1=%d, cac_time=%ds",
-               iface->freq,
-               iface->conf->channel, iface->conf->secondary_channel,
-               iface->conf->vht_oper_chwidth,
-               iface->conf->vht_oper_centr_freq_seg0_idx,
-               iface->conf->vht_oper_centr_freq_seg1_idx,
-               iface->dfs_cac_ms / 1000);

-       res = hostapd_start_dfs_cac(iface, iface->conf->hw_mode,
-                                   iface->freq,
-                                   iface->conf->channel,
-                                   iface->conf->ieee80211n,
-                                   iface->conf->ieee80211ac,
-                                   iface->conf->secondary_channel,
-                                   iface->conf->vht_oper_chwidth,
-                                   iface->conf->vht_oper_centr_freq_seg0_idx,
-                                   iface->conf->vht_oper_centr_freq_seg1_idx);
-
-       if (res) {
-               wpa_printf(MSG_ERROR, "DFS start_dfs_cac() failed, %d", res);
-               return -1;
-       }
-
-       return 0;
+       return dfs_start_cac(iface);
 }


@@ -763,18 +769,43 @@  static int hostapd_config_dfs_chan_available(struct hostapd_iface *iface)
        return dfs_check_chans_available(iface, start_chan_idx, n_chans);
 }

+static void hostapd_dfs_cac_restart(void *eloop_data, void *user_ctx)
+{
+       struct hostapd_iface *iface = eloop_data;
+       int res;
+
+       if (iface->state != HAPD_IFACE_DFS || !iface->cac_started) {
+               wpa_printf(MSG_WARNING, "CAC restart in wrong state=%d; started=%d",
+                          iface->state, iface->cac_started);
+               return;
+       }
+
+       wpa_printf(MSG_DEBUG, "DFS restart CAC on %d MHz", iface->freq);
+       res = dfs_start_cac(iface);
+
+       if (res)
+               hostapd_setup_interface_complete(iface, res);
+}

 int hostapd_dfs_complete_cac(struct hostapd_iface *iface, int success, int freq,
                             int ht_enabled, int chan_offset, int chan_width,
                             int cf1, int cf2)
 {
+       int res = 0;
+       int dfs_offload = !!(iface->drv_flags & WPA_DRIVER_FLAGS_DFS_OFFLOAD);
+
        wpa_msg(iface->bss[0]->msg_ctx, MSG_INFO, DFS_EVENT_CAC_COMPLETED
                "success=%d freq=%d ht_enabled=%d chan_offset=%d chan_width=%d cf1=%d cf2=%d",
                success, freq, ht_enabled, chan_offset, chan_width, cf1, cf2);

+       if (!iface->cac_started) {
+               wpa_printf(MSG_ERROR, "CAC complete received in wrong state");
+               return 1;
+       }
+
        if (success) {
                /* Complete iface/ap configuration */
-               if (iface->drv_flags & WPA_DRIVER_FLAGS_DFS_OFFLOAD) {
+               if (dfs_offload) {
                        /* Complete AP configuration for the first bring up. */
                        if (iface->state != HAPD_IFACE_ENABLED)
                                hostapd_setup_interface_complete(iface, 0);
@@ -800,9 +831,21 @@  int hostapd_dfs_complete_cac(struct hostapd_iface *iface, int success, int freq,
                                iface->cac_started = 0;
                        }
                }
+       } else {
+               if (dfs_offload) {
+                       if (iface->state == HAPD_IFACE_ENABLED)
+                               iface->cac_started = 0;
+               } else if (iface->state == HAPD_IFACE_DFS) {
+                       /* Schedule CAC restart in 1 second */
+                       eloop_register_timeout(1, 0, hostapd_dfs_cac_restart,
+                                              iface, NULL);
+               } else {
+                       wpa_printf(MSG_ERROR, "CAC aborted received in wrong state=%d offload=%d",
+                                  iface->state, dfs_offload);
+               }
        }

-       return 0;
+       return res;
 }


@@ -1074,7 +1117,7 @@  int hostapd_is_dfs_required(struct hostapd_iface *iface)
 }


-int hostapd_dfs_start_cac(struct hostapd_iface *iface, int freq,
+int hostapd_dfs_cac_started(struct hostapd_iface *iface, int freq,
                          int ht_enabled, int chan_offset, int chan_width,
                          int cf1, int cf2)
 {
diff --git a/src/ap/dfs.h b/src/ap/dfs.h
index f0fa6f688..2d1532815 100644
--- a/src/ap/dfs.h
+++ b/src/ap/dfs.h
@@ -25,7 +25,7 @@  int hostapd_dfs_nop_finished(struct hostapd_iface *iface, int freq,
                             int ht_enabled,
                             int chan_offset, int chan_width, int cf1, int cf2);
 int hostapd_is_dfs_required(struct hostapd_iface *iface);
-int hostapd_dfs_start_cac(struct hostapd_iface *iface, int freq,
+int hostapd_dfs_cac_started(struct hostapd_iface *iface, int freq,
                          int ht_enabled, int chan_offset, int chan_width,
                          int cf1, int cf2);
 int hostapd_handle_dfs_offload(struct hostapd_iface *iface);
diff --git a/src/ap/drv_callbacks.c b/src/ap/drv_callbacks.c
index e4a458107..58e967704 100644
--- a/src/ap/drv_callbacks.c
+++ b/src/ap/drv_callbacks.c
@@ -1398,9 +1398,9 @@  static void hostapd_event_dfs_cac_started(struct hostapd_data *hapd,
                                          struct dfs_event *radar)
 {
        wpa_printf(MSG_DEBUG, "DFS offload CAC started on %d MHz", radar->freq);
-       hostapd_dfs_start_cac(hapd->iface, radar->freq, radar->ht_enabled,
-                             radar->chan_offset, radar->chan_width,
-                             radar->cf1, radar->cf2);
+       hostapd_dfs_cac_started(hapd->iface, radar->freq, radar->ht_enabled,
+                               radar->chan_offset, radar->chan_width,
+                               radar->cf1, radar->cf2);
 }

 #endif /* NEED_AP_MLME */