diff mbox series

[1/2] hostapd: dfs: allow switch to available channel

Message ID 20201003120013.19160-1-janusz.dziedzic@gmail.com
State Changes Requested
Headers show
Series [1/2] hostapd: dfs: allow switch to available channel | expand

Commit Message

Janusz Dziedzic Oct. 3, 2020, noon UTC
For EU, where preCAC is allowed, we should
allow switch to DFS available channels, instead
of restart BSS.

Signed-off-by: Janusz Dziedzic <janusz.dziedzic@gmail.com>
---
 src/ap/dfs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Jouni Malinen Oct. 9, 2020, 8:34 a.m. UTC | #1
On Sat, Oct 03, 2020 at 02:00:12PM +0200, Janusz Dziedzic wrote:
> For EU, where preCAC is allowed, we should
> allow switch to DFS available channels, instead
> of restart BSS.

Could you please clarify how this is supposed to work in regulatory
domains that do not allow previously done CAC results to be used? The
test case added in 2/2 seems to pass even if running the AP with country
US or CA..
Janusz Dziedzic Oct. 9, 2020, 11:44 a.m. UTC | #2
pt., 9 paź 2020 o 10:37 Jouni Malinen <j@w1.fi> napisał(a):
>
> On Sat, Oct 03, 2020 at 02:00:12PM +0200, Janusz Dziedzic wrote:
> > For EU, where preCAC is allowed, we should
> > allow switch to DFS available channels, instead
> > of restart BSS.
>
> Could you please clarify how this is supposed to work in regulatory
> domains that do not allow previously done CAC results to be used? The
> test case added in 2/2 seems to pass even if running the AP with country
> US or CA..
>

Test itself choose/force EU country and expect AP-CSA-FINISHED
+        # Toggle regulatory - clean all preCAC
+        hostapd.cmd_execute(apdev[0], ['iw', 'reg', 'set', "US"])
+
+        hapd = start_dfs_ap(apdev[0], country="PL")

I can add negative test also, where will set (we could pass country as a param)
hapd = start_dfs_ap(apdev[0], country="US")
and will expect fail - (we will not get AP-CSA-FINISHED - while AP
will be restarted).

BR
Janusz
Jouni Malinen Oct. 9, 2020, 12:08 p.m. UTC | #3
On Fri, Oct 09, 2020 at 01:44:32PM +0200, Janusz Dziedzic wrote:
> pt., 9 paź 2020 o 10:37 Jouni Malinen <j@w1.fi> napisał(a):
> >
> > On Sat, Oct 03, 2020 at 02:00:12PM +0200, Janusz Dziedzic wrote:
> > > For EU, where preCAC is allowed, we should
> > > allow switch to DFS available channels, instead
> > > of restart BSS.
> >
> > Could you please clarify how this is supposed to work in regulatory
> > domains that do not allow previously done CAC results to be used? The
> > test case added in 2/2 seems to pass even if running the AP with country
> > US or CA..
> 
> Test itself choose/force EU country and expect AP-CSA-FINISHED
> +        # Toggle regulatory - clean all preCAC
> +        hostapd.cmd_execute(apdev[0], ['iw', 'reg', 'set', "US"])
> +
> +        hapd = start_dfs_ap(apdev[0], country="PL")

And that "PL" is indeed what I replaced with "CA" and "US" to confirm
that patch 1/2 did not enable something that it shouldn't enable..
However, the test was still passing when the country code was pointing
to non-ETSI country. Can you please clarify why this is the case and how
does this protect expected functionality (new CAC required) outside the
ETSI regulatory domain?

> I can add negative test also, where will set (we could pass country as a param)
> hapd = start_dfs_ap(apdev[0], country="US")
> and will expect fail - (we will not get AP-CSA-FINISHED - while AP
> will be restarted).

