[PATCH 1/5] auto-t: add test for roaming retry timing

[PATCH 1/5] auto-t: add test for roaming retry timing

[Alvin =?unknown-8bit?q?=C5=A0ipraga?=]

[-- Attachment #1: Type: text/plain, Size: 6971 bytes --]

From: Alvin Šipraga <alvin@pqrs.dk>

Recent changes make iwd more eager to retry roaming if it has detected a
high RSSI since the last attempt. Add an autotest to verify this
behaviour.
---
 autotests/testRoamRetry/fast_retry_test.py | 145 +++++++++++++++++++++
 autotests/testRoamRetry/hw.conf            |   7 +
 autotests/testRoamRetry/ssid1.conf         |  11 ++
 autotests/testRoamRetry/ssid2.conf         |  11 ++
 4 files changed, 174 insertions(+)
 create mode 100644 autotests/testRoamRetry/fast_retry_test.py
 create mode 100644 autotests/testRoamRetry/hw.conf
 create mode 100644 autotests/testRoamRetry/ssid1.conf
 create mode 100644 autotests/testRoamRetry/ssid2.conf

diff --git a/autotests/testRoamRetry/fast_retry_test.py b/autotests/testRoamRetry/fast_retry_test.py
new file mode 100644
index 00000000..32d80002
--- /dev/null
+++ b/autotests/testRoamRetry/fast_retry_test.py
@@ -0,0 +1,145 @@
+#!/usr/bin/python3
+
+import unittest
+import sys
+
+sys.path.append('../util')
+import iwd
+from iwd import IWD
+from iwd import PSKAgent
+from iwd import NetworkType
+
+import testutil
+
+from hostapd import HostapdCLI
+from hwsim import Hwsim
+
+class Test(unittest.TestCase):
+    # Normally the time between a failed roam attempt and the next roam attempt
+    # is 60 seconds (default RoamRetryInterval). Test that we retry roaming
+    # faster if the transision looks like this: LOW [roam] [same bss] HIGH LOW.
+    def test_fast_retry(self):
+        hwsim = Hwsim()
+
+        bss_hostapd = [ HostapdCLI(config='ssid1.conf'),
+                        HostapdCLI(config='ssid2.conf') ]
+        bss_radio =  [ hwsim.get_radio('rad0'),
+                       hwsim.get_radio('rad1') ]
+
+        rule0 = hwsim.rules.create()
+        rule0.source = bss_radio[0].addresses[0]
+        rule0.bidirectional = True
+
+        rule1 = hwsim.rules.create()
+        rule1.source = bss_radio[1].addresses[0]
+        rule1.bidirectional = True
+
+        # Fill in the neighbor AP tables in both BSSes.  By default each
+        # instance knows only about current BSS, even inside one hostapd
+        # process.
+        # Roaming still works without the neighbor AP table but neighbor
+        # reports have to be disabled in the .conf files
+        bss0_nr = ''.join(bss_radio[0].addresses[0].split(':')) + \
+                '8f0000005101060603000000'
+        bss1_nr = ''.join(bss_radio[1].addresses[0].split(':')) + \
+                '8f0000005102060603000000'
+
+        bss_hostapd[0].set_neighbor(bss_radio[1].addresses[0], 'TestRoamRetry',
+                bss1_nr)
+        bss_hostapd[1].set_neighbor(bss_radio[0].addresses[0], 'TestRoamRetry',
+                bss0_nr)
+
+        # Start in the vicinity of BSS 0, check that iwd connects to BSS 0
+        rule0.signal = -2000
+        rule1.signal = -8500
+
+        wd = IWD()
+
+        psk_agent = PSKAgent("secret123")
+        wd.register_psk_agent(psk_agent)
+
+        devices = wd.list_devices(1)
+        device = devices[0]
+
+        condition = 'not obj.scanning'
+        wd.wait_for_object_condition(device, condition)
+
+        device.scan()
+
+        condition = 'not obj.scanning'
+        wd.wait_for_object_condition(device, condition)
+
+        ordered_network = device.get_ordered_network('TestRoamRetry')
+
+        self.assertEqual(ordered_network.type, NetworkType.psk)
+
+        condition = 'not obj.connected'
+        wd.wait_for_object_condition(ordered_network.network_object, condition)
+
+        ordered_network.network_object.connect()
+
+        condition = 'obj.state == DeviceState.connected'
+        wd.wait_for_object_condition(device, condition)
+
+        self.assertTrue(bss_hostapd[0].list_sta())
+        self.assertFalse(bss_hostapd[1].list_sta())
+
+        wd.wait(5)
+
+        # Now push the signal LOW, wait for iwd to attempt a roam, fail, and
+        # schedule another attempt for 60 seconds later
+        rule0.signal = -8000
+
+        wd.wait(20)
+
+        self.assertEqual(device.state, iwd.DeviceState.connected)
+        self.assertTrue(bss_hostapd[0].list_sta())
+        self.assertFalse(bss_hostapd[1].list_sta())
+
+        testutil.test_iface_operstate(device.name)
+        testutil.test_ifaces_connected(bss_hostapd[0].ifname, device.name)
+
+        # Assert high signal for BSS 0 again. This clears the way for a faster
+        # roam attempt on LOW again
+        rule0.signal = -5000
+
+        # Wait a little for signal recognition
+        wd.wait(1)
+
+        # Assert low signal for BSS 0, check that iwd starts transition to BSS 1
+        # in less than 10 seconds. Because of the neighbor report a scan should
+        # not be necessary.
+        rule0.signal = -8000
+        rule1.signal = -2000
+
+        condition = 'obj.state == DeviceState.roaming'
+        wd.wait_for_object_condition(device, condition, max_wait=10)
+
+        # Check that iwd is on BSS 1 once out of roaming state and doesn't
+        # go through 'disconnected', 'autoconnect', 'connecting' in between
+        condition = 'obj.state != DeviceState.roaming'
+        wd.wait_for_object_condition(device, condition, max_wait=5)
+
+        self.assertEqual(device.state, iwd.DeviceState.connected)
+        self.assertTrue(bss_hostapd[1].list_sta())
+
+        device.disconnect()
+
+        condition = 'not obj.connected'
+        wd.wait_for_object_condition(ordered_network.network_object, condition)
+
+        wd.unregister_psk_agent(psk_agent)
+
+        rule0.remove()
+        rule1.remove()
+
+    @classmethod
+    def setUpClass(cls):
+        pass
+
+    @classmethod
+    def tearDownClass(cls):
+        IWD.clear_storage()
+
+if __name__ == '__main__':
+    unittest.main(exit=True)
diff --git a/autotests/testRoamRetry/hw.conf b/autotests/testRoamRetry/hw.conf
new file mode 100644
index 00000000..5dbe97b4
--- /dev/null
+++ b/autotests/testRoamRetry/hw.conf
@@ -0,0 +1,7 @@
+[SETUP]
+num_radios=3
+needs_hwsim=1
+
+[HOSTAPD]
+rad0=ssid1.conf
+rad1=ssid2.conf
diff --git a/autotests/testRoamRetry/ssid1.conf b/autotests/testRoamRetry/ssid1.conf
new file mode 100644
index 00000000..2c5cc8d6
--- /dev/null
+++ b/autotests/testRoamRetry/ssid1.conf
@@ -0,0 +1,11 @@
+hw_mode=g
+channel=1
+ssid=TestRoamRetry
+
+wpa=2
+wpa_pairwise=CCMP
+wpa_passphrase=secret123
+
+ctrl_interface=/var/run/hostapd
+
+rrm_neighbor_report=1
diff --git a/autotests/testRoamRetry/ssid2.conf b/autotests/testRoamRetry/ssid2.conf
new file mode 100644
index 00000000..5626c032
--- /dev/null
+++ b/autotests/testRoamRetry/ssid2.conf
@@ -0,0 +1,11 @@
+hw_mode=g
+channel=2
+ssid=TestRoamRetry
+
+wpa=2
+wpa_pairwise=CCMP
+wpa_passphrase=secret123
+
+ctrl_interface=/var/run/hostapd
+
+rrm_neighbor_report=1
-- 
2.29.2

[PATCH 2/5] auto-t: throw exception if hostapd can't send BSS transition req

[Alvin =?unknown-8bit?q?=C5=A0ipraga?=]

[-- Attachment #1: Type: text/plain, Size: 1173 bytes --]

The testAPRoam autotest was silently failing on my machine until I
realized that my distribution hostapd (Arch Linux) is not built with
CONFIG_WNM_AP=y. Indeed, it is also disabled by default in upstream
hostapd. This resulted in the send_bss_transition() function of
hostapd.py silently failing. With this change, throw an exception in
case the BSS_TM_REQ command does not succeed to hopefully save others
the time of debugging this problem.
---
 autotests/util/hostapd.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/autotests/util/hostapd.py b/autotests/util/hostapd.py
index 019d23ba..8e0b3766 100644
--- a/autotests/util/hostapd.py
+++ b/autotests/util/hostapd.py
@@ -184,7 +184,10 @@ class HostapdCLI:
                         (addr, bss_info, op_class, chan_num, phy_num)]
             pref += 1
 
-        ctx.start_process(cmd, wait=True)
+        proc = ctx.start_process(cmd, wait=True)
+
+        if 'OK' not in proc.out:
+            raise Exception('BSS_TM_REQ failed, is hostapd built with CONFIG_WNM_AP=y?')
 
     def get_config_value(self, key):
         # first find the right config file
-- 
2.29.2

Re: [PATCH 2/5] auto-t: throw exception if hostapd can't send BSS transition req

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]

