public inbox for iwd@lists.linux.dev
 help / color / mirror / Atom feed
* [PATCH 1/3] auto-t: make test timeout configurable
@ 2023-11-13 14:32 James Prestwood
  2023-11-13 14:32 ` [PATCH 2/3] auto-t: add stop APIs and fix some issues wpas.py James Prestwood
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: James Prestwood @ 2023-11-13 14:32 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

With the addition of DPP PKEX autotests some of the timeouts are
quite long and hit test-runners maximum timeouts. For UML we should
allow this since time-travel lets us skip idle waits. Move the test
timeout out of a global define and into the argument list so QEMU
and UML can define it differently.
---
 tools/run-tests | 12 +++++-------
 tools/runner.py |  7 +++++++
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/tools/run-tests b/tools/run-tests
index 32c09723..11ad73c0 100755
--- a/tools/run-tests
+++ b/tools/run-tests
@@ -27,8 +27,6 @@ from utils import Process, Namespace, BarChart
 config = None
 intf_id = 0
 
-TEST_MAX_TIMEOUT = 240
-
 dbus.mainloop.glib.DBusGMainLoop(set_as_default=True)
 
 def dbg(*s, **kwargs):
@@ -52,7 +50,7 @@ def exit_vm():
 			p.kill()
 
 		if config.ctx and config.ctx.results:
-			success = print_results(config.ctx.results)
+			success = print_results(config.ctx.results, int(config.ctx.args.timeout))
 
 			if config.ctx.args.result:
 				write_results(config.ctx.args.result, config.ctx.results)
@@ -917,7 +915,7 @@ def post_test(ctx, to_copy):
 		except:
 			pass
 
-def print_results(results):
+def print_results(results, max_timeout):
 	table = PrettyTable(['Test', colored('Passed', 'green'), colored('Failed', 'red'), \
 				colored('Skipped', 'cyan'), colored('Time', 'yellow')])
 
@@ -928,7 +926,7 @@ def print_results(results):
 
 	for test, result in results.items():
 
-		if result.time == TEST_MAX_TIMEOUT:
+		if result.time == max_timeout:
 			failed = "Timed out"
 			passed = "Timed out"
 		elif result.time == 0:
@@ -975,7 +973,7 @@ def run_auto_tests(ctx, args):
 			p.start()
 			# Rather than time each subtest we just time the total but
 			# mutiply the default time by the number of tests being run.
-			p.join(TEST_MAX_TIMEOUT * len(subtests))
+			p.join(int(args.timeout) * len(subtests))
 
 			if p.is_alive():
 				# Timeout
@@ -983,7 +981,7 @@ def run_auto_tests(ctx, args):
 
 				ctx.results[os.path.basename(test)] = SimpleResult(run=0,
 								failures=0, errors=0,
-								skipped=0, time=TEST_MAX_TIMEOUT)
+								skipped=0, time=int(args.timeout))
 			else:
 				ctx.results[os.path.basename(test)] = rqueue.get()
 
diff --git a/tools/runner.py b/tools/runner.py
index 004bf46d..03f44611 100644
--- a/tools/runner.py
+++ b/tools/runner.py
@@ -120,9 +120,12 @@ class RunnerCoreArgParse(ArgumentParser):
 				const=True,
 				action='store',
 				help='Use physical adapters for tests (passthrough)')
+
+		# Hidden options only meant to be passed to the kernel
 		self.add_argument('--testhome', help=SUPPRESS)
 		self.add_argument('--monitor-parent', help=SUPPRESS)
 		self.add_argument('--result-parent', help=SUPPRESS)
+		self.add_argument('--timeout', help=SUPPRESS)
 
 		# Prevent --autotest/--unittest from being used together
 		auto_unit_group = self.add_mutually_exclusive_group()
@@ -385,6 +388,8 @@ class QemuRunner(RunnerAbstract):
 
 		self._prepare_outfiles()
 
+		self.args.timeout = 240
+
 		if args.hw:
 			if os.path.isfile(args.hw):
 				hw_conf = ConfigParser()
@@ -543,6 +548,8 @@ class UmlRunner(RunnerAbstract):
 
 		self._prepare_outfiles()
 
+		self.args.timeout = 1000
+
 		kern_log = "ignore_loglevel" if "kernel" in args.verbose else "quiet"
 
 		cmd = [args.kernel, 'rootfstype=hostfs', 'ro', 'mem=256M', 'mac80211_hwsim.radios=0',
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] auto-t: add stop APIs and fix some issues wpas.py
  2023-11-13 14:32 [PATCH 1/3] auto-t: make test timeout configurable James Prestwood
@ 2023-11-13 14:32 ` James Prestwood
  2023-11-13 14:32 ` [PATCH 3/3] auto-t: get DPP PKEX test running reliably James Prestwood
  2023-11-13 15:48 ` [PATCH 1/3] auto-t: make test timeout configurable Denis Kenzior
  2 siblings, 0 replies; 4+ messages in thread