It would be useful to do that as well especially since I was trying to
do that exact check just to notice that the expected second CAC did not
happen.
Janusz Dziedzic Oct. 9, 2020, 1:20 p.m. UTC | #4
pt., 9 paź 2020 o 14:11 Jouni Malinen <j@w1.fi> napisał(a):
>
> On Fri, Oct 09, 2020 at 01:44:32PM +0200, Janusz Dziedzic wrote:
> > pt., 9 paź 2020 o 10:37 Jouni Malinen <j@w1.fi> napisał(a):
> > >
> > > On Sat, Oct 03, 2020 at 02:00:12PM +0200, Janusz Dziedzic wrote:
> > > > For EU, where preCAC is allowed, we should
> > > > allow switch to DFS available channels, instead
> > > > of restart BSS.
> > >
> > > Could you please clarify how this is supposed to work in regulatory
> > > domains that do not allow previously done CAC results to be used? The
> > > test case added in 2/2 seems to pass even if running the AP with country
> > > US or CA..
> >
> > Test itself choose/force EU country and expect AP-CSA-FINISHED
> > +        # Toggle regulatory - clean all preCAC
> > +        hostapd.cmd_execute(apdev[0], ['iw', 'reg', 'set', "US"])
> > +
> > +        hapd = start_dfs_ap(apdev[0], country="PL")
>
> And that "PL" is indeed what I replaced with "CA" and "US" to confirm
> that patch 1/2 did not enable something that it shouldn't enable..
> However, the test was still passing when the country code was pointing
> to non-ETSI country. Can you please clarify why this is the case and how
> does this protect expected functionality (new CAC required) outside the
> ETSI regulatory domain?
>
Seems I see what is wrong. This is mainly because we cache channels state
in hostapd and my code do fast chan_switch:

CHAN_SWITCH 5 5180 ht
CHAN_SWITCH 5 5260 ht

and we get late DFS-PRE-CAC-EXPIRED

When add 2 second sleep between chan_switch - fail correctly:
@@ -662,7 +662,7 @@ def test_dfs_chan_switch_precac(dev, apdev):
         # Toggle regulatory - clean all preCAC
         hostapd.cmd_execute(apdev[0], ['iw', 'reg', 'set', "US"])

-        hapd = start_dfs_ap(apdev[0], country="PL")
+        hapd = start_dfs_ap(apdev[0], country="US")

         ev = wait_dfs_event(hapd, "DFS-CAC-COMPLETED", 70)
         if "success=1" not in ev:
@@ -692,6 +692,8 @@ def test_dfs_chan_switch_precac(dev, apdev):
         if freq != "5180":
             raise Exception("Unexpected frequency")

+        time.sleep(2)
+

192.168.0.11/wlp2s0: <3>DFS-CAC-START freq=5260 chan=52 sec_chan=0,
width=0, seg0=0, seg1=0, cac_time=60s
192.168.0.11/wlp2s0: CTRL: STATUS
192.168.0.11/wlp2s0: <3>DFS-CAC-COMPLETED success=1 freq=5260
ht_enabled=1 chan_offset=0 chan_width=1 cf1=5260 cf2=0
192.168.0.11/wlp2s0: <3>AP-ENABLED
192.168.0.11/wlp2s0: CTRL: STATUS
192.168.0.11/wlp2s0: CTRL: CHAN_SWITCH 5 5180 ht
192.168.0.11/wlp2s0: <3>CTRL-EVENT-STARTED-CHANNEL-SWITCH freq=5180
ht_enabled=1 ch_offset=0 ch_width=20 MHz cf1=5180 cf2=0 dfs=0
192.168.0.11/wlp2s0: <3>CTRL-EVENT-CHANNEL-SWITCH freq=5180
ht_enabled=1 ch_offset=0 ch_width=20 MHz cf1=5180 cf2=0 dfs=0
192.168.0.11/wlp2s0: <3>AP-CSA-FINISHED freq=5180 dfs=0
192.168.0.11/wlp2s0: CTRL: STATUS
192.168.0.11/wlp2s0: CTRL: CHAN_SWITCH 5 5260 ht
192.168.0.11/wlp2s0: <3>DFS-PRE-CAC-EXPIRED freq=5260 ht_enabled=0
chan_offset=0 chan_width=0 cf1=5260 cf2=0
192.168.0.11/wlp2s0: <3>AP-DISABLED
192.168.0.11/wlp2s0: <3>DFS-CAC-START freq=5260 chan=52 sec_chan=0,
width=0, seg0=0, seg1=0, cac_time=60s
192.168.0.11/wlp2s0: CTRL: DISABLE