Hi Alvin,

On Wed, 2021-01-27 at 10:05 +0000, Alvin Šipraga wrote:
> The testAPRoam autotest was silently failing on my machine until I
> realized that my distribution hostapd (Arch Linux) is not built with
> CONFIG_WNM_AP=y. Indeed, it is also disabled by default in upstream
> hostapd. This resulted in the send_bss_transition() function of
> hostapd.py silently failing. With this change, throw an exception in
> case the BSS_TM_REQ command does not succeed to hopefully save others
> the time of debugging this problem.
> ---
>  autotests/util/hostapd.py | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/autotests/util/hostapd.py b/autotests/util/hostapd.py
> index 019d23ba..8e0b3766 100644
> --- a/autotests/util/hostapd.py
> +++ b/autotests/util/hostapd.py
> @@ -184,7 +184,10 @@ class HostapdCLI:
>                          (addr, bss_info, op_class, chan_num,
> phy_num)]
>              pref += 1
>  
> -        ctx.start_process(cmd, wait=True)
> +        proc = ctx.start_process(cmd, wait=True)

Good catch on this. Within the last day I fixed a logging/verbose issue
with test-runner which this patch would be affected by. My
fix/workaround is to explicitly tell start_process if there is output
being requested with 'need_out=True'. I've got other ideas in the
works, but for now this is how its got to be done. This should work for
you:

proc = ctx.start_process(cmd, wait=True, need_out=True)

Thanks,
James

Re: [PATCH 2/5] auto-t: throw exception if hostapd can't send BSS transition req

[Alvin =?unknown-8bit?q?=C5=A0ipraga?=]

[-- Attachment #1: Type: text/plain, Size: 1823 bytes --]

Hi James,

On 1/27/21 6:28 PM, James Prestwood wrote:
> Hi Alvin,
> 
> On Wed, 2021-01-27 at 10:05 +0000, Alvin Šipraga wrote:
>> The testAPRoam autotest was silently failing on my machine until I
>> realized that my distribution hostapd (Arch Linux) is not built with
>> CONFIG_WNM_AP=y. Indeed, it is also disabled by default in upstream
>> hostapd. This resulted in the send_bss_transition() function of
>> hostapd.py silently failing. With this change, throw an exception in
>> case the BSS_TM_REQ command does not succeed to hopefully save others
>> the time of debugging this problem.
>> ---
>>   autotests/util/hostapd.py | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/autotests/util/hostapd.py b/autotests/util/hostapd.py
>> index 019d23ba..8e0b3766 100644
>> --- a/autotests/util/hostapd.py
>> +++ b/autotests/util/hostapd.py
>> @@ -184,7 +184,10 @@ class HostapdCLI:
>>                           (addr, bss_info, op_class, chan_num,
>> phy_num)]
>>               pref += 1
>>   
>> -        ctx.start_process(cmd, wait=True)
>> +        proc = ctx.start_process(cmd, wait=True)
> 
> Good catch on this. Within the last day I fixed a logging/verbose issue
> with test-runner which this patch would be affected by. My
> fix/workaround is to explicitly tell start_process if there is output
> being requested with 'need_out=True'. I've got other ideas in the
> works, but for now this is how its got to be done. This should work for
> you:
> 
> proc = ctx.start_process(cmd, wait=True, need_out=True)

Thanks. I actually fixed the same issue of --log not working with 
hostapd_cli, but your solution was more generic so I dropped the patch 
when sending. Forgot to modify this accordingly though. Sent a v2 patch now.

Kind regards,
Alvin

[PATCH 3/5] doc: update kernel configuration instructions for KVM guests

[Alvin =?unknown-8bit?q?=C5=A0ipraga?=]

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

The kvmguest shorthand was removed after the release of Linux 5.10. It
was just shorthand for kvm_guest.config anyway, so update the
test-runner documentation accordingly.
---
 doc/test-runner.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/doc/test-runner.txt b/doc/test-runner.txt
