[PATCH v3 1/4] auto-t: modify PSK-roam test to use FT failure path

[PATCH v3 1/4] auto-t: modify PSK-roam test to use FT failure path

[James Prestwood]

This adds another radio so IWD hits the FT failure path after
authentication to the first BSS fails. This causes a wiphy work
item to be rescheduled which previously was unsafe.
---
 autotests/testPSK-roam/connection_test.py     | 13 ++++---
 autotests/testPSK-roam/failed_roam_test.py    | 33 ++++++++++------
 autotests/testPSK-roam/ft-psk-ccmp-3.conf     | 39 +++++++++++++++++++
 autotests/testPSK-roam/hw.conf                |  3 +-
 .../testPSK-roam/roam_ap_disconnect_test.py   |  4 +-
 5 files changed, 74 insertions(+), 18 deletions(-)
 create mode 100644 autotests/testPSK-roam/ft-psk-ccmp-3.conf

diff --git a/autotests/testPSK-roam/connection_test.py b/autotests/testPSK-roam/connection_test.py
index 7a135e95..459c25cf 100644
--- a/autotests/testPSK-roam/connection_test.py
+++ b/autotests/testPSK-roam/connection_test.py
@@ -48,7 +48,7 @@ class Test(unittest.TestCase):
             self.rule2.enabled = True
             # Send 100 packets (to be dropped), should trigger beacon loss
             testutil.tx_packets(device.name, self.bss_hostapd[0].ifname, 100)
-            device.wait_for_event('packet-loss-roam', timeout=30)
+            device.wait_for_event('packet-loss-roam', timeout=60)
         else:
             device.roam(self.bss_hostapd[1].bssid)
 
@@ -173,19 +173,22 @@ class Test(unittest.TestCase):
         IWD.copy_to_storage('TestFT.psk')
 
         cls.bss_hostapd = [ HostapdCLI(config='ft-psk-ccmp-1.conf'),
-                            HostapdCLI(config='ft-psk-ccmp-2.conf') ]
+                            HostapdCLI(config='ft-psk-ccmp-2.conf'),
+                            HostapdCLI(config='ft-psk-ccmp-3.conf') ]
+        cls.bss_hostapd[2].disable()
+
         rad0 = hwsim.get_radio('rad0')
-        rad2 = hwsim.get_radio('rad2')
+        rad3 = hwsim.get_radio('rad3')
 
         cls.rule0 = hwsim.rules.create()
-        cls.rule0.source = rad2.addresses[0]
+        cls.rule0.source = rad3.addresses[0]
         cls.rule0.bidirectional = True
         cls.rule0.signal = -2000
         cls.rule0.prefix = 'b0'
         cls.rule0.drop = True
 
         cls.rule1 = hwsim.rules.create()
-        cls.rule1.source = rad2.addresses[0]
+        cls.rule1.source = rad3.addresses[0]
         cls.rule1.prefix = '08'
         cls.rule1.drop = True
 
diff --git a/autotests/testPSK-roam/failed_roam_test.py b/autotests/testPSK-roam/failed_roam_test.py
index 360c1de5..eda2b4e2 100644
--- a/autotests/testPSK-roam/failed_roam_test.py
+++ b/autotests/testPSK-roam/failed_roam_test.py
@@ -62,21 +62,17 @@ class Test(unittest.TestCase):
 
         self.rule0.enabled = True
 
-        device.roam(self.bss_hostapd[1].bssid)
-
-        # Roam should fail...
-        device.wait_for_event('ft-roam-failed')
+        # IWD should connect, then attempt a roam to BSS 1, which should fail...
+        device.wait_for_event('ft-roam-failed', timeout=60)
         # ... but IWD should remain connected
         self.assertTrue(device.state == DeviceState.connected)
 
         self.rule0.enabled = False
 
-        # Try again once more
-        device.roam(self.bss_hostapd[1].bssid)
-
-        self.verify_roam(wd, device, self.bss_hostapd[0], self.bss_hostapd[1])
+        # IWD should then try BSS 2, and succeed
+        self.verify_roam(wd, device, self.bss_hostapd[0], self.bss_hostapd[2])
 
-        self.bss_hostapd[1].deauthenticate(device.address)
+        self.bss_hostapd[2].deauthenticate(device.address)
         condition = 'obj.state == DeviceState.disconnected'
         wd.wait_for_object_condition(device, condition)
 
@@ -88,6 +84,7 @@ class Test(unittest.TestCase):
 
         self.rule0.enabled = False
         self.rule1.enabled = False