Should I send patch with 2 seconds delay between chan_switch and add
negative test also?

But for the future shouldn't we always read channels state from
kernel/driver/lower_layer?
Each time we get some EVENT/CTRL re-read channel states?

BR
Janusz


> > I can add negative test also, where will set (we could pass country as a param)
> > hapd = start_dfs_ap(apdev[0], country="US")
> > and will expect fail - (we will not get AP-CSA-FINISHED - while AP
> > will be restarted).
>
> It would be useful to do that as well especially since I was trying to
> do that exact check just to notice that the expected second CAC did not
> happen.
>
> --
> Jouni Malinen                                            PGP id EFC895FA



--
Janusz Dziedzic
Janusz Dziedzic Oct. 9, 2020, 3:31 p.m. UTC | #5
pt., 9 paź 2020 o 15:20 Janusz Dziedzic <janusz.dziedzic@gmail.com> napisał(a):
>
> pt., 9 paź 2020 o 14:11 Jouni Malinen <j@w1.fi> napisał(a):
> >
> > On Fri, Oct 09, 2020 at 01:44:32PM +0200, Janusz Dziedzic wrote:
> > > pt., 9 paź 2020 o 10:37 Jouni Malinen <j@w1.fi> napisał(a):
> > > >
> > > > On Sat, Oct 03, 2020 at 02:00:12PM +0200, Janusz Dziedzic wrote:
> > > > > For EU, where preCAC is allowed, we should
> > > > > allow switch to DFS available channels, instead
> > > > > of restart BSS.
> > > >
> > > > Could you please clarify how this is supposed to work in regulatory
> > > > domains that do not allow previously done CAC results to be used? The
> > > > test case added in 2/2 seems to pass even if running the AP with country
> > > > US or CA..
> > >
> > > Test itself choose/force EU country and expect AP-CSA-FINISHED
> > > +        # Toggle regulatory - clean all preCAC
> > > +        hostapd.cmd_execute(apdev[0], ['iw', 'reg', 'set', "US"])
> > > +
> > > +        hapd = start_dfs_ap(apdev[0], country="PL")
> >
> > And that "PL" is indeed what I replaced with "CA" and "US" to confirm
> > that patch 1/2 did not enable something that it shouldn't enable..
> > However, the test was still passing when the country code was pointing
> > to non-ETSI country. Can you please clarify why this is the case and how
> > does this protect expected functionality (new CAC required) outside the
> > ETSI regulatory domain?
> >
> Seems I see what is wrong. This is mainly because we cache channels state
> in hostapd and my code do fast chan_switch:
>
> CHAN_SWITCH 5 5180 ht
> CHAN_SWITCH 5 5260 ht
>
> and we get late DFS-PRE-CAC-EXPIRED
>
> When add 2 second sleep between chan_switch - fail correctly:
> @@ -662,7 +662,7 @@ def test_dfs_chan_switch_precac(dev, apdev):
>          # Toggle regulatory - clean all preCAC
>          hostapd.cmd_execute(apdev[0], ['iw', 'reg', 'set', "US"])
>
> -        hapd = start_dfs_ap(apdev[0], country="PL")
> +        hapd = start_dfs_ap(apdev[0], country="US")
>
>          ev = wait_dfs_event(hapd, "DFS-CAC-COMPLETED", 70)
>          if "success=1" not in ev:
> @@ -692,6 +692,8 @@ def test_dfs_chan_switch_precac(dev, apdev):
>          if freq != "5180":
>              raise Exception("Unexpected frequency")
>
> +        time.sleep(2)
> +
>
> 192.168.0.11/wlp2s0: <3>DFS-CAC-START freq=5260 chan=52 sec_chan=0,
> width=0, seg0=0, seg1=0, cac_time=60s
> 192.168.0.11/wlp2s0: CTRL: STATUS
> 192.168.0.11/wlp2s0: <3>DFS-CAC-COMPLETED success=1 freq=5260
> ht_enabled=1 chan_offset=0 chan_width=1 cf1=5260 cf2=0
> 192.168.0.11/wlp2s0: <3>AP-ENABLED
> 192.168.0.11/wlp2s0: CTRL: STATUS
> 192.168.0.11/wlp2s0: CTRL: CHAN_SWITCH 5 5180 ht
> 192.168.0.11/wlp2s0: <3>CTRL-EVENT-STARTED-CHANNEL-SWITCH freq=5180
> ht_enabled=1 ch_offset=0 ch_width=20 MHz cf1=5180 cf2=0 dfs=0
> 192.168.0.11/wlp2s0: <3>CTRL-EVENT-CHANNEL-SWITCH freq=5180
> ht_enabled=1 ch_offset=0 ch_width=20 MHz cf1=5180 cf2=0 dfs=0
> 192.168.0.11/wlp2s0: <3>AP-CSA-FINISHED freq=5180 dfs=0
> 192.168.0.11/wlp2s0: CTRL: STATUS
> 192.168.0.11/wlp2s0: CTRL: CHAN_SWITCH 5 5260 ht
> 192.168.0.11/wlp2s0: <3>DFS-PRE-CAC-EXPIRED freq=5260 ht_enabled=0
> chan_offset=0 chan_width=0 cf1=5260 cf2=0
> 192.168.0.11/wlp2s0: <3>AP-DISABLED
> 192.168.0.11/wlp2s0: <3>DFS-CAC-START freq=5260 chan=52 sec_chan=0,
> width=0, seg0=0, seg1=0, cac_time=60s
> 192.168.0.11/wlp2s0: CTRL: DISABLE
>
> Should I send patch with 2 seconds delay between chan_switch and add
> negative test also?
>
> But for the future shouldn't we always read channels state from
> kernel/driver/lower_layer?
> Each time we get some EVENT/CTRL re-read channel states?
>
Did fast check in cfg80211 code and seems we:

