[PATCH 1/3] autotests: Fix testNetconfig ACD test

[PATCH 1/3] autotests: Fix testNetconfig ACD test

[Andrew Zaborowski]

Part of static_test.py starts a second IWD instance and tries to make
it connect to the AP with the same IP address as the first IWD instance
which is already connected, to produce an IP conflict.  For this, the
second instance uses DHCP and the test expects the DHCP server to offer
the address 192.168.1.10 to it.  However in the current setup the DHCP
server manages to detect that 192.168.1.10 is in use and offers .11
instead.  Break the DHCP server's conflict detection by disabling ICMP
ping replies in order to fix the test.

Previously this has worked because the AP's and the DHCP server's
network interface is in the same network namespace as the first IWD
instance's network interface meaning that pings between the two
interfaces shouldn't work (a known Linux kernel routing quirk...).
I am not sure why those pings currently do work but take no chances and
disable ICMP pings.
---
 autotests/testNetconfig/static_test.py | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/autotests/testNetconfig/static_test.py b/autotests/testNetconfig/static_test.py
index 01d694ca..4ce279f2 100644
--- a/autotests/testNetconfig/static_test.py
+++ b/autotests/testNetconfig/static_test.py
@@ -85,11 +85,16 @@ class Test(unittest.TestCase):
         condition = 'not obj.connected'
         wd_ns0.wait_for_object_condition(ordered_network.network_object, condition)
 
-        # Connect to the same network from a dynamically configured client.  The
-        # DHCP server doesn't know (even though dev1 announced itself) that
-        # 192.168.1.10 is already in use and if it assigns dev2 the lowest
-        # available address, that's going to be 192.168.1.10.  dev1's ACD
-        # implementation should then stop using this address.
+        # Connect to the same network from a dynamically configured client.  We
+        # block ICMP pings so that the DHCP server can't confirm that
+        # 192.168.1.10 is in use by dev1 and if it assigns dev2 the lowest
+        # available address, that's going to be 192.168.1.10.  We also keep the
+        # second client's netdev in a separate namespace so that the kernel
+        # lets us assign the same IP.  dev1's ACD implementation should then
+        # stop using this address.  Yes, a quite unrealistic scenario but this
+        # lets us test our reaction to a conflict appearing after successful
+        # initial setup.
+        os.system("echo 1 > /proc/sys/net/ipv4/icmp_echo_ignore_all")
         ordered_network.network_object.connect()
 
         condition = 'obj.state == DeviceState.connected'
-- 
2.34.1

[PATCH 2/3] autotests: Fix class variables that should be object vars

[Andrew Zaborowski]

Due to those variables being global (IWD class variables) calling either
unregister_psk_agent or del on one IWD class instance would unregister
all agents on all instances.  Move .psk_agents and two other class
variables to the object.  They were already referenced using "self."
as if they were object variables throughout the class.
---
 autotests/util/iwd.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/autotests/util/iwd.py b/autotests/util/iwd.py