From: James Prestwood @ 2023-11-13 14:32 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

 - wait_for_event was returning a list in certain cases, not the
   event itself
 - The configurator ID was not being printed (',' instead of '%')
 - The DPP ID was not being properly waited for with PKEX
---
 autotests/util/wpas.py | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/autotests/util/wpas.py b/autotests/util/wpas.py
index eab08b43..0b127672 100644
--- a/autotests/util/wpas.py
+++ b/autotests/util/wpas.py
@@ -76,7 +76,7 @@ class Wpas:
 
         for e in self._rx_data:
             if event in e:
-                return self._rx_data
+                return e
 
         return False
 
@@ -281,7 +281,7 @@ class Wpas:
             self._dpp_conf_id = self.wait_for_result()
 
         if not uri:
-            print("DPP Configurator ID: %s", self._dpp_conf_id)
+            print("DPP Configurator ID: %s" % self._dpp_conf_id)
             return
 
         self._rx_data = []
@@ -315,6 +315,8 @@ class Wpas:
         self._rx_data = []
         self._ctrl_request(cmd)
         self._dpp_id = self.wait_for_result()
+        while not self._dpp_id.isnumeric():
+            self._dpp_id = self.wait_for_result()
 
     def dpp_pkex_add(self, code, identifier=None, version=None, initiator=False, role=None):
         cmd = f'DPP_PKEX_ADD own={self._dpp_id}'
@@ -336,10 +338,18 @@ class Wpas:
         self._rx_data = []
         self._ctrl_request(cmd)
 
+    def dpp_pkex_remove(self):
+        self._rx_data = []
+        self._ctrl_request("DPP_PKEX_REMOVE *")
+
     def dpp_listen(self, freq):
         self._rx_data = []
         self._ctrl_request(f'DPP_LISTEN {freq}')
 