+        self.rule2.enabled = False
 
     @classmethod
     def setUpClass(cls):
@@ -96,14 +93,16 @@ class Test(unittest.TestCase):
         IWD.copy_to_storage('TestFT.psk')
 
         cls.bss_hostapd = [ HostapdCLI(config='ft-psk-ccmp-1.conf'),
-                            HostapdCLI(config='ft-psk-ccmp-2.conf') ]
+                            HostapdCLI(config='ft-psk-ccmp-2.conf'),
+                            HostapdCLI(config='ft-psk-ccmp-3.conf') ]
 
         cls.bss_hostapd[0].set_address('12:00:00:00:00:01')
         cls.bss_hostapd[1].set_address('12:00:00:00:00:02')
+        cls.bss_hostapd[2].set_address('12:00:00:00:00:03')
 
         # Drop Authenticate frames
         cls.rule0 = hwsim.rules.create()
-        cls.rule0.bidirectional = True
+        cls.rule0.source = hwsim.get_radio('rad1').addresses[0]
         cls.rule0.prefix = 'b0'
         cls.rule0.drop = True
 
@@ -113,6 +112,18 @@ class Test(unittest.TestCase):
         cls.rule1.prefix = 'd0'
         cls.rule1.drop = True
 
+        # Causes IWD to immediately roam away from BSS 0
+        cls.rule2 = hwsim.rules.create()
+        cls.rule2.source = hwsim.get_radio('rad0').addresses[0]
+        cls.rule2.signal = -8000
+        cls.rule2.enabled = True
+
+        # Causes IWD to first prefer BSS 1 to roam, then BSS 2.
+        cls.rule3 = hwsim.rules.create()
+        cls.rule3.source = hwsim.get_radio('rad2').addresses[0]
+        cls.rule3.signal = -7000
+        cls.rule3.enabled = True
+
     @classmethod
     def tearDownClass(cls):
         IWD.clear_storage()
diff --git a/autotests/testPSK-roam/ft-psk-ccmp-3.conf b/autotests/testPSK-roam/ft-psk-ccmp-3.conf
new file mode 100644
index 00000000..a42e175d
--- /dev/null
+++ b/autotests/testPSK-roam/ft-psk-ccmp-3.conf
@@ -0,0 +1,39 @@
+hw_mode=g
+channel=2
+ssid=TestFT
+utf8_ssid=1
+ctrl_interface=/var/run/hostapd
+
+r1_key_holder=120000000002
+nas_identifier=dummy2
+
+wpa=2
+# Can support WPA-PSK and FT-PSK (space separated list) and/or EAP at the same
+# time but we want to force FT
+wpa_key_mgmt=FT-PSK
+wpa_pairwise=CCMP
+wpa_passphrase=EasilyGuessedPassword
+ieee80211w=1
+rsn_preauth=1
+rsn_preauth_interfaces=lo
+disable_pmksa_caching=0
+# Allow PMK cache to be shared opportunistically among configured interfaces
+# and BSSes (i.e., all configurations within a single hostapd process).
+okc=1
+mobility_domain=1234
+reassociation_deadline=60000
+r0kh=12:00:00:00:00:01 dummy1 000102030405060708090a0b0c0d0e0f
+r0kh=12:00:00:00:00:02 dummy2 000102030405060708090a0b0c0d0e0f
+r1kh=12:00:00:00:00:01 00:00:00:00:00:01 000102030405060708090a0b0c0d0e0f
+r1kh=12:00:00:00:00:02 00:00:00:00:00:02 000102030405060708090a0b0c0d0e0f
+# Push mode only needed for 8021x, not PSK mode since msk already known
+pmk_r1_push=0
+# Allow locally generated FT response so we don't have to configure push/pull
+# between BSSes running as separate hostapd processes as in the test-runner
+# case.  Only works with FT-PSK, otherwise brctl needs to be installed and
+# CONFIG_BRIDGE enabled in the kernel.
+ft_psk_generate_local=1
+ft_over_ds=0
+ap_table_expiration_time=36000
+ap_table_max_size=10
+rrm_neighbor_report=1
diff --git a/autotests/testPSK-roam/hw.conf b/autotests/testPSK-roam/hw.conf
index c2b35d6e..da9e385c 100644
--- a/autotests/testPSK-roam/hw.conf
+++ b/autotests/testPSK-roam/hw.conf
@@ -1,8 +1,9 @@
 [SETUP]
