[PATCH 1/3] auto-t: add explicit stop() to IWD class

[PATCH 1/3] auto-t: add explicit stop() to IWD class

[James Prestwood]

If tests end in an unknown state it is sometimes required that IWD
be stopped manually in order for future tests to run. Add a stop()
method so test tearDown() methods can explicitly stop IWD.
---
 autotests/util/iwd.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/autotests/util/iwd.py b/autotests/util/iwd.py
index 3f200beb..b1a57cab 100755
--- a/autotests/util/iwd.py
+++ b/autotests/util/iwd.py
@@ -1363,6 +1363,10 @@ class IWD(AsyncOpAbstract):
 
         self.psk_agents = []
 
+    def stop(self):
+        if self.namespace.is_process_running('iwd'):
+            self._iwd_proc.kill()
+
     def __del__(self):
         for agent in self.psk_agents:
             self.unregister_psk_agent(agent)
-- 
2.34.1

[PATCH 2/3] auto-t: add association timeout test

[James Prestwood]

This tests ensures IWD disconnects after receiving an association
timeout event. This exposes a current bug where IWD does not
transition to disconnected after an association timeout when
FT-roaming.
---
 autotests/testPSK-roam/failed_roam_test.py | 56 +++++++++++++++-------
 1 file changed, 38 insertions(+), 18 deletions(-)

diff --git a/autotests/testPSK-roam/failed_roam_test.py b/autotests/testPSK-roam/failed_roam_test.py
index 90f07f6b..d42002d4 100644
--- a/autotests/testPSK-roam/failed_roam_test.py
+++ b/autotests/testPSK-roam/failed_roam_test.py
@@ -46,11 +46,9 @@ class Test(unittest.TestCase):
         self.rule2.enabled = True
         self.rule3.enabled = True
 
-        wd = IWD(True)
+        device = self.wd.list_devices(1)[0]
 
-        device = wd.list_devices(1)[0]
-
-        self.connect(wd, device, self.bss_hostapd[0])
+        self.connect(self.wd, device, self.bss_hostapd[0])
 
         self.rule0.enabled = True
 
@@ -63,10 +61,25 @@ class Test(unittest.TestCase):
 
         # IWD should then try BSS 2, and succeed
         device.wait_for_event('ft-roam', timeout=60)
-        self.verify_roam(wd, device, self.bss_hostapd[0], self.bss_hostapd[2])
+        self.verify_roam(self.wd, device, self.bss_hostapd[0], self.bss_hostapd[2])
 
         self.bss_hostapd[2].deauthenticate(device.address)
 
+    # Tests that an associate even should cause a disconnect
+    def test_ft_over_air_assoc_timeout(self):
+        self.rule2.enabled = True
+        self.rule3.enabled = True
+        self.assoc_rule.enabled = True
+
+        device = self.wd.list_devices(1)[0]
+
+        self.connect(self.wd, device, self.bss_hostapd[0])
+
+        device.wait_for_event('ft-roam', timeout=60)
+
+        condition = 'obj.state == DeviceState.disconnected'
+        self.wd.wait_for_object_condition(device, condition)
+
     # FT-over-Air failure with Invalid PMKID, should reassociate
     def test_ft_over_air_fallback(self):
         self.rule_bss0.signal = -8000
@@ -81,18 +94,16 @@ class Test(unittest.TestCase):
         self.bss_hostapd[2].set_value('ft_psk_generate_local', '0')
         self.bss_hostapd[2].reload()
 
-        wd = IWD(True)
+        device = self.wd.list_devices(1)[0]
 
-        device = wd.list_devices(1)[0]
-
-        self.connect(wd, device, self.bss_hostapd[0])
+        self.connect(self.wd, device, self.bss_hostapd[0])
 
         # IWD should connect, then attempt a roam to BSS 1, which should
         # fail and cause a fallback to reassociation
         device.wait_for_event('ft-fallback-to-reassoc', timeout=60)
         device.wait_for_event('reassoc-roam', timeout=60)
 
-        self.verify_roam(wd, device, self.bss_hostapd[0], self.bss_hostapd[2])
+        self.verify_roam(self.wd, device, self.bss_hostapd[0], self.bss_hostapd[2])
 
         # Trigger another roam
         self.rule_bss2.signal = -8000
