From b5bdee492bccc721519eff36ed5b06ab707b7a1a Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Fri, 1 Nov 2024 10:36:57 -0400 Subject: [PATCH 1/5] Utils / network: typo (leading whitespace) fix Signed-off-by: Cleber Rosa --- avocado/utils/network/interfaces.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/avocado/utils/network/interfaces.py b/avocado/utils/network/interfaces.py index d0263669bf..75dc7370ed 100644 --- a/avocado/utils/network/interfaces.py +++ b/avocado/utils/network/interfaces.py @@ -821,7 +821,7 @@ def ping_flood(self, int_name, peer_ip, ping_count): returns False on ping flood failure. :rtype: boolean """ - cmd = f"ping -I {int_name} {peer_ip} -c {ping_count} -f " + cmd = f"ping -I {int_name} {peer_ip} -c {ping_count} -f" ping_process = subprocess.Popen( cmd, shell=True, From a8f40b12849cec67564ab4d456e4e351de2552c8 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Fri, 1 Nov 2024 10:36:57 -0400 Subject: [PATCH 2/5] Utils / network: improve odds of success for ping_flood() It's not possible for regular users to flood ping without a interval of less than 2 ms: ping: cannot flood, minimal interval for user must be >= 2 ms, use -i 0.002 (or higher) Given that it's a fact, let's improve the odds of this function behaving well by not causing the ping utility to simply error out in such situations. Signed-off-by: Cleber Rosa --- avocado/utils/network/interfaces.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/avocado/utils/network/interfaces.py b/avocado/utils/network/interfaces.py index 75dc7370ed..268676da3c 100644 --- a/avocado/utils/network/interfaces.py +++ b/avocado/utils/network/interfaces.py @@ -822,6 +822,9 @@ def ping_flood(self, int_name, peer_ip, ping_count): :rtype: boolean """ cmd = f"ping -I {int_name} {peer_ip} -c {ping_count} -f" + if os.getuid() != 0: + cmd = f"{cmd} -i 0.002" + print(cmd) ping_process = subprocess.Popen( cmd, shell=True, From e500af2d6154f6ca62211d555c9fa45612596eaf Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Fri, 1 Nov 2024 10:36:57 -0400 Subject: [PATCH 3/5] Utils / Network: minimize failires of ping_flood() The implementation of ping_flood() counts on the output generated by the ping command line utility. But, the way and amount of data it reads looking for the failure pattern is both too small and is susceptible to timing issues. In order to minimize those possibilities, let's send error messages (which will never contain the info this function is looking for) to another file. It also allows to use that as an improvement to the log messages. Signed-off-by: Cleber Rosa --- avocado/utils/network/interfaces.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/avocado/utils/network/interfaces.py b/avocado/utils/network/interfaces.py index 268676da3c..9fe26af770 100644 --- a/avocado/utils/network/interfaces.py +++ b/avocado/utils/network/interfaces.py @@ -829,7 +829,7 @@ def ping_flood(self, int_name, peer_ip, ping_count): cmd, shell=True, stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, + stderr=subprocess.PIPE, universal_newlines=True, ) pattern = r"\.{10}" @@ -838,8 +838,10 @@ def ping_flood(self, int_name, peer_ip, ping_count): match = re.search(pattern, char) if match: ping_process.terminate() - msg = "ping flood failed to remote machine, Please check the logs" - LOG.debug(msg) + LOG.debug( + "ping flood failed to remote machine, error output: %s", + ping_process.stderr.read(), + ) return False return True ping_process.stdout.close() From 5392a1c23d5a597da2139cc4e9738785c939cd6f Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Fri, 1 Nov 2024 10:36:57 -0400 Subject: [PATCH 4/5] Utils / network: improve coverage of ping_flood() The ping_flood() function is a bit delicate, and further improvements are needed. To make sure we introduce improvements and not regressions, let's add some basic functional tests. Signed-off-by: Cleber Rosa --- selftests/check.py | 2 +- selftests/functional/utils/network.py | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 selftests/functional/utils/network.py diff --git a/selftests/check.py b/selftests/check.py index d24009bd49..0313da8468 100755 --- a/selftests/check.py +++ b/selftests/check.py @@ -29,7 +29,7 @@ "nrunner-requirement": 28, "unit": 678, "jobs": 11, - "functional-parallel": 313, + "functional-parallel": 315, "functional-serial": 7, "optional-plugins": 0, "optional-plugins-golang": 2, diff --git a/selftests/functional/utils/network.py b/selftests/functional/utils/network.py new file mode 100644 index 0000000000..9b9ed1ffdc --- /dev/null +++ b/selftests/functional/utils/network.py @@ -0,0 +1,14 @@ +from avocado import Test +from avocado.utils.network.hosts import LocalHost +from avocado.utils.network.interfaces import NetworkInterface + + +class Interface(Test): + def setUp(self): + self.interface = NetworkInterface("lo", LocalHost("lo")) + + def test_ping_flood(self): + self.assertTrue(self.interface.ping_flood("lo", "127.0.0.1", "1")) + + def test_ping_flood_fail(self): + self.assertFalse(self.interface.ping_flood("lo", "172.16.1.1", "100")) From e69d6078d4467a89170ee809adcc9c3128f3f999 Mon Sep 17 00:00:00 2001 From: Cleber Rosa Date: Fri, 1 Nov 2024 10:36:57 -0400 Subject: [PATCH 5/5] selftests/check.py: state the suite name when checking for number of tests We have the name of the suite, so let's give it to users when printing out a possible mismatch. Signed-off-by: Cleber Rosa --- selftests/check.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/selftests/check.py b/selftests/check.py index 0313da8468..e345ec7f9b 100755 --- a/selftests/check.py +++ b/selftests/check.py @@ -864,9 +864,10 @@ def main(args): # pylint: disable=W0621 f" it has {suite.size}." ) print( - "If you made some changes into selftests please update `TEST_SIZE`" - " variable in `check.py`. If you haven't done any changes to" - " selftests this behavior is an ERROR, and it needs to be fixed." + f"If you made some changes into selftests please update the " + f"value for the `{suite.name}` key in the `TEST_SIZE` " + f"dictionary in `check.py`. If you haven't done any changes to " + f"selftests this behavior is an ERROR, and it needs to be fixed." ) # tmp dirs clean up check