-num_radios=3
+num_radios=4
 start_iwd=0
 hwsim_medium=yes
 
 [HOSTAPD]
 rad0=ft-psk-ccmp-1.conf
 rad1=ft-psk-ccmp-2.conf
+rad2=ft-psk-ccmp-3.conf
diff --git a/autotests/testPSK-roam/roam_ap_disconnect_test.py b/autotests/testPSK-roam/roam_ap_disconnect_test.py
index 416541ce..130e1702 100644
--- a/autotests/testPSK-roam/roam_ap_disconnect_test.py
+++ b/autotests/testPSK-roam/roam_ap_disconnect_test.py
@@ -70,8 +70,10 @@ class Test(unittest.TestCase):
         IWD.copy_to_storage('TestFT.psk')
 
         cls.bss_hostapd = [ HostapdCLI(config='ft-psk-ccmp-1.conf'),
-                            HostapdCLI(config='ft-psk-ccmp-2.conf') ]
+                            HostapdCLI(config='ft-psk-ccmp-2.conf'),
+                            HostapdCLI(config='ft-psk-ccmp-3.conf') ]
         cls.bss_hostapd[1].disable()
+        cls.bss_hostapd[2].disable()
 
         cls.bss_hostapd[0].set_value('ocv', '0')
         cls.bss_hostapd[0].set_value('ieee80211w', '0')
-- 
2.25.1

[PATCH v3 2/4] auto-t: increase timeout in testPSK-roam

[James Prestwood]

This was causing random failures and increasing the timeout seems
to be a lot more reliable.
---
 autotests/testPSK-roam/roam_ap_disconnect_test.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/autotests/testPSK-roam/roam_ap_disconnect_test.py b/autotests/testPSK-roam/roam_ap_disconnect_test.py
index 130e1702..b5775990 100644
--- a/autotests/testPSK-roam/roam_ap_disconnect_test.py
+++ b/autotests/testPSK-roam/roam_ap_disconnect_test.py
@@ -33,7 +33,7 @@ class Test(unittest.TestCase):
         # Since both BSS's have low signal, the roam should fail and trigger
         # another roam scan.
         device.wait_for_event('roam-scan-triggered', timeout=30)
-        device.wait_for_event('no-roam-candidates', timeout=30)
+        device.wait_for_event('no-roam-candidates', timeout=60)
 
         # Hostapd sends disconnect
         self.bss_hostapd[0].disable()
-- 
2.25.1

[PATCH v3 3/4] wiphy: make wiphy work queue reentrant

[James Prestwood]

In some situations its convenient for the same work item to be
inserted (rescheduled) while its in progress. FT for example does
this now if a roam fails. The same ft_work item gets re-inserted
which, currently, is not safe to do since the item is modified
and removed once completed.

Fix this by introducing wiphy_radio_work_reschedule which is an
explicit API for re-inserting work items from within the do_work
callback.

The wiphy work logic was changed around slightly to remove the item
at the head of the queue prior to starting and note the ID going
into do_work. If do_work signaled done and ID changed we know it
was re-inserted and can skip the destroy logic and move onto the
next item. If the item is not done continue as normal but set the
priority to INT_MIN, as usual, to prevent other items from getting
to the head of the queue.
---
 src/wiphy.c | 45 +++++++++++++++++++++++++++++++++++++--------
 src/wiphy.h |  2 ++
 2 files changed, 39 insertions(+), 8 deletions(-)

v3:
 * Reworked the logic to simplify as well as added an explicit
   API for work rescheduling to make it clear to the caller.

diff --git a/src/wiphy.c b/src/wiphy.c
index 2db2d2cd..d06b2447 100644
--- a/src/wiphy.c
+++ b/src/wiphy.c
@@ -2574,16 +2574,13 @@ static void wiphy_radio_work_next(struct wiphy *wiphy)
 {
 	struct wiphy_radio_work_item *work;
 	bool done;
+	uint32_t id;
 
-	work = l_queue_peek_head(wiphy->work);
+	work = l_queue_pop_head(wiphy->work);
 	if (!work)
 		return;
 
-	/*
-	 * Ensures no other work item will get inserted before this one while
-	 * the work is being done.
-	 */
-	work->priority = INT_MIN;
+	id = work->id;
 
 	l_debug("Starting work item %u", work->id);
 
@@ -2592,15 +2589,25 @@ static void wiphy_radio_work_next(struct wiphy *wiphy)
 	wiphy->work_in_callback = false;
 
 	if (done) {
-		work->id = 0;
+		/* Item was rescheduled, don't destroy */
+		if (work->id != id)
+			goto next;
 
-		l_queue_remove(wiphy->work, work);
+		work->id = 0;
 
 		wiphy->work_in_callback = true;
 		destroy_work(work);
 		wiphy->work_in_callback = false;
 
+next:
 		wiphy_radio_work_next(wiphy);
+	} else {
+		/*
+		 * Ensures no other work item will get inserted before this one
+		 * while the work is being done.
+		 */
+		work->priority = INT_MIN;
+		l_queue_push_head(wiphy->work, work);
 	}
 }
 
@@ -2684,6 +2691,28 @@ int wiphy_radio_work_is_running(struct wiphy *wiphy, uint32_t id)
 	return item == l_queue_peek_head(wiphy->work) ? 1 : 0;
 }
 
+uint32_t wiphy_radio_work_reschedule(struct wiphy *wiphy,
+					struct wiphy_radio_work_item *item)
+{
+	/*
+	 * This should only be called from within the do_work callback, meaning
+	 * the item should not be in the queue. Any re-insertion on a running
+	 * item after do_work is not allowed.
+	 */
+	if (L_WARN_ON(wiphy_radio_work_is_running(wiphy, item->id) != -ENOENT))
+		return 0;
+
+	work_ids++;
+
+	l_debug("Rescheduling work item %u, new id %u", item->id, work_ids);
+
+	item->id = work_ids;
+
+	l_queue_insert(wiphy->work, item, insert_by_priority, NULL);
+
+	return item->id;
+}
+
 static int wiphy_init(void)
 {
 	struct l_genl *genl = iwd_get_genl();
diff --git a/src/wiphy.h b/src/wiphy.h
index d5d1cc8f..f4f205ad 100644
--- a/src/wiphy.h
+++ b/src/wiphy.h
@@ -168,3 +168,5 @@ uint32_t wiphy_radio_work_insert(struct wiphy *wiphy,
 				const struct wiphy_radio_work_item_ops *ops);
 void wiphy_radio_work_done(struct wiphy *wiphy, uint32_t id);
 int wiphy_radio_work_is_running(struct wiphy *wiphy, uint32_t id);
+uint32_t wiphy_radio_work_reschedule(struct wiphy *wiphy,
+					struct wiphy_radio_work_item *item);
-- 
2.25.1

[PATCH v3 4/4] station: use wiphy_radio_work_reschedule in FT path

[James Prestwood]

If FT fails the work item needs to be rescheduled rather than
inserted.
---
 src/station.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/station.c b/src/station.c
index be571083..f830ab7a 100644
--- a/src/station.c
+++ b/src/station.c
@@ -2330,8 +2330,12 @@ static bool station_fast_transition(struct station *station,
 	ft_authenticate(netdev_get_ifindex(station->netdev), bss);
 
 done:
-	wiphy_radio_work_insert(station->wiphy, &station->ft_work,
-				WIPHY_WORK_PRIORITY_CONNECT, &ft_work_ops);
+	if (station->ft_work.id)
+		wiphy_radio_work_reschedule(station->wiphy, &station->ft_work);
+	else
+		wiphy_radio_work_insert(station->wiphy, &station->ft_work,
+					WIPHY_WORK_PRIORITY_CONNECT,
+					&ft_work_ops);
 
 	return true;
 }
-- 
2.25.1

Re: [PATCH v3 1/4] auto-t: modify PSK-roam test to use FT failure path

[Denis Kenzior]

Hi James,

On 5/18/23 11:49, James Prestwood wrote:
> This adds another radio so IWD hits the FT failure path after
> authentication to the first BSS fails. This causes a wiphy work
> item to be rescheduled which previously was unsafe.
> ---
>   autotests/testPSK-roam/connection_test.py     | 13 ++++---
>   autotests/testPSK-roam/failed_roam_test.py    | 33 ++++++++++------
>   autotests/testPSK-roam/ft-psk-ccmp-3.conf     | 39 +++++++++++++++++++
>   autotests/testPSK-roam/hw.conf                |  3 +-
>   .../testPSK-roam/roam_ap_disconnect_test.py   |  4 +-
>   5 files changed, 74 insertions(+), 18 deletions(-)
>   create mode 100644 autotests/testPSK-roam/ft-psk-ccmp-3.conf
> 

All applied, thanks.

Regards,
-Denis