queue_delayed_work(cfg80211_wq, &rdev->dfs_update_channels_wk, 0);
send(NL80211_CMD_CH_SWITCH_NOTIFY)

Unfortunately delayed work recalc also dfs_state and send PRE-CAC-EXPIRED
First we get CH_SWITCH_NOTIFY and next PRE-CAC-EXPIRED..

More, we can run another chan_switch before set correct dfs_state
(clean CAC state in our case).
Seems like a cfg80211 bug, while we should send CH_SWITCH_NOTIFY after
dfs_state recalc.

BR
Janusz
diff mbox series

Patch

diff --git a/src/ap/dfs.c b/src/ap/dfs.c
index 3c078b9cb..a19b8f164 100644
--- a/src/ap/dfs.c
+++ b/src/ap/dfs.c
@@ -1332,12 +1332,15 @@  int hostapd_is_dfs_overlap(struct hostapd_iface *iface, enum chan_width width,
 		if (!(chan->flag & HOSTAPD_CHAN_RADAR))
 			continue;
 
+		if ((chan->flag & HOSTAPD_CHAN_DFS_MASK) == HOSTAPD_CHAN_DFS_AVAILABLE)
+			continue;
+
 		if (center_freq - chan->freq < half_width &&
 		    chan->freq - center_freq < half_width)
 			res++;
 	}
 
-	wpa_printf(MSG_DEBUG, "DFS: (%d, %d): in range: %s",
+	wpa_printf(MSG_DEBUG, "DFS CAC required: (%d, %d): in range: %s",
 		   center_freq - half_width, center_freq + half_width,
 		   res ? "yes" : "no");