+    def dpp_stop_listen(self):
+        self._rx_data = []
+        self._ctrl_request("DPP_STOP_LISTEN")
+
     def dpp_configurator_remove(self):
         self._ctrl_request('DPP_CONFIGURATOR_REMOVE *')
         self.wait_for_result()
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] auto-t: get DPP PKEX test running reliably
  2023-11-13 14:32 [PATCH 1/3] auto-t: make test timeout configurable James Prestwood
  2023-11-13 14:32 ` [PATCH 2/3] auto-t: add stop APIs and fix some issues wpas.py James Prestwood
@ 2023-11-13 14:32 ` James Prestwood
  2023-11-13 15:48 ` [PATCH 1/3] auto-t: make test timeout configurable Denis Kenzior
  2 siblings, 0 replies; 4+ messages in thread
From: James Prestwood @ 2023-11-13 14:32 UTC (permalink / raw)
  To: iwd; +Cc: James Prestwood

Several tests do not pass due to some additional changes that have
not been merged. Remove these cases and add some hardening after
discovering some unfortunate wpa_supplicant behavior.

 - Disable p2p in wpa_supplicant. With p2p enabled an extra device
   is created which starts receiving DPP frames and printing
   confusing messages.
 - Remove extra asserts which don't make sense currently. These
   will be added back later as future additions to PKEX are
   upstreamed.
 - Work around wpa_supplicant retransmit limitation. This is
   described in detail in the comment in pkex_test.py
---
 autotests/testDPP/pkex_test.py | 56 ++++++++++++++++++++++++++++------
 autotests/testDPP/wpas.conf    |  1 +
 2 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/autotests/testDPP/pkex_test.py b/autotests/testDPP/pkex_test.py
index a568e619..6c5cf054 100644
--- a/autotests/testDPP/pkex_test.py
+++ b/autotests/testDPP/pkex_test.py
@@ -21,6 +21,11 @@ class Test(unittest.TestCase):
             self.wpas.dpp_configurator_create()
             self.wpas.dpp_listen(2437)
 
+    def stop_wpas_pkex(self):
+        self.wpas.dpp_pkex_remove()
+        self.wpas.dpp_stop_listen()
+        self.wpas.dpp_configurator_remove()
+
     def start_iwd_pkex_configurator(self, device, agent=False):
         self.hapd.reload()
         self.hapd.wait_for_event('AP-ENABLED')
@@ -38,11 +43,51 @@ class Test(unittest.TestCase):
         else:
             device.dpp_pkex_configure_enrollee('secret123', identifier="test")
 
+    #
+    # WPA Supplicant has awful handling of retransmissions and no-ACK
+    # conditions. It only handles retransmitting the exchange request when
+    # there is no-ACK, which makes zero sense since its a broadcast...
+    #
+    # So, really, testing against wpa_supplicant is fragile and dependent on
+    # how the scheduling works out. If IWD doesn't ACK due to being on the
+    # next frequency or in between offchannel requests wpa_supplicant gets
+    # into a state where it thinks a PKEX session has been started (having
+    # received the exchange request) but will only accept commit-reveal
+    # frames. IWD is unaware because it never got a response so it keeps
+    # sending exchange requests which are ignored.
+    #
+    # Nevertheless we should still test against wpa_supplicant for
+    # compatibility so attempt to detect this case and restart the
+    # wpa_supplicant configurator.
+    #
+    def restart_wpas_if_needed(self):
+        i = 0
+
+        while i < 10:
+            data = self.wpas.wait_for_event("DPP-RX")
+            self.assertIn("type=7", data)
+
+            data = self.wpas.wait_for_event("DPP-TX")
+            self.assertIn("type=8", data)
+
+            data = self.wpas.wait_for_event("DPP-TX-STATUS")
+            if "result=no-ACK" in data:
+                self.stop_wpas_pkex()
+                self.start_wpas_pkex('secret123', identifier="test")
+            else:
+                return
+
+            i += 1
+
+        raise Exception("wpa_supplicant could not complete PKEX after 10 retries")
+
     def test_pkex_iwd_as_enrollee(self):
         self.start_wpas_pkex('secret123', identifier="test")
 
         self.device[0].dpp_pkex_enroll('secret123', identifier="test")
 
+        self.restart_wpas_if_needed()
+
         self.wpas.wait_for_event("DPP-AUTH-SUCCESS")
 
     def test_pkex_iwd_as_enrollee_retransmit(self):
@@ -52,6 +97,8 @@ class Test(unittest.TestCase):
 
         self.device[0].dpp_pkex_enroll('secret123', identifier="test")
 
+        self.restart_wpas_if_needed()
+
         self.wpas.wait_for_event("DPP-AUTH-SUCCESS")
 
     def test_pkex_unsupported_version(self):
@@ -121,15 +168,6 @@ class Test(unittest.TestCase):
         condition = 'obj.state == DeviceState.connected'
         self.wd.wait_for_object_condition(self.device[1], condition)
 
-        self.assertTrue(os.path.exists('/tmp/ns0/ssidCCMP.psk'))
-
-        with open('/tmp/ns0/ssidCCMP.psk') as f:
-            data = f.read()
-
-        self.assertIn("SendHostname", data)
-        self.assertIn("SharedCode=secret123", data)
-        self.assertIn("ExactConfig=true", data)
-
     def test_pkex_configurator_with_agent(self):
         self.start_iwd_pkex_configurator(self.device[0], agent=True)
 
diff --git a/autotests/testDPP/wpas.conf b/autotests/testDPP/wpas.conf
index 521fb29b..19df4350 100644
--- a/autotests/testDPP/wpas.conf
+++ b/autotests/testDPP/wpas.conf
@@ -3,3 +3,4 @@ ctrl_interface=/tmp/rad1-p2p-wpas
 update_config=0
 pmf=2
 dpp_config_processing=2
+p2p_disabled=1
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/3] auto-t: make test timeout configurable
  2023-11-13 14:32 [PATCH 1/3] auto-t: make test timeout configurable James Prestwood
  2023-11-13 14:32 ` [PATCH 2/3] auto-t: add stop APIs and fix some issues wpas.py James Prestwood
  2023-11-13 14:32 ` [PATCH 3/3] auto-t: get DPP PKEX test running reliably James Prestwood
@ 2023-11-13 15:48 ` Denis Kenzior
  2 siblings, 0 replies; 4+ messages in thread
From: Denis Kenzior @ 2023-11-13 15:48 UTC (permalink / raw)
  To: James Prestwood, iwd

Hi James,

On 11/13/23 08:32, James Prestwood wrote:
> With the addition of DPP PKEX autotests some of the timeouts are
> quite long and hit test-runners maximum timeouts. For UML we should
> allow this since time-travel lets us skip idle waits. Move the test
> timeout out of a global define and into the argument list so QEMU
> and UML can define it differently.
> ---
>   tools/run-tests | 12 +++++-------
>   tools/runner.py |  7 +++++++
>   2 files changed, 12 insertions(+), 7 deletions(-)
> 

All applied, thanks.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-11-13 15:48 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-13 14:32 [PATCH 1/3] auto-t: make test timeout configurable James Prestwood
2023-11-13 14:32 ` [PATCH 2/3] auto-t: add stop APIs and fix some issues wpas.py James Prestwood
2023-11-13 14:32 ` [PATCH 3/3] auto-t: get DPP PKEX test running reliably James Prestwood
2023-11-13 15:48 ` [PATCH 1/3] auto-t: make test timeout configurable Denis Kenzior

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox