[iptables PATCH 0/4] tests: iptables-test: Test both variants by default

[iptables PATCH 0/4] tests: iptables-test: Test both variants by default

[Phil Sutter]

First three patches contain minor code refactoring for simplification.
Fourth patch changes default behaviour to run for both variants and
print a total summary, similar to shell testsuite.

Phil Sutter (4):
  tests: iptables-test: Simplify '-N' option a bit
  tests: iptables-test: Simplify execute_cmd() calling
  tests: iptables-test: Pass netns to execute_cmd()
  tests: iptables-test: Test both variants by default

 iptables-test.py | 135 ++++++++++++++++++++++++++---------------------
 1 file changed, 74 insertions(+), 61 deletions(-)

-- 
2.34.1

[iptables PATCH 1/4] tests: iptables-test: Simplify '-N' option a bit

[Phil Sutter]

Instead of hard-coding, store the netns name in args.netns if the flag
was given. The value defaults to None, so existing 'if netns' checks are
still valid.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables-test.py | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/iptables-test.py b/iptables-test.py
index 6acaa82228fa3..69c96b79927b5 100755
--- a/iptables-test.py
+++ b/iptables-test.py
@@ -79,12 +79,13 @@ def run_test(iptables, rule, rule_save, res, filename, lineno, netns):
     :param res: expected result of the rule. Valid values: "OK", "FAIL"
     :param filename: name of the file tested (used for print_error purposes)
     :param lineno: line number being tested (used for print_error purposes)
+    :param netns: network namespace to call commands in (or None)
     '''
     ret = 0
 
     cmd = iptables + " -A " + rule
     if netns:
-            cmd = "ip netns exec ____iptables-container-test " + EXECUTABLE + " " + cmd
+            cmd = "ip netns exec " + netns + " " + EXECUTABLE + " " + cmd
 
     ret = execute_cmd(cmd, filename, lineno)
 
@@ -126,7 +127,7 @@ def run_test(iptables, rule, rule_save, res, filename, lineno, netns):
     command = EXECUTABLE + " " + command
 
     if netns:
-            command = "ip netns exec ____iptables-container-test " + command
+            command = "ip netns exec " + netns + " " + command
 
     args = tokens[1:]
     proc = subprocess.Popen(command, shell=True,
@@ -226,6 +227,7 @@ def run_test_file(filename, netns):
     Runs a test file
 
     :param filename: name of the file with the test rules
+    :param netns: network namespace to perform test run in
     '''
     #
     # if this is not a test file, skip.
@@ -262,7 +264,7 @@ def run_test_file(filename, netns):
     total_test_passed = True
 
     if netns:
-        execute_cmd("ip netns add ____iptables-container-test", filename, 0)
+        execute_cmd("ip netns add " + netns, filename, 0)
 
     for lineno, line in enumerate(f):
         if line[0] == "#" or len(line.strip()) == 0:
@@ -276,7 +278,7 @@ def run_test_file(filename, netns):
         if line[0] == "@":
             external_cmd = line.rstrip()[1:]
             if netns:
-                external_cmd = "ip netns exec ____iptables-container-test " + external_cmd
+                external_cmd = "ip netns exec " + netns + " " + external_cmd
             execute_cmd(external_cmd, filename, lineno)
             continue
 
@@ -284,7 +286,7 @@ def run_test_file(filename, netns):
         if line[0] == "%":
             external_cmd = line.rstrip()[1:]
             if netns:
-                external_cmd = "ip netns exec ____iptables-container-test " + EXECUTABLE + " " + external_cmd
+                external_cmd = "ip netns exec " + netns + " " + EXECUTABLE + " " + external_cmd
             execute_cmd(external_cmd, filename, lineno)
             continue
 
@@ -334,7 +336,7 @@ def run_test_file(filename, netns):
             passed += 1
 
     if netns:
-        execute_cmd("ip netns del ____iptables-container-test", filename, 0)
+        execute_cmd("ip netns del " + netns, filename, 0)
     if total_test_passed:
         print(filename + ": " + maybe_colored('green', "OK", STDOUT_IS_TTY))
 
@@ -400,7 +402,8 @@ def main():
                         help='Check for missing tests')
     parser.add_argument('-n', '--nftables', action='store_true',
                         help='Test iptables-over-nftables')