index b17fe163..5f5699df 100755
--- a/autotests/util/iwd.py
+++ b/autotests/util/iwd.py
@@ -1091,17 +1091,15 @@ class IWD(AsyncOpAbstract):
         some tests do require starting IWD using this constructor (by passing
         start_iwd_daemon=True)
     '''
-    _object_manager_if = None
-    _iwd_proc = None
-    _devices = None
     _default_instance = None
-    psk_agents = []
 
     def __init__(self, start_iwd_daemon = False, iwd_config_dir = '/tmp',
                             iwd_storage_dir = IWD_STORAGE_DIR, namespace=ctx,
                             developer_mode = True):
         self.namespace = namespace
         self._bus = namespace.get_bus()
+        self._object_manager_if = None
+        self._iwd_proc = None
 
         if start_iwd_daemon:
             if self.namespace.is_process_running('iwd'):
@@ -1120,6 +1118,8 @@ class IWD(AsyncOpAbstract):
         if self.namespace.name is None:
             IWD._default_instance = weakref.ref(self)
 
+        self.psk_agents = []
+
     def __del__(self):
         for agent in self.psk_agents:
             self.unregister_psk_agent(agent)
-- 
2.34.1

[PATCH 3/3] autotests: Test ACD failure during connect

[Andrew Zaborowski]

Previously we had an ACD failure scenario where a new client forces its
IP to create an IP conflict and an already-connected client detects the
conflict and reacts.  Now first test a scenario where a newly connecting
IWD client runs ACD before setting its statically configured IP, detects
a conflict and refuses to continue, then run the second scenario where
the newly connecting DHCP-configured client ignores the conflict and
starts ACD in defend-indefinitely mode and the older client in
defent-once mode gives up its IP.
---
 autotests/testNetconfig/static_test.py | 83 +++++++++++++++++++-------
 1 file changed, 61 insertions(+), 22 deletions(-)

diff --git a/autotests/testNetconfig/static_test.py b/autotests/testNetconfig/static_test.py
index 4ce279f2..94307a8c 100644
--- a/autotests/testNetconfig/static_test.py
+++ b/autotests/testNetconfig/static_test.py
@@ -13,36 +13,28 @@ import testutil
 from config import ctx
 import os
 import socket
+import gc
 
 class Test(unittest.TestCase):
 
     def test_connection_success(self):
         # Use a non-default storage_dir for one of the instances, the default for the other one
-        wd = IWD(True, iwd_storage_dir='/tmp/storage')
-
-        ns0 = ctx.get_namespace('ns0')
-
-        wd_ns0 = IWD(True, namespace=ns0)
-
-        psk_agent = PSKAgent("secret123")
-        psk_agent_ns0 = PSKAgent("secret123", namespace=ns0)
-        wd.register_psk_agent(psk_agent)
-        wd_ns0.register_psk_agent(psk_agent_ns0)
-
-        dev1 = wd.list_devices(1)[0]
-        dev2 = wd_ns0.list_devices(1)[0]
+        iwd_main = IWD(True, iwd_storage_dir='/tmp/storage-main')
+        psk_agent_main = PSKAgent("secret123")
+        iwd_main.register_psk_agent(psk_agent_main)
+        dev1 = iwd_main.list_devices(1)[0]
 
         ordered_network = dev1.get_ordered_network('ap-main')
 
         self.assertEqual(ordered_network.type, NetworkType.psk)
 
         condition = 'not obj.connected'
-        wd.wait_for_object_condition(ordered_network.network_object, condition)
+        iwd_main.wait_for_object_condition(ordered_network.network_object, condition)
 
         ordered_network.network_object.connect()
 
         condition = 'obj.state == DeviceState.connected'
-        wd.wait_for_object_condition(dev1, condition)
+        iwd_main.wait_for_object_condition(dev1, condition)
 
         testutil.test_iface_operstate()
         testutil.test_ifaces_connected()
@@ -80,10 +72,56 @@ class Test(unittest.TestCase):
         # of the log since we care about the end result here.
         self.assertEqual(expected_rclog, entries[-3:])
 
+        # Run our second client in a separate namespace to allow ACD (ARP) to
+        # work, and also be able to set identical IPs on both interfaces for
+        # the next part of this test.
+        ns0 = ctx.get_namespace('ns0')
+
+        iwd_ns0_1 = IWD(True, namespace=ns0, iwd_storage_dir='/tmp/storage-ns0-1')
+        psk_agent_ns0_1 = PSKAgent("secret123", namespace=ns0)
+        iwd_ns0_1.register_psk_agent(psk_agent_ns0_1)
+        dev2 = iwd_ns0_1.list_devices(1)[0]
+
+        ordered_network = dev2.get_ordered_network('ap-main')
+
+        condition = 'not obj.connected'
+        iwd_ns0_1.wait_for_object_condition(ordered_network.network_object, condition)
+
+        # Attempt a connection to the same AP that iwd_main is connected to
+        # using the same static config.  The new client's ACD client should
+        # detect an IP conflict and not allow the device to reach the
+        # "connected" state although the DBus .Connect call will succeed.
+        ordered_network.network_object.connect()
+        self.assertEqual(dev2.state, iwd.DeviceState.connecting)
+        try:
+            # We should either stay in "connecting" indefinitely or move to
+            # "disconnecting"
+            condition = 'obj.state != DeviceState.connecting'
+            iwd_ns0_1.wait_for_object_condition(dev2, condition, max_wait=21)
+            self.assertEqual(dev2.state, iwd.DeviceState.disconnecting)
+        except TimeoutError:
+            dev2.disconnect()
+
+        iwd_ns0_1.unregister_psk_agent(psk_agent_ns0_1)
+        del dev2
+        # Note: if any references to iwd_ns0_1 are left, the "del iwd_ns0_1"
+        # will not kill the IWD process the iwd_ns0_2 initialization will raise
+        # an exception.  The iwd_ns0_1.wait_for_object_condition() above
+        # creates a circular reference (which is not wrong in itself) and
+        # gc.collect() gets rid of it.  The actual solution is to eventually
+        # avoid executing anything important in .__del__ (which is wrong.)
+        gc.collect()
+        del iwd_ns0_1
+
+        iwd_ns0_2 = IWD(True, namespace=ns0, iwd_storage_dir='/tmp/storage-ns0-2')
+        psk_agent_ns0_2 = PSKAgent("secret123", namespace=ns0)
+        iwd_ns0_2.register_psk_agent(psk_agent_ns0_2)
+        dev2 = iwd_ns0_2.list_devices(1)[0]
+
         ordered_network = dev2.get_ordered_network('ap-main')
 
         condition = 'not obj.connected'
-        wd_ns0.wait_for_object_condition(ordered_network.network_object, condition)
+        iwd_ns0_2.wait_for_object_condition(ordered_network.network_object, condition)
 
         # Connect to the same network from a dynamically configured client.  We
         # block ICMP pings so that the DHCP server can't confirm that
@@ -98,9 +136,9 @@ class Test(unittest.TestCase):
         ordered_network.network_object.connect()
 
         condition = 'obj.state == DeviceState.connected'
-        wd_ns0.wait_for_object_condition(dev2, condition)
+        iwd_ns0_2.wait_for_object_condition(dev2, condition)
 
-        wd.wait(1)
+        iwd_main.wait(1)
         # Check dev1 is now disconnected or without its IPv4 address
         if dev1.state == iwd.DeviceState.connected:
             testutil.test_ip_address_match(dev1.name, None)
@@ -109,9 +147,9 @@ class Test(unittest.TestCase):
         dev2.disconnect()
 
         condition = 'not obj.connected'
-        wd.wait_for_object_condition(ordered_network.network_object, condition)
+        iwd_main.wait_for_object_condition(ordered_network.network_object, condition)
 
-        wd.unregister_psk_agent(psk_agent)
+        iwd_main.unregister_psk_agent(psk_agent_main)
 
     @classmethod
     def setUpClass(cls):
@@ -132,7 +170,8 @@ class Test(unittest.TestCase):
         cls.dhcpd_pid = ctx.start_process(['dhcpd', '-f', '-cf', '/tmp/dhcpd.conf',
                                             '-lf', '/tmp/dhcpd.leases',
                                             hapd.ifname], cleanup=remove_lease)
-        IWD.copy_to_storage('static.psk', '/tmp/storage', 'ap-main.psk')
+        IWD.copy_to_storage('static.psk', '/tmp/storage-main', 'ap-main.psk')
+        IWD.copy_to_storage('static.psk', '/tmp/storage-ns0-1', 'ap-main.psk')
 
         cls.orig_path = os.environ['PATH']
         os.environ['PATH'] = '/tmp/test-bin:' + os.environ['PATH']
@@ -141,7 +180,7 @@ class Test(unittest.TestCase):
     @classmethod
     def tearDownClass(cls):
         cls.dhcpd_pid.kill()
-        os.system('rm -rf /tmp/resolvconf.log /tmp/test-bin /tmp/storage')
+        os.system('rm -rf /tmp/resolvconf.log /tmp/test-bin /tmp/storage-*')
         os.environ['PATH'] = cls.orig_path
 
 if __name__ == '__main__':
-- 
2.34.1

Re: [PATCH 1/3] autotests: Fix testNetconfig ACD test

[Denis Kenzior]

Hi Andrew,

On 8/23/22 11:36, Andrew Zaborowski wrote:
> Part of static_test.py starts a second IWD instance and tries to make
> it connect to the AP with the same IP address as the first IWD instance
> which is already connected, to produce an IP conflict.  For this, the
> second instance uses DHCP and the test expects the DHCP server to offer
> the address 192.168.1.10 to it.  However in the current setup the DHCP
> server manages to detect that 192.168.1.10 is in use and offers .11
> instead.  Break the DHCP server's conflict detection by disabling ICMP
> ping replies in order to fix the test.
> 
> Previously this has worked because the AP's and the DHCP server's
> network interface is in the same network namespace as the first IWD
> instance's network interface meaning that pings between the two
> interfaces shouldn't work (a known Linux kernel routing quirk...).
> I am not sure why those pings currently do work but take no chances and
> disable ICMP pings.
> ---
>   autotests/testNetconfig/static_test.py | 15 ++++++++++-----
>   1 file changed, 10 insertions(+), 5 deletions(-)
> 

All applied, thanks.

Regards,
-Denis