@@ -100,11 +111,11 @@ class Test(unittest.TestCase):
         device.wait_for_event('ft-roam', timeout=60)
 
         # Ensure an FT roam back to a properly configured AP works.
-        self.verify_roam(wd, device, self.bss_hostapd[2], self.bss_hostapd[1])
+        self.verify_roam(self.wd, device, self.bss_hostapd[2], self.bss_hostapd[1])
 
         self.bss_hostapd[1].deauthenticate(device.address)
         condition = 'obj.state == DeviceState.disconnected'
-        wd.wait_for_object_condition(device, condition)
+        self.wd.wait_for_object_condition(device, condition)
 
     # FT-over-Air failure with Invalid PMKID. The ranking is such that other
     # FT candidates are available so it should FT elsewhere rather than
@@ -122,11 +133,9 @@ class Test(unittest.TestCase):
         self.bss_hostapd[2].set_value('ft_psk_generate_local', '0')
         self.bss_hostapd[2].reload()
 
-        wd = IWD(True)
+        device = self.wd.list_devices(1)[0]
 
-        device = wd.list_devices(1)[0]
-
-        self.connect(wd, device, self.bss_hostapd[0])
+        self.connect(self.wd, device, self.bss_hostapd[0])
 
         # IWD should connect, then attempt a roam to BSS 1, which should
         # fail and cause the rank to be re-computed. This should then put
@@ -134,12 +143,14 @@ class Test(unittest.TestCase):
         device.wait_for_event('ft-fallback-to-reassoc', timeout=60)
         device.wait_for_event('ft-roam', timeout=60)
 
-        self.verify_roam(wd, device, self.bss_hostapd[0], self.bss_hostapd[1])
+        self.verify_roam(self.wd, device, self.bss_hostapd[0], self.bss_hostapd[1])
 
         self.bss_hostapd[1].deauthenticate(device.address)
         condition = 'obj.state == DeviceState.disconnected'
-        wd.wait_for_object_condition(device, condition)
+        self.wd.wait_for_object_condition(device, condition)
 
+    def setUp(self):
+        self.wd = IWD(True)
 
     def tearDown(self):
         os.system('ip link set "' + self.bss_hostapd[0].ifname + '" down')
@@ -154,10 +165,14 @@ class Test(unittest.TestCase):
         self.rule_bss0.enabled = False
         self.rule_bss1.enabled = False
         self.rule_bss2.enabled = False
+        self.assoc_rule.enabled = False
 
         for hapd in self.bss_hostapd:
             hapd.default()
 
+        self.wd.stop()
+        self.wd = None
+
     @classmethod
     def setUpClass(cls):
         hwsim = Hwsim()
@@ -178,6 +193,11 @@ class Test(unittest.TestCase):
         cls.rule0.prefix = 'b0'
         cls.rule0.drop = True
 
+        # Drop Associate frames
+        cls.assoc_rule = hwsim.rules.create()
+        cls.assoc_rule.prefix = '20'
+        cls.assoc_rule.drop = True
+
         # Drop Action frames
         cls.rule1 = hwsim.rules.create()
         cls.rule1.bidirectional = True
-- 
2.34.1

[PATCH 3/3] netdev: hack back in disconnect event

[James Prestwood]

Not sure how we want to address this, but FT is special in that it
has no connect_cb but can still trigger an associate timeout.
Currently if FT times out associating IWD will hang indefinitely.

This behavior can be tested with the prior autotest patches (without
this patch applied).

Fixes: 30c6a10f28 ("netdev: Separate connect_failed and disconnected paths")
---
 src/netdev.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/netdev.c b/src/netdev.c
index f2e887b4..2d1120c4 100644
--- a/src/netdev.c
+++ b/src/netdev.c
@@ -3045,7 +3045,13 @@ static void netdev_associate_event(struct l_genl_msg *msg,
 			 * out. The failed connection must be explicitly
 			 * initiated here.
 			 */
-			netdev_connect_failed(netdev,
+			if (!netdev->ap) {
+				if (netdev->event_filter)
+					netdev->event_filter(netdev,
+						NETDEV_EVENT_DISCONNECT_BY_SME,
+						NULL, netdev->user_data);
+			} else
+				netdev_connect_failed(netdev,
 					NETDEV_RESULT_ASSOCIATION_FAILED,
 					status_code);
 			return;
-- 
2.34.1

Re: [PATCH 3/3] netdev: hack back in disconnect event

[Denis Kenzior]

Hi James,

On 12/5/23 11:52, James Prestwood wrote:
> Not sure how we want to address this, but FT is special in that it
> has no connect_cb but can still trigger an associate timeout.
> Currently if FT times out associating IWD will hang indefinitely.

Do our auto tests cover this case?  I wonder why they didn't flag this earlier?

> 
> This behavior can be tested with the prior autotest patches (without
> this patch applied).
> 
> Fixes: 30c6a10f28 ("netdev: Separate connect_failed and disconnected paths")
> ---
>   src/netdev.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/netdev.c b/src/netdev.c
> index f2e887b4..2d1120c4 100644
> --- a/src/netdev.c
> +++ b/src/netdev.c
> @@ -3045,7 +3045,13 @@ static void netdev_associate_event(struct l_genl_msg *msg,
>   			 * out. The failed connection must be explicitly
>   			 * initiated here.
>   			 */
> -			netdev_connect_failed(netdev,
> +			if (!netdev->ap) {

if (netdev->in_ft) ?

> +				if (netdev->event_filter)
> +					netdev->event_filter(netdev,
> +						NETDEV_EVENT_DISCONNECT_BY_SME,
> +						NULL, netdev->user_data);
> +			} else
> +				netdev_connect_failed(netdev,
>   					NETDEV_RESULT_ASSOCIATION_FAILED,
>   					status_code);
>   			return;

I wonder if we should make ft_associate use netdev_reassociate?

Regards,
-Denis

Re: [PATCH 3/3] netdev: hack back in disconnect event

[James Prestwood]

Hi Denis,

On 12/5/23 13:37, Denis Kenzior wrote:
> Hi James,
>
> On 12/5/23 11:52, James Prestwood wrote:
>> Not sure how we want to address this, but FT is special in that it
>> has no connect_cb but can still trigger an associate timeout.
>> Currently if FT times out associating IWD will hang indefinitely.
>
> Do our auto tests cover this case?  I wonder why they didn't flag this 
> earlier?
They don't (only auth timeouts), but patches 1/2 add an associate 
timeout test.
>
>>
>> This behavior can be tested with the prior autotest patches (without
>> this patch applied).
>>
>> Fixes: 30c6a10f28 ("netdev: Separate connect_failed and disconnected 
>> paths")
>> ---
>>   src/netdev.c | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/netdev.c b/src/netdev.c
>> index f2e887b4..2d1120c4 100644
>> --- a/src/netdev.c
>> +++ b/src/netdev.c
>> @@ -3045,7 +3045,13 @@ static void netdev_associate_event(struct 
>> l_genl_msg *msg,
>>                * out. The failed connection must be explicitly
>>                * initiated here.
>>                */
>> -            netdev_connect_failed(netdev,
>> +            if (!netdev->ap) {
>
> if (netdev->in_ft) ?
>
>> +                if (netdev->event_filter)
>> +                    netdev->event_filter(netdev,
>> +                        NETDEV_EVENT_DISCONNECT_BY_SME,
>> +                        NULL, netdev->user_data);
>> +            } else
>> +                netdev_connect_failed(netdev,
>>                       NETDEV_RESULT_ASSOCIATION_FAILED,
>>                       status_code);
>>               return;
>
> I wonder if we should make ft_associate use netdev_reassociate?

So pass station_connect_cb into ft_associate, then to netdev? That seems 
reasonable.

> Regards,
> -Denis

Re: [PATCH 3/3] netdev: hack back in disconnect event

[Denis Kenzior]

Hi James,

> They don't (only auth timeouts), but patches 1/2 add an associate timeout test.

Ah, I guess I should have looked at those too, whoops :)

>>
>> I wonder if we should make ft_associate use netdev_reassociate?
> 
> So pass station_connect_cb into ft_associate, then to netdev? That seems 
> reasonable.
> 

Yep.  Probably need a few additional checks to make sure we don't trigger any 
offloading behavior.

Regards,
-Denis