-    parser.add_argument('-N', '--netns', action='store_true',
+    parser.add_argument('-N', '--netns', action='store_const',
+                        const='____iptables-container-test',
                         help='Test netnamespace path')
     parser.add_argument('--no-netns', action='store_true',
                         help='Do not run testsuite in own network namespace')
-- 
2.34.1

[iptables PATCH 2/4] tests: iptables-test: Simplify execute_cmd() calling

[Phil Sutter]

Default 'lineno' parameter to zero,

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables-test.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/iptables-test.py b/iptables-test.py
index 69c96b79927b5..25561bc9ba971 100755
--- a/iptables-test.py
+++ b/iptables-test.py
@@ -168,7 +168,7 @@ def run_test(iptables, rule, rule_save, res, filename, lineno, netns):
 
     return delete_rule(iptables, rule, filename, lineno)
 
-def execute_cmd(cmd, filename, lineno):
+def execute_cmd(cmd, filename, lineno = 0):
     '''
     Executes a command, checking for segfaults and returning the command exit
     code.
@@ -264,7 +264,7 @@ def run_test_file(filename, netns):
     total_test_passed = True
 
     if netns:
-        execute_cmd("ip netns add " + netns, filename, 0)
+        execute_cmd("ip netns add " + netns, filename)
 
     for lineno, line in enumerate(f):
         if line[0] == "#" or len(line.strip()) == 0:
@@ -336,7 +336,7 @@ def run_test_file(filename, netns):
             passed += 1
 
     if netns:
-        execute_cmd("ip netns del " + netns, filename, 0)
+        execute_cmd("ip netns del " + netns, filename)
     if total_test_passed:
         print(filename + ": " + maybe_colored('green', "OK", STDOUT_IS_TTY))
 
-- 
2.34.1

[iptables PATCH 3/4] tests: iptables-test: Pass netns to execute_cmd()

[Phil Sutter]

The command to run might have to be prefixed. Once if the command is
'iptables' (or related) to define the variant, once if '-N' was given to
run the command inside the netns. Doing both prefixing inside
execute_cmd() avoids a potential conflict and thus simplifies things:
The "external command" and "external iptables call" lines become
identical in handling, there is no need for a separate prefix char
anymore.

As a side-effect, this commit also fixes for delete_rule() calls in
error case ignoring the netns value.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables-test.py | 40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/iptables-test.py b/iptables-test.py
index 25561bc9ba971..6504b231666d1 100755
--- a/iptables-test.py
+++ b/iptables-test.py
@@ -54,12 +54,12 @@ def print_error(reason, filename=None, lineno=None):
         ": line %d (%s)" % (lineno, reason), file=sys.stderr)
 
 
-def delete_rule(iptables, rule, filename, lineno):
+def delete_rule(iptables, rule, filename, lineno, netns = None):
     '''
     Removes an iptables rule
     '''
     cmd = iptables + " -D " + rule
-    ret = execute_cmd(cmd, filename, lineno)
+    ret = execute_cmd(cmd, filename, lineno, netns)
     if ret == 1:
         reason = "cannot delete: " + iptables + " -I " + rule
         print_error(reason, filename, lineno)
@@ -84,10 +84,7 @@ def run_test(iptables, rule, rule_save, res, filename, lineno, netns):
     ret = 0
 
     cmd = iptables + " -A " + rule
-    if netns:
-            cmd = "ip netns exec " + netns + " " + EXECUTABLE + " " + cmd
-
-    ret = execute_cmd(cmd, filename, lineno)
+    ret = execute_cmd(cmd, filename, lineno, netns)
 
     #
     # report failed test
@@ -104,7 +101,7 @@ def run_test(iptables, rule, rule_save, res, filename, lineno, netns):
         if res == "FAIL":
             reason = "should fail: " + cmd
             print_error(reason, filename, lineno)
-            delete_rule(iptables, rule, filename, lineno)
+            delete_rule(iptables, rule, filename, lineno, netns)
             return -1
 
     matching = 0
@@ -141,7 +138,7 @@ def run_test(iptables, rule, rule_save, res, filename, lineno, netns):
     if proc.returncode == -11:
         reason = "iptables-save segfaults: " + cmd
         print_error(reason, filename, lineno)
-        delete_rule(iptables, rule, filename, lineno)
+        delete_rule(iptables, rule, filename, lineno, netns)
         return -1
 
     # find the rule
@@ -150,7 +147,7 @@ def run_test(iptables, rule, rule_save, res, filename, lineno, netns):
         if res == "OK":
             reason = "cannot find: " + iptables + " -I " + rule
             print_error(reason, filename, lineno)
-            delete_rule(iptables, rule, filename, lineno)
+            delete_rule(iptables, rule, filename, lineno, netns)
             return -1
         else:
             # do not report this error
@@ -159,7 +156,7 @@ def run_test(iptables, rule, rule_save, res, filename, lineno, netns):
         if res != "OK":
             reason = "should not match: " + cmd
             print_error(reason, filename, lineno)
-            delete_rule(iptables, rule, filename, lineno)
+            delete_rule(iptables, rule, filename, lineno, netns)
             return -1
 
     # Test "ip netns del NETNS" path with rules in place
@@ -168,7 +165,7 @@ def run_test(iptables, rule, rule_save, res, filename, lineno, netns):
 
     return delete_rule(iptables, rule, filename, lineno)
 
-def execute_cmd(cmd, filename, lineno = 0):
+def execute_cmd(cmd, filename, lineno = 0, netns = None):
     '''
     Executes a command, checking for segfaults and returning the command exit
     code.
@@ -176,11 +173,15 @@ def execute_cmd(cmd, filename, lineno = 0):
     :param cmd: string with the command to be executed
     :param filename: name of the file tested (used for print_error purposes)
     :param lineno: line number being tested (used for print_error purposes)
+    :param netns: network namespace to run command in
     '''
     global log_file
     if cmd.startswith('iptables ') or cmd.startswith('ip6tables ') or cmd.startswith('ebtables ') or cmd.startswith('arptables '):
         cmd = EXECUTABLE + " " + cmd
 
+    if netns:
+        cmd = "ip netns exec " + netns + " " + cmd
+
     print("command: {}".format(cmd), file=log_file)
     ret = subprocess.call(cmd, shell=True, universal_newlines=True,
         stderr=subprocess.STDOUT, stdout=log_file)
@@ -274,20 +275,11 @@ def run_test_file(filename, netns):
             chain_array = line.rstrip()[1:].split(",")
             continue
 
-        # external non-iptables invocation, executed as is.
-        if line[0] == "@":
-            external_cmd = line.rstrip()[1:]
-            if netns:
-                external_cmd = "ip netns exec " + netns + " " + external_cmd
-            execute_cmd(external_cmd, filename, lineno)
-            continue
-
-        # external iptables invocation, executed as is.
-        if line[0] == "%":
+        # external command invocation, executed as is.
+        # detects iptables commands to prefix with EXECUTABLE automatically
+        if line[0] in ["@", "%"]:
             external_cmd = line.rstrip()[1:]
-            if netns:
-                external_cmd = "ip netns exec " + netns + " " + EXECUTABLE + " " + external_cmd
-            execute_cmd(external_cmd, filename, lineno)
+            execute_cmd(external_cmd, filename, lineno, netns)
             continue
 
         if line[0] == "*":
-- 
2.34.1

[iptables PATCH 4/4] tests: iptables-test: Test both variants by default

[Phil Sutter]

Via '--legacy' and '--nftables' flags one may choose the variant to
test. Change the default (none of them given) from legacy to both,
by effectively running twice. Prefix the summary line with the tested
variant for clarity and print a total count line as well.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables-test.py | 84 +++++++++++++++++++++++++++++-------------------
 1 file changed, 51 insertions(+), 33 deletions(-)

diff --git a/iptables-test.py b/iptables-test.py
index 6504b231666d1..b5a70e44b9e44 100755
--- a/iptables-test.py
+++ b/iptables-test.py
@@ -408,10 +408,13 @@ def main():
         show_missing()
         return
 
-    global EXECUTABLE
-    EXECUTABLE = "xtables-legacy-multi"
+    variants = []
+    if args.legacy:
+        variants.append("legacy")
     if args.nftables:
-        EXECUTABLE = "xtables-nft-multi"
+        variants.append("nft")
+    if len(variants) == 0:
+        variants = [ "legacy", "nft" ]
 
     if os.getuid() != 0:
         print("You need to be root to run this, sorry", file=sys.stderr)
@@ -426,36 +429,51 @@ def main():
         os.putenv("PATH", "%s/iptables:%s" % (os.path.abspath(os.path.curdir),
                                               os.getenv("PATH")))
 
-    test_files = 0
-    tests = 0
-    passed = 0
-
-    # setup global var log file
-    global log_file
-    try:
-        log_file = open(LOGFILE, 'w')
-    except IOError:
-        print("Couldn't open log file %s" % LOGFILE, file=sys.stderr)
-        return
-
-    if args.filename:
-        file_list = args.filename
-    else:
-        file_list = [os.path.join(EXTENSIONS_PATH, i)
-                     for i in os.listdir(EXTENSIONS_PATH)
-                     if i.endswith('.t')]
-        file_list.sort()
-
-    for filename in file_list:
-        file_tests, file_passed = run_test_file(filename, args.netns)
-        if file_tests:
-            tests += file_tests
-            passed += file_passed
-            test_files += 1
-
-    print("%d test files, %d unit tests, %d passed" % (test_files, tests, passed))
-    return passed - tests
-
+    total_test_files = 0
+    total_passed = 0
+    total_tests = 0
+    for variant in variants:
+        global EXECUTABLE
+        EXECUTABLE = "xtables-" + variant + "-multi"
+
+        test_files = 0
+        tests = 0
+        passed = 0
+
+        # setup global var log file
+        global log_file
+        try:
+            log_file = open(LOGFILE, 'w')
+        except IOError:
+            print("Couldn't open log file %s" % LOGFILE, file=sys.stderr)
+            return
+
+        if args.filename:
+            file_list = args.filename
+        else:
+            file_list = [os.path.join(EXTENSIONS_PATH, i)
+                         for i in os.listdir(EXTENSIONS_PATH)
+                         if i.endswith('.t')]
+            file_list.sort()
+
+        for filename in file_list:
+            file_tests, file_passed = run_test_file(filename, args.netns)
+            if file_tests:
+                tests += file_tests
+                passed += file_passed
+                test_files += 1
+
+        print("%s: %d test files, %d unit tests, %d passed"
+              % (variant, test_files, tests, passed))
+
+        total_passed += passed
+        total_tests += tests
+        total_test_files = max(total_test_files, test_files)
+
+    if len(variants) > 1:
+        print("total: %d test files, %d unit tests, %d passed"
+              % (total_test_files, total_tests, total_passed))
+    return total_passed - total_tests
 
 if __name__ == '__main__':
     sys.exit(main())
-- 
2.34.1