index 17d7bbb2..3426d1dc 100644
--- a/doc/test-runner.txt
+++ b/doc/test-runner.txt
@@ -48,7 +48,7 @@ minimal options for a successful boot and execution:
 
 	<arch>_defconfig 			Default kernel configuration
 
-	kvmconfig				Default configuration for
+	kvm_guest.config			Default configuration for
 						kvm guests
 
 	<iwd>/tools/test_runner_kernel_config	The test-runner specific
@@ -62,7 +62,7 @@ as follows:
 
 	$ make x86_64_defconfig
 
-	$ make kvmconfig
+	$ make kvm_guest.config
 
 	$ sh <iwd>/tools/test_runner_kernel_config
 
-- 
2.29.2

Re: [PATCH 3/5] doc: update kernel configuration instructions for KVM guests

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 387 bytes --]

Hi Alvin,

On 1/27/21 4:05 AM, Alvin Šipraga wrote:
> The kvmguest shorthand was removed after the release of Linux 5.10. It
> was just shorthand for kvm_guest.config anyway, so update the
> test-runner documentation accordingly.
> ---
>   doc/test-runner.txt | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 

Patches 3-5 applied, thanks.

Regards,
-Denis

[PATCH 4/5] doc: add python package requirements for test-runner

[Alvin =?unknown-8bit?q?=C5=A0ipraga?=]

[-- Attachment #1: Type: text/plain, Size: 720 bytes --]

---
 doc/test-runner.txt | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/doc/test-runner.txt b/doc/test-runner.txt
index 3426d1dc..c5f29324 100644
--- a/doc/test-runner.txt
+++ b/doc/test-runner.txt
@@ -40,6 +40,15 @@ Note: Running EAP-SIM/AKA/AKA' tests using oFono will require oFono and
 phonesim to be installed on the host. This is explained further in the
 "Running with oFono and phonesim" section.
 
+In addition, the following Python packages are required:
+
+	Name:			Tested ver.:
+	 1. python-prettytable	0.7.2
+	 2. python-termcolor	1.1.0
+	 3. python-pyroute2	0.5.14
+	 4. python-gobject	3.38.0
+	 5. python-dbus		1.2.16
+
 Building Kernel
 ===============
 
-- 
2.29.2

[PATCH 5/5] doc: update test-runner invocation documentation

[Alvin =?unknown-8bit?q?=C5=A0ipraga?=]

[-- Attachment #1: Type: text/plain, Size: 1014 bytes --]

The test-runner no longer recognizes -t to specify an autotest. Instead
it uses -A.
---
 doc/test-runner.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/test-runner.txt b/doc/test-runner.txt
index c5f29324..89a7c44c 100644
--- a/doc/test-runner.txt
+++ b/doc/test-runner.txt
@@ -137,16 +137,16 @@ tests shipped with IWD:
 	$ sudo ./test-runner
 
 One can specify a particular set of test configurations to be executed by using
-'-t <dir1,dir2>' parameter. An absolute path is necessary for the test
+'-A <dir1,dir2>' parameter. An absolute path is necessary for the test
 configuration directories outside of <iwd>/autotests.
 
 The command line may look as follows:
 
-	$ sudo ./test-runner -t test1,test3,/home/test4
+	$ sudo ./test-runner -A test1,test3,/home/test4
 
 Glob matching is also supported when specifying a test list:
 
-	$ sudo ./test-runner -t testWPA*
+	$ sudo ./test-runner -A testWPA*
 
 
 Creating Test Configurations
-- 
2.29.2

Re: [PATCH 1/5] auto-t: add test for roaming retry timing

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 919 bytes --]

Hi Alvin,

On 1/27/21 4:05 AM, Alvin Šipraga wrote:
> From: Alvin Šipraga <alvin@pqrs.dk>
> 
> Recent changes make iwd more eager to retry roaming if it has detected a
> high RSSI since the last attempt. Add an autotest to verify this
> behaviour.
> ---
>   autotests/testRoamRetry/fast_retry_test.py | 145 +++++++++++++++++++++
>   autotests/testRoamRetry/hw.conf            |   7 +
>   autotests/testRoamRetry/ssid1.conf         |  11 ++
>   autotests/testRoamRetry/ssid2.conf         |  11 ++
>   4 files changed, 174 insertions(+)
>   create mode 100644 autotests/testRoamRetry/fast_retry_test.py
>   create mode 100644 autotests/testRoamRetry/hw.conf
>   create mode 100644 autotests/testRoamRetry/ssid1.conf
>   create mode 100644 autotests/testRoamRetry/ssid2.conf
> 

I went ahead and applied this after running dos2unix to fix the weird Windows 
style line endings.

Regards,
-Denis

Re: [PATCH 1/5] auto-t: add test for roaming retry timing

[Alvin =?unknown-8bit?q?=C5=A0ipraga?=]

[-- Attachment #1: Type: text/plain, Size: 1359 bytes --]

Hi Denis,

On 1/27/21 8:19 PM, Denis Kenzior wrote:
> Hi Alvin,
> 
> On 1/27/21 4:05 AM, Alvin Šipraga wrote:
>> From: Alvin Šipraga <alvin@pqrs.dk>
>>
>> Recent changes make iwd more eager to retry roaming if it has detected a
>> high RSSI since the last attempt. Add an autotest to verify this
>> behaviour.
>> ---
>>   autotests/testRoamRetry/fast_retry_test.py | 145 +++++++++++++++++++++
>>   autotests/testRoamRetry/hw.conf            |   7 +
>>   autotests/testRoamRetry/ssid1.conf         |  11 ++
>>   autotests/testRoamRetry/ssid2.conf         |  11 ++
>>   4 files changed, 174 insertions(+)
>>   create mode 100644 autotests/testRoamRetry/fast_retry_test.py
>>   create mode 100644 autotests/testRoamRetry/hw.conf
>>   create mode 100644 autotests/testRoamRetry/ssid1.conf
>>   create mode 100644 autotests/testRoamRetry/ssid2.conf
>>
> 
> I went ahead and applied this after running dos2unix to fix the weird 
> Windows style line endings.

Hmm, that's strange. Was it just for this patch? I prepared the initial 
commit on a separate machine, but I would be surprised about Windows 
line endings. Just wondering if my outgoing mail is getting modified or 
if my setup is busted in some other way. Could not detect anything wrong 
on my end though...

Kind regards,
Alvin

Re: [PATCH 1/5] auto-t: add test for roaming retry timing

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 1670 bytes --]

Hi Alvin,

On 1/28/21 3:31 AM, Alvin Šipraga wrote:
> Hi Denis,
> 
> On 1/27/21 8:19 PM, Denis Kenzior wrote:
>> Hi Alvin,
>>
>> On 1/27/21 4:05 AM, Alvin Šipraga wrote:
>>> From: Alvin Šipraga <alvin@pqrs.dk>
>>>
>>> Recent changes make iwd more eager to retry roaming if it has detected a
>>> high RSSI since the last attempt. Add an autotest to verify this
>>> behaviour.
>>> ---
>>>    autotests/testRoamRetry/fast_retry_test.py | 145 +++++++++++++++++++++
>>>    autotests/testRoamRetry/hw.conf            |   7 +
>>>    autotests/testRoamRetry/ssid1.conf         |  11 ++
>>>    autotests/testRoamRetry/ssid2.conf         |  11 ++
>>>    4 files changed, 174 insertions(+)
>>>    create mode 100644 autotests/testRoamRetry/fast_retry_test.py
>>>    create mode 100644 autotests/testRoamRetry/hw.conf
>>>    create mode 100644 autotests/testRoamRetry/ssid1.conf
>>>    create mode 100644 autotests/testRoamRetry/ssid2.conf
>>>
>>
>> I went ahead and applied this after running dos2unix to fix the weird
>> Windows style line endings.
> 
> Hmm, that's strange. Was it just for this patch? I prepared the initial
> commit on a separate machine, but I would be surprised about Windows

Yes, just for that patch.  The others were fine.  These were all new files; were 
they somehow treated differently?

> line endings. Just wondering if my outgoing mail is getting modified or
> if my setup is busted in some other way. Could not detect anything wrong
> on my end though...

Could be the mail server is mangling them.  Doesn't explain why the rest was 
fine though.

Regards,
-Denis

Re: [PATCH 1/5] auto-t: add test for roaming retry timing

[James Prestwood]

[-- Attachment #1: Type: text/plain, Size: 665 bytes --]

Hi Alvin,

Looks like this already got applied as I was writing but...

<snip>

> diff --git a/autotests/testRoamRetry/hw.conf
> b/autotests/testRoamRetry/hw.conf
> new file mode 100644
> index 00000000..5dbe97b4
> --- /dev/null
> +++ b/autotests/testRoamRetry/hw.conf
> @@ -0,0 +1,7 @@
> +[SETUP]
> +num_radios=3
> +needs_hwsim=1

'needs_hwsim' is no longer required (yes, I know lots of tests still
have this). test-runner assumes hwsim is being used unless
'hwsim_medium=no' is found. I need to take some time and remove this
along with several other stale options. Ill remove this for you since
it was already applied.

Thanks,
James