[iptables PATCH 00/12] Speed up iptables-tests.py

[iptables PATCH 00/12] Speed up iptables-tests.py

[Phil Sutter]

I had this in mind for a while now and finally got around to do it: When
testing an extensions/*.t file with iptables-tests.py, act in a "batch"
mode applying all rules at once and checking the expected output in one
go, thereby reducing the overhead per test file to a single
iptables-restore and iptables-save call each. This was a bit optimistic,
but the result is still significant - on my rather slow testing VM, a
full iptables-tests.py run completes in ~7min instead of ~30min (yes,
it's slow).

See patch 1 for the implementation details. As a side-effect, rule
existence checking became much stricter, so the remaining patches in
this series deal with eliminating those differences:

* Patch 2 avoids having to add '-j CONTINUE' to almost all ebtables
  rules.
* Patches 3-10 adjust expected output to reality, mostly adding content
  the script didn't care about since the old 'output.find(<rule>)'
  worked fine as long as the output *started* like '<rule>'.
* Patch 11 Changes output by omitting an obvious default value, so a
  real functional change.
* Patch 12 drops another default value (from NFQUEUE target) I'm not
  sure we should keep.

So patches are roughly sorted by my confidence in correctness. Please
have (at least) a close look at the last two, I don't want to break
iptables for anyone just to keep test files small.

Phil Sutter (12):
  tests: iptables-test: Implement fast test mode
  tests: iptables-test: Cover for obligatory -j CONTINUE in ebtables
  tests: *.t: Fix expected output for simple calls
  tests: *.t: Fix for hexadecimal output
  tests: libebt_redirect.t: Plain redirect prints with trailing
    whitespace
  tests: libxt_length.t: Fix odd use-case output
  tests: libxt_recent.t: Add missing default values
  tests: libxt_tos.t, libxt_TOS.t: Add missing masks in output
  tests: libebt_vlan.t: Drop trailing whitespace from rules
  tests: libxt_connlimit.t: Add missing --connlimit-saddr
  extensions: Do not print all-one's netmasks
  extensions: NFQUEUE: Do not print default queue number 0

 extensions/libebt_log.t      |   2 +-
 extensions/libebt_nflog.t    |   2 +-
 extensions/libebt_redirect.t |   2 +-
 extensions/libebt_vlan.t     |   4 +-
 extensions/libip6t_NETMAP.c  |   2 +-
 extensions/libip6t_REJECT.t  |   2 +-
 extensions/libipt_NETMAP.c   |   2 +-
 extensions/libipt_REJECT.t   |   2 +-
 extensions/libxt_CONNMARK.c  |  32 ++++--
 extensions/libxt_CONNMARK.t  |   4 +-
 extensions/libxt_DSCP.t      |   2 +-
 extensions/libxt_MARK.c      |   4 +-
 extensions/libxt_MARK.t      |   2 +-
 extensions/libxt_NFQUEUE.c   |  27 ++---
 extensions/libxt_NFQUEUE.t   |   4 +-
 extensions/libxt_TOS.t       |  12 +--
 extensions/libxt_connlimit.c |   8 +-
 extensions/libxt_connlimit.t |  20 ++--
 extensions/libxt_connmark.t  |   4 +-
 extensions/libxt_dscp.t      |   2 +-
 extensions/libxt_length.t    |   2 +-
 extensions/libxt_mark.t      |   2 +-
 extensions/libxt_recent.c    |  45 ++++----
 extensions/libxt_recent.t    |  14 +--
 extensions/libxt_tos.t       |   8 +-
 iptables-test.py             | 200 ++++++++++++++++++++++++++++++++++-
 26 files changed, 317 insertions(+), 93 deletions(-)

-- 
2.34.1

[iptables PATCH 01/12] tests: iptables-test: Implement fast test mode

[Phil Sutter]

Implement a faster mode of operation for suitable test files:

1) Collect all rules to add and all expected output in lists
2) Any supposedly failing rules are checked immediately like in slow
   mode.
3) Create and load iptables-restore input from the list in (1)
5) Construct the expected iptables-save output from (1) and check it in
   a single search
5) If any of the steps above fail, fall back to slow mode for
   verification and detailed error analysis. Fast mode failures are not
   fatal, merely warn about them.

To keep things simple (and feasible), avoid complicated test files
involving external commands, multiple tables or variant-specific
results.

Aside from speeding up testsuite run-time, rule searching has become
more strict since EOL char is practically part of the search string.
This revealed many false positives where the expected string was
actually a substring of the printed rule.

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

diff --git a/iptables-test.py b/iptables-test.py
index b5a70e44b9e44..89220f29fe552 100755
--- a/iptables-test.py
+++ b/iptables-test.py
@@ -28,6 +28,11 @@ EBTABLES_SAVE = "ebtables-save"
 #IPTABLES_SAVE = ['xtables-save','-4']
 #IP6TABLES_SAVE = ['xtables-save','-6']
 
+IPTABLES_RESTORE = "iptables-restore"
+IP6TABLES_RESTORE = "ip6tables-restore"
+ARPTABLES_RESTORE = "arptables-restore"
+EBTABLES_RESTORE = "ebtables-restore"
+
 EXTENSIONS_PATH = "extensions"
 LOGFILE="/tmp/iptables-test.log"
 log_file = None
@@ -222,6 +227,185 @@ STDERR_IS_TTY = sys.stderr.isatty()
         return alt_res
     return res_inverse[res]
 
+def fast_run_possible(filename):
+    '''
+    Keep things simple, run only for simple test files:
+    - no external commands
+    - no multiple tables
+    - no variant-specific results
+    '''
+    table = None
+    rulecount = 0
+    for line in open(filename):
+        if line[0] in ["#", ":"] or len(line.strip()) == 0:
+            continue
+        if line[0] == "*":
+            if table or rulecount > 0:
+                return False
+            table = line.rstrip()[1:]
+        if line[0] in ["@", "%"]:
+            return False
+        if len(line.split(";")) > 3:
+            return False
+        rulecount += 1
+
+    return True
+
+def run_test_file_fast(filename, netns):
+    '''
+    Run a test file, but fast
+
+    :param filename: name of the file with the test rules
+    :param netns: network namespace to perform test run in
+    '''
+    if "libipt_" in filename:
+        iptables = IPTABLES
+        iptables_save = IPTABLES_SAVE
+        iptables_restore = IPTABLES_RESTORE
+    elif "libip6t_" in filename:
+        iptables = IP6TABLES
+        iptables_save = IP6TABLES_SAVE
+        iptables_restore = IP6TABLES_RESTORE
+    elif "libxt_"  in filename:
+        iptables = IPTABLES
+        iptables_save = IPTABLES_SAVE
+        iptables_restore = IPTABLES_RESTORE
+    elif "libarpt_" in filename:
+        # only supported with nf_tables backend
+        if EXECUTABLE != "xtables-nft-multi":
+           return 0
+        iptables = ARPTABLES
+        iptables_save = ARPTABLES_SAVE
+        iptables_restore = ARPTABLES_RESTORE
+    elif "libebt_" in filename:
+        # only supported with nf_tables backend
+        if EXECUTABLE != "xtables-nft-multi":
+           return 0
+        iptables = EBTABLES
+        iptables_save = EBTABLES_SAVE
+        iptables_restore = EBTABLES_RESTORE
+    else:
+        # default to iptables if not known prefix
+        iptables = IPTABLES
+        iptables_save = IPTABLES_SAVE
+        iptables_restore = IPTABLES_RESTORE
+
+    f = open(filename)
+
+    rules = {}
+    table = "filter"
+    chain_array = []
+    tests = 0
+
+    for lineno, line in enumerate(f):
+        if line[0] == "#" or len(line.strip()) == 0:
+            continue
+
+        if line[0] == "*":
+            table = line.rstrip()[1:]
+            continue
+
+        if line[0] == ":":
+            chain_array = line.rstrip()[1:].split(",")
+            continue
+
+        if len(chain_array) == 0:
+            return -1
+
+        tests += 1
+
+        for chain in chain_array:
+            item = line.split(";")
+            rule = chain + " " + item[0]
+
+            if item[1] == "=":
+                rule_save = chain + " " + item[0]
+            else:
+                rule_save = chain + " " + item[1]
+
+            res = item[2].rstrip()
+            if res != "OK":
+                rule = chain + " -t " + table + " " + item[0]
+                ret = run_test(iptables, rule, rule_save,
+                               res, filename, lineno + 1, netns)
+
+                if ret < 0:
+                    return -1
+                continue
+
+            if not chain in rules.keys():
+                rules[chain] = []
+            rules[chain].append((rule, rule_save))
+
+    restore_data = ["*" + table]
+    out_expect = []
+    for chain in ["PREROUTING", "INPUT", "FORWARD", "OUTPUT", "POSTROUTING"]:
+        if not chain in rules.keys():
+            continue
+        for rule in rules[chain]:
+            restore_data.append("-A " + rule[0])
+            out_expect.append("-A " + rule[1])
+    restore_data.append("COMMIT")
+
+    out_expect = "\n".join(out_expect)
+
+    # load all rules via iptables_restore
+
+    command = EXECUTABLE + " " + iptables_restore
+    if netns:
+        command = "ip netns exec " + netns + " " + command
+
+    for line in restore_data:
+        print(iptables_restore + ": " + line, file=log_file)
+
+    proc = subprocess.Popen(command, shell = True, text = True,
+                            stdin = subprocess.PIPE,
+                            stdout = subprocess.PIPE,
+                            stderr = subprocess.PIPE)
+    restore_data = "\n".join(restore_data) + "\n"
+    out, err = proc.communicate(input = restore_data)
+
+    if proc.returncode == -11:
+        reason = "iptables-restore segfaults: " + cmd
+        print_error(reason, filename, lineno)
+        return -1
+
+    if proc.returncode != 0:
+        print("%s returned %d: %s" % (iptables_restore, proc.returncode, err),
+              file=log_file)
+        return -1
+
+    # find all rules in iptables_save output
+
+    command = EXECUTABLE + " " + iptables_save
+    if netns:
+        command = "ip netns exec " + netns + " " + command
+
+    proc = subprocess.Popen(command, shell = True,
+                            stdin = subprocess.PIPE,
+                            stdout = subprocess.PIPE,
+                            stderr = subprocess.PIPE)
+    out, err = proc.communicate()
+
+    if proc.returncode == -11:
+        reason = "iptables-save segfaults: " + cmd
+        print_error(reason, filename, lineno)
+        return -1
+
+    cmd = iptables + " -F -t " + table
+    execute_cmd(cmd, filename, 0, netns)
+
+    out = out.decode('utf-8').rstrip()
+    if out.find(out_expect) < 0:
+        print("dumps differ!", file=log_file)
+        print("\n".join(["expect: " + l for l in out_expect.split("\n")]), file=log_file)
+        print("\n".join(["got: " + l
+                         for l in out.split("\n")
+                         if not l[0] in ['*', ':', '#']]),
+              file=log_file)
+        return -1
+
+    return tests
 
 def run_test_file(filename, netns):
     '''
@@ -236,6 +420,14 @@ STDERR_IS_TTY = sys.stderr.isatty()
     if not filename.endswith(".t"):
         return 0, 0
 
+    fast_failed = False
+    if fast_run_possible(filename):
+        tests = run_test_file_fast(filename, netns)
+        if tests > 0:
+            print(filename + ": " + maybe_colored('green', "OK", STDOUT_IS_TTY))
+            return tests, tests
+        fast_failed = True
+
     if "libipt_" in filename:
         iptables = IPTABLES
     elif "libip6t_" in filename:
@@ -330,7 +522,10 @@ STDERR_IS_TTY = sys.stderr.isatty()
     if netns:
         execute_cmd("ip netns del " + netns, filename)
     if total_test_passed:
-        print(filename + ": " + maybe_colored('green', "OK", STDOUT_IS_TTY))
+        suffix = ""
+        if fast_failed:
+            suffix = maybe_colored('red', " but fast mode failed!", STDOUT_IS_TTY)
+        print(filename + ": " + maybe_colored('green', "OK", STDOUT_IS_TTY) + suffix)
 
     f.close()
     return tests, passed
-- 
2.34.1

[iptables PATCH 02/12] tests: iptables-test: Cover for obligatory -j CONTINUE in ebtables

[Phil Sutter]

Unlike iptables, ebtables includes the default rule target in output.
Instead of adding it to every rule in ebtables tests, add special casing
to the testscript checking if the expected rule output contains a target
already and adding the default one if not.

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

diff --git a/iptables-test.py b/iptables-test.py
index 89220f29fe552..ac4f37c614452 100755
--- a/iptables-test.py
+++ b/iptables-test.py
@@ -323,6 +323,9 @@ STDERR_IS_TTY = sys.stderr.isatty()
             else:
                 rule_save = chain + " " + item[1]
 
+            if iptables == EBTABLES and rule_save.find('-j') < 0:
+                rule_save += " -j CONTINUE"
+
             res = item[2].rstrip()
             if res != "OK":
                 rule = chain + " -t " + table + " " + item[0]
-- 
2.34.1

[iptables PATCH 03/12] tests: *.t: Fix expected output for simple calls

[Phil Sutter]

These minimal extension uses print in more detailed form. Track this,
the output is desired.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libebt_log.t     | 2 +-
 extensions/libebt_nflog.t   | 2 +-
 extensions/libip6t_REJECT.t | 2 +-
 extensions/libipt_REJECT.t  | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/extensions/libebt_log.t b/extensions/libebt_log.t
index f7116c417b0ab..e6adbd3b44380 100644
--- a/extensions/libebt_log.t
+++ b/extensions/libebt_log.t
@@ -1,5 +1,5 @@
 :INPUT,FORWARD,OUTPUT
---log;=;OK
+--log;--log-level notice;OK
 --log-level crit;=;OK
 --log-level 1;--log-level alert;OK
 --log-level emerg --log-ip --log-arp --log-ip6;=;OK
diff --git a/extensions/libebt_nflog.t b/extensions/libebt_nflog.t
index f867df303fa95..e98d8f5fdb9d6 100644
--- a/extensions/libebt_nflog.t
+++ b/extensions/libebt_nflog.t
@@ -1,5 +1,5 @@
 :INPUT,FORWARD,OUTPUT
---nflog;=;OK
+--nflog;--nflog-group 1;OK
 --nflog-group 42;=;OK
 --nflog-range 42;--nflog-group 1 --nflog-range 42 -j CONTINUE;OK
 --nflog-threshold 100 --nflog-prefix foo;--nflog-prefix "foo" --nflog-group 1 --nflog-threshold 100 -j CONTINUE;OK
diff --git a/extensions/libip6t_REJECT.t b/extensions/libip6t_REJECT.t
index d2b337d7ebdeb..8294f0bb77b29 100644
--- a/extensions/libip6t_REJECT.t
+++ b/extensions/libip6t_REJECT.t
@@ -1,5 +1,5 @@
 :INPUT,FORWARD,OUTPUT
--j REJECT;=;OK
+-j REJECT;-j REJECT --reject-with icmp6-port-unreachable;OK
 # manpage for IPv6 variant of REJECT does not show up for some reason?
 -j REJECT --reject-with icmp6-no-route;=;OK
 -j REJECT --reject-with icmp6-adm-prohibited;=;OK
diff --git a/extensions/libipt_REJECT.t b/extensions/libipt_REJECT.t
index 5b26b1076b5b2..3f69a72994955 100644
--- a/extensions/libipt_REJECT.t
+++ b/extensions/libipt_REJECT.t
@@ -1,5 +1,5 @@
 :INPUT,FORWARD,OUTPUT
--j REJECT;=;OK
+-j REJECT;-j REJECT --reject-with icmp-port-unreachable;OK
 -j REJECT --reject-with icmp-net-unreachable;=;OK
 -j REJECT --reject-with icmp-host-unreachable;=;OK
 -j REJECT --reject-with icmp-port-unreachable;=;OK
-- 
2.34.1

[iptables PATCH 04/12] tests: *.t: Fix for hexadecimal output

[Phil Sutter]

Use hex input to avoid having to specify an expected output in trivial
cases.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_DSCP.t     | 2 +-
 extensions/libxt_MARK.t     | 2 +-
 extensions/libxt_connmark.t | 4 ++--
 extensions/libxt_dscp.t     | 2 +-
 extensions/libxt_mark.t     | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/extensions/libxt_DSCP.t b/extensions/libxt_DSCP.t
index fcc55986dbcd7..762fcd31d4d28 100644
--- a/extensions/libxt_DSCP.t
+++ b/extensions/libxt_DSCP.t
@@ -1,6 +1,6 @@
 :PREROUTING,INPUT,FORWARD,OUTPUT,POSTROUTING
 *mangle
--j DSCP --set-dscp 0;=;OK
+-j DSCP --set-dscp 0x00;=;OK
 -j DSCP --set-dscp 0x3f;=;OK
 -j DSCP --set-dscp -1;;FAIL
 -j DSCP --set-dscp 0x40;;FAIL
diff --git a/extensions/libxt_MARK.t b/extensions/libxt_MARK.t
index 9d1aa7d7d58a4..2902a14f06742 100644
--- a/extensions/libxt_MARK.t
+++ b/extensions/libxt_MARK.t
@@ -1,6 +1,6 @@
 :INPUT,FORWARD,OUTPUT
 -j MARK --set-xmark 0xfeedcafe/0xfeedcafe;=;OK
--j MARK --set-xmark 0;=;OK
+-j MARK --set-xmark 0x0;=;OK
 -j MARK --set-xmark 4294967295;-j MARK --set-xmark 0xffffffff;OK
 -j MARK --set-xmark 4294967296;;FAIL
 -j MARK --set-xmark -1;;FAIL
diff --git a/extensions/libxt_connmark.t b/extensions/libxt_connmark.t
index 4dd7d9af265a5..353970a8995f6 100644
--- a/extensions/libxt_connmark.t
+++ b/extensions/libxt_connmark.t
@@ -2,8 +2,8 @@
 *mangle
 -m connmark --mark 0xffffffff;=;OK
 -m connmark --mark 0xffffffff/0xffffffff;-m connmark --mark 0xffffffff;OK
--m connmark --mark 0xffffffff/0;=;OK
--m connmark --mark 0/0xffffffff;-m connmark --mark 0;OK
+-m connmark --mark 0xffffffff/0x0;=;OK
+-m connmark --mark 0/0xffffffff;-m connmark --mark 0x0;OK
 -m connmark --mark -1;;FAIL
 -m connmark --mark 0xfffffffff;;FAIL
 -m connmark;;FAIL
diff --git a/extensions/libxt_dscp.t b/extensions/libxt_dscp.t
index 38d7f04e18698..e010190661ae8 100644
--- a/extensions/libxt_dscp.t
+++ b/extensions/libxt_dscp.t
@@ -1,5 +1,5 @@
 :INPUT,FORWARD,OUTPUT
--m dscp --dscp 0;=;OK
+-m dscp --dscp 0x00;=;OK
 -m dscp --dscp 0x3f;=;OK
 -m dscp --dscp -1;;FAIL
 -m dscp --dscp 0x40;;FAIL
diff --git a/extensions/libxt_mark.t b/extensions/libxt_mark.t
index 7c005379f6d64..7aeb871588ca6 100644
--- a/extensions/libxt_mark.t
+++ b/extensions/libxt_mark.t
@@ -1,6 +1,6 @@
 :INPUT,FORWARD,OUTPUT
 -m mark --mark 0xfeedcafe/0xfeedcafe;=;OK
--m mark --mark 0;=;OK
+-m mark --mark 0x0;=;OK
 -m mark --mark 4294967295;-m mark --mark 0xffffffff;OK
 -m mark --mark 4294967296;;FAIL
 -m mark --mark -1;;FAIL
-- 
2.34.1

[iptables PATCH 05/12] tests: libebt_redirect.t: Plain redirect prints with trailing whitespace

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libebt_redirect.t | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extensions/libebt_redirect.t b/extensions/libebt_redirect.t
index 23858afa3b588..58492b7968153 100644
--- a/extensions/libebt_redirect.t
+++ b/extensions/libebt_redirect.t
@@ -1,4 +1,4 @@
 :PREROUTING
 *nat
--j redirect;=;OK
+-j redirect ;=;OK
 -j redirect --redirect-target RETURN;=;OK
-- 
2.34.1

[iptables PATCH 06/12] tests: libxt_length.t: Fix odd use-case output

[Phil Sutter]

Specifying the lower boundary suffixed by colon is an undocumented
feature. Explicitly printing the upper boundary in that case seems sane.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_length.t | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/extensions/libxt_length.t b/extensions/libxt_length.t
index 0b6624ee069f6..8b70fc317485c 100644
--- a/extensions/libxt_length.t
+++ b/extensions/libxt_length.t
@@ -2,7 +2,7 @@
 -m length --length 1;=;OK
 -m length --length :2;-m length --length 0:2;OK
 -m length --length 0:3;=;OK
--m length --length 4:;=;OK
+-m length --length 4:;-m length --length 4:65535;OK
 -m length --length 0:65535;=;OK
 -m length ! --length 0:65535;=;OK
 -m length --length 0:65536;;FAIL
-- 
2.34.1

[iptables PATCH 07/12] tests: libxt_recent.t: Add missing default values

[Phil Sutter]

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_recent.t | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/extensions/libxt_recent.t b/extensions/libxt_recent.t
index 9a83918ea5835..ce85b91bf9ac6 100644
--- a/extensions/libxt_recent.t
+++ b/extensions/libxt_recent.t
@@ -1,8 +1,8 @@
 :INPUT,FORWARD,OUTPUT
--m recent --set;=;OK
+-m recent --set;-m recent --set --name DEFAULT --rsource;OK
 -m recent --rcheck --hitcount 8 --name foo --mask 255.255.255.255 --rsource;=;OK
 -m recent --rcheck --hitcount 12 --name foo --mask 255.255.255.255 --rsource;=;OK
--m recent --update --rttl;=;OK
+-m recent --update --rttl;-m recent --update --rttl --name DEFAULT --mask 255.255.255.255 --rsource;OK
 -m recent --set --rttl;;FAIL
 -m recent --rcheck --hitcount 999 --name foo --mask 255.255.255.255 --rsource;;FAIL
 # nonsensical, but all should load successfully:
-- 
2.34.1

[iptables PATCH 08/12] tests: libxt_tos.t, libxt_TOS.t: Add missing masks in output

[Phil Sutter]

Mask differs between numeric --set-tos values and symbolic ones, make
sure it is covered by the tests.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_TOS.t | 12 ++++++------
 extensions/libxt_tos.t |  8 ++++----
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/extensions/libxt_TOS.t b/extensions/libxt_TOS.t
index ae8531cc582e2..9f8e33fda7116 100644
--- a/extensions/libxt_TOS.t
+++ b/extensions/libxt_TOS.t
@@ -1,15 +1,15 @@
 :PREROUTING,INPUT,FORWARD,OUTPUT,POSTROUTING
 *mangle
--j TOS --set-tos 0x1f;=;OK
+-j TOS --set-tos 0x1f;-j TOS --set-tos 0x1f/0xff;OK
 -j TOS --set-tos 0x1f/0x1f;=;OK
 # maximum TOS is 0x1f (5 bits)
 # ERROR: should fail: iptables -A PREROUTING -t mangle -j TOS --set-tos 0xff
 # -j TOS --set-tos 0xff;;FAIL
--j TOS --set-tos Minimize-Delay;-j TOS --set-tos 0x10;OK
--j TOS --set-tos Maximize-Throughput;-j TOS --set-tos 0x08;OK
--j TOS --set-tos Maximize-Reliability;-j TOS --set-tos 0x04;OK
--j TOS --set-tos Minimize-Cost;-j TOS --set-tos 0x02;OK
--j TOS --set-tos Normal-Service;-j TOS --set-tos 0x00;OK
+-j TOS --set-tos Minimize-Delay;-j TOS --set-tos 0x10/0x3f;OK
+-j TOS --set-tos Maximize-Throughput;-j TOS --set-tos 0x08/0x3f;OK
+-j TOS --set-tos Maximize-Reliability;-j TOS --set-tos 0x04/0x3f;OK
+-j TOS --set-tos Minimize-Cost;-j TOS --set-tos 0x02/0x3f;OK
+-j TOS --set-tos Normal-Service;-j TOS --set-tos 0x00/0x3f;OK
 -j TOS --and-tos 0x12;-j TOS --set-tos 0x00/0xed;OK
 -j TOS --or-tos 0x12;-j TOS --set-tos 0x12/0x12;OK
 -j TOS --xor-tos 0x12;-j TOS --set-tos 0x12/0x00;OK
diff --git a/extensions/libxt_tos.t b/extensions/libxt_tos.t
index ccbe80099bdd9..6cceeeb4adbd8 100644
--- a/extensions/libxt_tos.t
+++ b/extensions/libxt_tos.t
@@ -4,10 +4,10 @@
 -m tos --tos Maximize-Reliability;-m tos --tos 0x04/0x3f;OK
 -m tos --tos Minimize-Cost;-m tos --tos 0x02/0x3f;OK
 -m tos --tos Normal-Service;-m tos --tos 0x00/0x3f;OK
--m tos --tos 0xff;=;OK
--m tos ! --tos 0xff;=;OK
--m tos --tos 0x00;=;OK
--m tos --tos 0x0f;=;OK
+-m tos --tos 0xff;-m tos --tos 0xff/0xff;OK
+-m tos ! --tos 0xff;-m tos ! --tos 0xff/0xff;OK
+-m tos --tos 0x00;-m tos --tos 0x00/0xff;OK
+-m tos --tos 0x0f;-m tos --tos 0x0f/0xff;OK
 -m tos --tos 0x0f/0x0f;=;OK
 -m tos --tos wrong;;FAIL
 -m tos;;FAIL
-- 
2.34.1

[iptables PATCH 09/12] tests: libebt_vlan.t: Drop trailing whitespace from rules

[Phil Sutter]

Fast iptables-test.py mode is picky and it has to: Plain redirect target
prints a trailing whitespace, generally stripping the rules in test
cases won't work therefore.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libebt_vlan.t | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/extensions/libebt_vlan.t b/extensions/libebt_vlan.t
index 81c795854fca0..3ec0559953331 100644
--- a/extensions/libebt_vlan.t
+++ b/extensions/libebt_vlan.t
@@ -4,8 +4,8 @@
 -p 802_1Q --vlan-prio 1;=;OK
 -p 802_1Q --vlan-prio ! 1;=;OK
 -p 802_1Q --vlan-encap ip;-p 802_1Q --vlan-encap 0800 -j CONTINUE;OK
--p 802_1Q --vlan-encap 0800 ;=;OK
--p 802_1Q --vlan-encap ! 0800 ;=;OK
+-p 802_1Q --vlan-encap 0800;=;OK
+-p 802_1Q --vlan-encap ! 0800;=;OK
 -p 802_1Q --vlan-encap IPv6 ! --vlan-id 1;-p 802_1Q --vlan-id ! 1 --vlan-encap 86DD -j CONTINUE;OK
 -p 802_1Q --vlan-id ! 1 --vlan-encap 86DD;=;OK
 --vlan-encap ip;=;FAIL
-- 
2.34.1

[iptables PATCH 10/12] tests: libxt_connlimit.t: Add missing --connlimit-saddr

[Phil Sutter]

This default option is included in output and it is not intuitive enough
to omit it. So extend the test cases to cover for it.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_connlimit.t | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/extensions/libxt_connlimit.t b/extensions/libxt_connlimit.t
index c7ea61e95fbc8..23bba69474fed 100644
--- a/extensions/libxt_connlimit.t
+++ b/extensions/libxt_connlimit.t
@@ -1,11 +1,11 @@
 :INPUT,FORWARD,OUTPUT
--m connlimit --connlimit-upto 0;=;OK
--m connlimit --connlimit-upto 4294967295;=;OK
--m connlimit --connlimit-upto 4294967296;;FAIL
+-m connlimit --connlimit-upto 0;-m connlimit --connlimit-upto 0 --connlimit-saddr;OK
+-m connlimit --connlimit-upto 4294967295 --connlimit-saddr;=;OK
+-m connlimit --connlimit-upto 4294967296 --connlimit-saddr;;FAIL
 -m connlimit --connlimit-upto -1;;FAIL
--m connlimit --connlimit-above 0;=;OK
--m connlimit --connlimit-above 4294967295;=;OK
--m connlimit --connlimit-above 4294967296;;FAIL
+-m connlimit --connlimit-above 0;-m connlimit --connlimit-above 0 --connlimit-saddr;OK
+-m connlimit --connlimit-above 4294967295 --connlimit-saddr;=;OK
+-m connlimit --connlimit-above 4294967296 --connlimit-saddr;;FAIL
 -m connlimit --connlimit-above -1;;FAIL
 -m connlimit --connlimit-upto 1 --conlimit-above 1;;FAIL
 -m connlimit --connlimit-above 10 --connlimit-saddr;-m connlimit --connlimit-above 10 --connlimit-mask 32 --connlimit-saddr;OK
-- 
2.34.1

[iptables PATCH 11/12] extensions: Do not print all-one's netmasks

[Phil Sutter]

All one's netmasks are a trivial default, no point in printing them.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libip6t_NETMAP.c  |  2 +-
 extensions/libipt_NETMAP.c   |  2 +-
 extensions/libxt_CONNMARK.c  | 32 +++++++++++++++++--------
 extensions/libxt_CONNMARK.t  |  4 ++--
 extensions/libxt_MARK.c      |  4 +++-
 extensions/libxt_connlimit.c |  8 +++++--
 extensions/libxt_connlimit.t |  8 +++----
 extensions/libxt_recent.c    | 45 ++++++++++++++++++++----------------
 extensions/libxt_recent.t    | 12 +++++-----
 9 files changed, 70 insertions(+), 47 deletions(-)

diff --git a/extensions/libip6t_NETMAP.c b/extensions/libip6t_NETMAP.c
index 579ed04ef0058..996c4059d3f11 100644
--- a/extensions/libip6t_NETMAP.c
+++ b/extensions/libip6t_NETMAP.c
@@ -64,7 +64,7 @@ static void __NETMAP_print(const void *ip, const struct xt_entry_target *target,
 	bits = xtables_ip6mask_to_cidr(&a);
 	if (bits < 0)
 		printf("/%s", xtables_ip6addr_to_numeric(&a));
-	else
+	else if (bits < sizeof(a) * 8)
 		printf("/%d", bits);
 }
 
diff --git a/extensions/libipt_NETMAP.c b/extensions/libipt_NETMAP.c
index f30615a357821..208d831009667 100644
--- a/extensions/libipt_NETMAP.c
+++ b/extensions/libipt_NETMAP.c
@@ -76,7 +76,7 @@ static void __NETMAP_print(const void *ip, const struct xt_entry_target *target,
 	bits = netmask2bits(a.s_addr);
 	if (bits < 0)
 		printf("/%s", xtables_ipaddr_to_numeric(&a));
-	else
+	else if (bits < sizeof(a) * 8)
 		printf("/%d", bits);
 }
 
diff --git a/extensions/libxt_CONNMARK.c b/extensions/libxt_CONNMARK.c
index 21e1091386294..4fa88854f7fd6 100644
--- a/extensions/libxt_CONNMARK.c
+++ b/extensions/libxt_CONNMARK.c
@@ -496,6 +496,12 @@ static void CONNMARK_init(struct xt_entry_target *t)
 	markinfo->mask = 0xffffffffUL;
 }
 
+static void print_mask_v1(const char *pfx, __u32 mask)
+{
+	if (mask != UINT32_MAX)
+		printf("%s0x%x", pfx, mask);
+}
+
 static void
 connmark_tg_save(const void *ip, const struct xt_entry_target *target)
 {
@@ -503,15 +509,18 @@ connmark_tg_save(const void *ip, const struct xt_entry_target *target)
 
 	switch (info->mode) {
 	case XT_CONNMARK_SET:
-		printf(" --set-xmark 0x%x/0x%x", info->ctmark, info->ctmask);
+		printf(" --set-xmark 0x%x", info->ctmark);
+		print_mask_v1("/", info->ctmask);
 		break;
 	case XT_CONNMARK_SAVE:
-		printf(" --save-mark --nfmask 0x%x --ctmask 0x%x",
-		       info->nfmask, info->ctmask);
+		printf(" --save-mark");
+		print_mask_v1(" --nfmask ", info->nfmask);
+		print_mask_v1(" --ctmask ", info->ctmask);
 		break;
 	case XT_CONNMARK_RESTORE:
-		printf(" --restore-mark --nfmask 0x%x --ctmask 0x%x",
-		       info->nfmask, info->ctmask);
+		printf(" --restore-mark");
+		print_mask_v1(" --nfmask ", info->nfmask);
+		print_mask_v1(" --ctmask ", info->ctmask);
 		break;
 	default:
 		printf(" ERROR: UNKNOWN CONNMARK MODE");
@@ -527,15 +536,18 @@ connmark_tg_save_v2(const void *ip, const struct xt_entry_target *target)
 
 	switch (info->mode) {
 	case XT_CONNMARK_SET:
-		printf(" --set-xmark 0x%x/0x%x", info->ctmark, info->ctmask);
+		printf(" --set-xmark 0x%x", info->ctmark);
+		print_mask_v1("/", info->ctmask);
 		break;
 	case XT_CONNMARK_SAVE:
-		printf(" --save-mark --nfmask 0x%x --ctmask 0x%x",
-		       info->nfmask, info->ctmask);
+		printf(" --save-mark");
+		print_mask_v1(" --nfmask ", info->nfmask);
+		print_mask_v1(" --ctmask ", info->ctmask);
 		break;
 	case XT_CONNMARK_RESTORE:
-		printf(" --restore-mark --nfmask 0x%x --ctmask 0x%x",
-		       info->nfmask, info->ctmask);
+		printf(" --restore-mark");
+		print_mask_v1(" --nfmask ", info->nfmask);
+		print_mask_v1(" --ctmask ", info->ctmask);
 		break;
 	default:
 		printf(" ERROR: UNKNOWN CONNMARK MODE");
diff --git a/extensions/libxt_CONNMARK.t b/extensions/libxt_CONNMARK.t
index 79a838fefaa14..1783f7a447195 100644
--- a/extensions/libxt_CONNMARK.t
+++ b/extensions/libxt_CONNMARK.t
@@ -2,6 +2,6 @@
 *mangle
 -j CONNMARK --restore-mark;=;OK
 -j CONNMARK --save-mark;=;OK
--j CONNMARK --save-mark --nfmask 0xfffffff --ctmask 0xffffffff;-j CONNMARK --save-mark;OK
--j CONNMARK --restore-mark --nfmask 0xfffffff --ctmask 0xffffffff;-j CONNMARK --restore-mark;OK
+-j CONNMARK --save-mark --nfmask 0xffffffff --ctmask 0xffffffff;-j CONNMARK --save-mark;OK
+-j CONNMARK --restore-mark --nfmask 0xffffffff --ctmask 0xffffffff;-j CONNMARK --restore-mark;OK
 -j CONNMARK;;FAIL
diff --git a/extensions/libxt_MARK.c b/extensions/libxt_MARK.c
index 1536563d0f4c7..36355f6645fbe 100644
--- a/extensions/libxt_MARK.c
+++ b/extensions/libxt_MARK.c
@@ -242,7 +242,9 @@ static void mark_tg_save(const void *ip, const struct xt_entry_target *target)
 {
 	const struct xt_mark_tginfo2 *info = (const void *)target->data;
 
-	printf(" --set-xmark 0x%x/0x%x", info->mark, info->mask);
+	printf(" --set-xmark 0x%x", info->mark);
+	if (info->mask != 0xffffffffU)
+		printf("/0x%x", info->mask);
 }
 
 static void mark_tg_arp_save(const void *ip, const struct xt_entry_target *target)
diff --git a/extensions/libxt_connlimit.c b/extensions/libxt_connlimit.c
index 118faea560f73..00cd0ba2f852c 100644
--- a/extensions/libxt_connlimit.c
+++ b/extensions/libxt_connlimit.c
@@ -152,13 +152,15 @@ static void connlimit_print6(const void *ip,
 static void connlimit_save4(const void *ip, const struct xt_entry_match *match)
 {
 	const struct xt_connlimit_info *info = (const void *)match->data;
+	unsigned int bits = count_bits4(info->v4_mask);
 	const int revision = match->u.user.revision;
 
 	if (info->flags & XT_CONNLIMIT_INVERT)
 		printf(" --connlimit-upto %u", info->limit);
 	else
 		printf(" --connlimit-above %u", info->limit);
-	printf(" --connlimit-mask %u", count_bits4(info->v4_mask));
+	if (bits != 32)
+		printf(" --connlimit-mask %u", bits);
 	if (revision >= 1) {
 		if (info->flags & XT_CONNLIMIT_DADDR)
 			printf(" --connlimit-daddr");
@@ -170,13 +172,15 @@ static void connlimit_save4(const void *ip, const struct xt_entry_match *match)
 static void connlimit_save6(const void *ip, const struct xt_entry_match *match)
 {
 	const struct xt_connlimit_info *info = (const void *)match->data;
+	unsigned int bits = count_bits6(info->v6_mask);
 	const int revision = match->u.user.revision;
 
 	if (info->flags & XT_CONNLIMIT_INVERT)
 		printf(" --connlimit-upto %u", info->limit);
 	else
 		printf(" --connlimit-above %u", info->limit);
-	printf(" --connlimit-mask %u", count_bits6(info->v6_mask));
+	if (bits != 128)
+		printf(" --connlimit-mask %u", bits);
 	if (revision >= 1) {
 		if (info->flags & XT_CONNLIMIT_DADDR)
 			printf(" --connlimit-daddr");
diff --git a/extensions/libxt_connlimit.t b/extensions/libxt_connlimit.t
index 23bba69474fed..8495b77fd2d2c 100644
--- a/extensions/libxt_connlimit.t
+++ b/extensions/libxt_connlimit.t
@@ -8,9 +8,9 @@
 -m connlimit --connlimit-above 4294967296 --connlimit-saddr;;FAIL
 -m connlimit --connlimit-above -1;;FAIL
 -m connlimit --connlimit-upto 1 --conlimit-above 1;;FAIL
--m connlimit --connlimit-above 10 --connlimit-saddr;-m connlimit --connlimit-above 10 --connlimit-mask 32 --connlimit-saddr;OK
--m connlimit --connlimit-above 10 --connlimit-daddr;-m connlimit --connlimit-above 10 --connlimit-mask 32 --connlimit-daddr;OK
+-m connlimit --connlimit-above 10 --connlimit-saddr;=;OK
+-m connlimit --connlimit-above 10 --connlimit-daddr;=;OK
 -m connlimit --connlimit-above 10 --connlimit-saddr --connlimit-daddr;;FAIL
--m connlimit --connlimit-above 10 --connlimit-mask 32 --connlimit-saddr;=;OK
--m connlimit --connlimit-above 10 --connlimit-mask 32 --connlimit-daddr;=;OK
+-m connlimit --connlimit-above 10 --connlimit-mask 32 --connlimit-saddr;-m connlimit --connlimit-above 10 --connlimit-saddr;OK
+-m connlimit --connlimit-above 10 --connlimit-mask 32 --connlimit-daddr;-m connlimit --connlimit-above 10 --connlimit-daddr;OK
 -m connlimit;;FAIL
diff --git a/extensions/libxt_recent.c b/extensions/libxt_recent.c
index 055ae35080346..659a91ba7b707 100644
--- a/extensions/libxt_recent.c
+++ b/extensions/libxt_recent.c
@@ -176,6 +176,29 @@ static void recent_check(struct xt_fcheck_call *cb)
 			"`--update' or `--remove'");
 }
 
+static void recent_print_mask(int family, const char *prefix,
+			      const union nf_inet_addr *mask)
+{
+	static const __u32 allones[4] = { -1, -1, -1, -1 };
+
+	switch(family) {
+	case NFPROTO_IPV4:
+		if (!memcmp(&mask->in, allones, sizeof(mask->in)))
+			return;
+
+		printf(" %s %s", prefix,
+		       xtables_ipaddr_to_numeric(&mask->in));
+		break;
+	case NFPROTO_IPV6:
+		if (!memcmp(&mask->in6, allones, sizeof(mask->in6)))
+			return;
+
+		printf(" %s %s", prefix,
+		       xtables_ip6addr_to_numeric(&mask->in6));
+		break;
+	}
+}
+
 static void recent_print(const void *ip, const struct xt_entry_match *match,
                          unsigned int family)
 {
@@ -205,16 +228,7 @@ static void recent_print(const void *ip, const struct xt_entry_match *match,
 	if (info->side == XT_RECENT_DEST)
 		printf(" side: dest");
 
-	switch(family) {
-	case NFPROTO_IPV4:
-		printf(" mask: %s",
-			xtables_ipaddr_to_numeric(&info->mask.in));
-		break;
-	case NFPROTO_IPV6:
-		printf(" mask: %s",
-			xtables_ip6addr_to_numeric(&info->mask.in6));
-		break;
-	}
+	recent_print_mask(family, "mask:", &info->mask);
 }
 
 static void recent_save(const void *ip, const struct xt_entry_match *match,
@@ -241,16 +255,7 @@ static void recent_save(const void *ip, const struct xt_entry_match *match,
 		printf(" --rttl");
 	printf(" --name %s",info->name);
 
-	switch(family) {
-	case NFPROTO_IPV4:
-		printf(" --mask %s",
-			xtables_ipaddr_to_numeric(&info->mask.in));
-		break;
-	case NFPROTO_IPV6:
-		printf(" --mask %s",
-			xtables_ip6addr_to_numeric(&info->mask.in6));
-		break;
-	}
+	recent_print_mask(family, "--mask", &info->mask);
 
 	if (info->side == XT_RECENT_SOURCE)
 		printf(" --rsource");
diff --git a/extensions/libxt_recent.t b/extensions/libxt_recent.t
index ce85b91bf9ac6..bccb8cecfd924 100644
--- a/extensions/libxt_recent.t
+++ b/extensions/libxt_recent.t
@@ -1,11 +1,11 @@
 :INPUT,FORWARD,OUTPUT
 -m recent --set;-m recent --set --name DEFAULT --rsource;OK
--m recent --rcheck --hitcount 8 --name foo --mask 255.255.255.255 --rsource;=;OK
--m recent --rcheck --hitcount 12 --name foo --mask 255.255.255.255 --rsource;=;OK
--m recent --update --rttl;-m recent --update --rttl --name DEFAULT --mask 255.255.255.255 --rsource;OK
+-m recent --rcheck --hitcount 8 --name foo --mask 255.255.255.255 --rsource;-m recent --rcheck --hitcount 8 --name foo --rsource;OK
+-m recent --rcheck --hitcount 12 --name foo --mask 255.255.255.255 --rsource;-m recent --rcheck --hitcount 12 --name foo --rsource;OK
+-m recent --update --rttl;-m recent --update --rttl --name DEFAULT --rsource;OK
 -m recent --set --rttl;;FAIL
 -m recent --rcheck --hitcount 999 --name foo --mask 255.255.255.255 --rsource;;FAIL
 # nonsensical, but all should load successfully:
--m recent --rcheck --hitcount 3 --name foo --mask 255.255.255.255 --rsource -m recent --rcheck --hitcount 4 --name foo --mask 255.255.255.255 --rsource;=;OK
--m recent --rcheck --hitcount 4 --name foo --mask 255.255.255.255 --rsource -m recent --rcheck --hitcount 4 --name foo --mask 255.255.255.255 --rsource;=;OK
--m recent --rcheck --hitcount 8 --name foo --mask 255.255.255.255 --rsource -m recent --rcheck --hitcount 12 --name foo --mask 255.255.255.255 --rsource;=;OK
+-m recent --rcheck --hitcount 3 --name foo --rsource -m recent --rcheck --hitcount 4 --name foo --rsource;=;OK
+-m recent --rcheck --hitcount 4 --name foo --rsource -m recent --rcheck --hitcount 4 --name foo --rsource;=;OK
+-m recent --rcheck --hitcount 8 --name foo --rsource -m recent --rcheck --hitcount 12 --name foo --rsource;=;OK
-- 
2.34.1

[iptables PATCH 12/12] extensions: NFQUEUE: Do not print default queue number 0

[Phil Sutter]

Instead make sure it is mentioned in help output.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libxt_NFQUEUE.c | 27 +++++++++++++++------------
 extensions/libxt_NFQUEUE.t |  4 ++--
 2 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/extensions/libxt_NFQUEUE.c b/extensions/libxt_NFQUEUE.c
index fe5190789e306..9de417ae633ca 100644
--- a/extensions/libxt_NFQUEUE.c
+++ b/extensions/libxt_NFQUEUE.c
@@ -24,7 +24,7 @@ static void NFQUEUE_help(void)
 	printf(
 "NFQUEUE target options\n"
 "  --queue-num value		Send packet to QUEUE number <value>.\n"
-"  		                Valid queue numbers are 0-65535\n"
+"  		                Valid queue numbers are 0-65535 (default 0)\n"
 );
 }
 
@@ -33,7 +33,7 @@ static void NFQUEUE_help_v1(void)
 	printf(
 "NFQUEUE target options\n"
 "  --queue-num value            Send packet to QUEUE number <value>.\n"
-"                               Valid queue numbers are 0-65535\n"
+"                               Valid queue numbers are 0-65535 (default 0)\n"
 "  --queue-balance first:last	Balance flows between queues <value> to <value>.\n");
 }
 
@@ -42,7 +42,7 @@ static void NFQUEUE_help_v2(void)
 	printf(
 "NFQUEUE target options\n"
 "  --queue-num value            Send packet to QUEUE number <value>.\n"
-"                               Valid queue numbers are 0-65535\n"
+"                               Valid queue numbers are 0-65535 (default 0)\n"
 "  --queue-balance first:last   Balance flows between queues <value> to <value>.\n"
 "  --queue-bypass		Bypass Queueing if no queue instance exists.\n"
 "  --queue-cpu-fanout	Use current CPU (no hashing)\n");
@@ -53,7 +53,7 @@ static void NFQUEUE_help_v3(void)
 	printf(
 "NFQUEUE target options\n"
 "  --queue-num value            Send packet to QUEUE number <value>.\n"
-"                               Valid queue numbers are 0-65535\n"
+"                               Valid queue numbers are 0-65535 (default 0)\n"
 "  --queue-balance first:last   Balance flows between queues <value> to <value>.\n"
 "  --queue-bypass               Bypass Queueing if no queue instance exists.\n"
 "  --queue-cpu-fanout	Use current CPU (no hashing)\n");
@@ -157,7 +157,9 @@ static void NFQUEUE_print(const void *ip,
 {
 	const struct xt_NFQ_info *tinfo =
 		(const struct xt_NFQ_info *)target->data;
-	printf(" NFQUEUE num %u", tinfo->queuenum);
+
+	if (tinfo->queuenum)
+		printf(" NFQUEUE num %u", tinfo->queuenum);
 }
 
 static void NFQUEUE_print_v1(const void *ip,
@@ -169,7 +171,7 @@ static void NFQUEUE_print_v1(const void *ip,
 	if (last > 1) {
 		last += tinfo->queuenum - 1;
 		printf(" NFQUEUE balance %u:%u", tinfo->queuenum, last);
-	} else {
+	} else if (tinfo->queuenum) {
 		printf(" NFQUEUE num %u", tinfo->queuenum);
 	}
 }
@@ -183,7 +185,7 @@ static void NFQUEUE_print_v2(const void *ip,
 	if (last > 1) {
 		last += info->queuenum - 1;
 		printf(" NFQUEUE balance %u:%u", info->queuenum, last);
-	} else
+	} else if (info->queuenum)
 		printf(" NFQUEUE num %u", info->queuenum);
 
 	if (info->bypass & NFQ_FLAG_BYPASS)
@@ -199,7 +201,7 @@ static void NFQUEUE_print_v3(const void *ip,
 	if (last > 1) {
 		last += info->queuenum - 1;
 		printf(" NFQUEUE balance %u:%u", info->queuenum, last);
-	} else
+	} else if (info->queuenum)
 		printf(" NFQUEUE num %u", info->queuenum);
 
 	if (info->flags & NFQ_FLAG_BYPASS)
@@ -214,7 +216,8 @@ static void NFQUEUE_save(const void *ip, const struct xt_entry_target *target)
 	const struct xt_NFQ_info *tinfo =
 		(const struct xt_NFQ_info *)target->data;
 
-	printf(" --queue-num %u", tinfo->queuenum);
+	if (tinfo->queuenum)
+		printf(" --queue-num %u", tinfo->queuenum);
 }
 
 static void NFQUEUE_save_v1(const void *ip, const struct xt_entry_target *target)
@@ -225,7 +228,7 @@ static void NFQUEUE_save_v1(const void *ip, const struct xt_entry_target *target
 	if (last > 1) {
 		last += tinfo->queuenum - 1;
 		printf(" --queue-balance %u:%u", tinfo->queuenum, last);
-	} else {
+	} else if (tinfo->queuenum) {
 		printf(" --queue-num %u", tinfo->queuenum);
 	}
 }
@@ -238,7 +241,7 @@ static void NFQUEUE_save_v2(const void *ip, const struct xt_entry_target *target
 	if (last > 1) {
 		last += info->queuenum - 1;
 		printf(" --queue-balance %u:%u", info->queuenum, last);
-	} else
+	} else if (info->queuenum)
 		printf(" --queue-num %u", info->queuenum);
 
 	if (info->bypass & NFQ_FLAG_BYPASS)
@@ -254,7 +257,7 @@ static void NFQUEUE_save_v3(const void *ip,
 	if (last > 1) {
 		last += info->queuenum - 1;
 		printf(" --queue-balance %u:%u", info->queuenum, last);
-	} else
+	} else if (info->queuenum)
 		printf(" --queue-num %u", info->queuenum);
 
 	if (info->flags & NFQ_FLAG_BYPASS)
diff --git a/extensions/libxt_NFQUEUE.t b/extensions/libxt_NFQUEUE.t
index b51b19fd435f7..de816247ef024 100644
--- a/extensions/libxt_NFQUEUE.t
+++ b/extensions/libxt_NFQUEUE.t
@@ -1,6 +1,6 @@
 :INPUT,FORWARD,OUTPUT
 -j NFQUEUE;=;OK
--j NFQUEUE --queue-num 0;=;OK
+-j NFQUEUE --queue-num 0;-j NFQUEUE;OK
 -j NFQUEUE --queue-num 65535;=;OK
 -j NFQUEUE --queue-num 65536;;FAIL
 -j NFQUEUE --queue-num -1;;FAIL
@@ -13,4 +13,4 @@
 -j NFQUEUE --queue-balance 0:6 --queue-cpu-fanout --queue-bypass;-j NFQUEUE --queue-balance 0:6 --queue-bypass --queue-cpu-fanout;OK
 -j NFQUEUE --queue-bypass --queue-balance 0:6 --queue-cpu-fanout;-j NFQUEUE --queue-balance 0:6 --queue-bypass --queue-cpu-fanout;OK
 -j NFQUEUE --queue-balance 0:6 --queue-bypass;=;OK
--j NFQUEUE --queue-bypass;-j NFQUEUE --queue-num 0 --queue-bypass;OK
+-j NFQUEUE --queue-bypass;=;OK
-- 
2.34.1

Re: [iptables PATCH 01/12] tests: iptables-test: Implement fast test mode

[Jan Engelhardt]

On Thursday 2022-10-06 02:27, Phil Sutter wrote:

>+def run_test_file_fast(filename, netns):
>...
>+    elif "libarpt_" in filename:
>+        # only supported with nf_tables backend
>+        if EXECUTABLE != "xtables-nft-multi":
>+           return 0

Should this particular check for executable be part of fast_run_possible
instead? (Or somewhere else completely - if not running under x-n-m,
even slow mode is not possible ;)

Re: [iptables PATCH 11/12] extensions: Do not print all-one's netmasks

[Jan Engelhardt]

On Thursday 2022-10-06 02:28, Phil Sutter wrote:

>All one's netmasks are a trivial default, no point in printing them.
>
>@@ -64,7 +64,7 @@ static void __NETMAP_print(const void *ip, const struct xt_entry_target *target,
> 	bits = xtables_ip6mask_to_cidr(&a);
> 	if (bits < 0)
> 		printf("/%s", xtables_ip6addr_to_numeric(&a));
>-	else
>+	else if (bits < sizeof(a) * 8)
> 		printf("/%d", bits);

I would rather see it stay.
- iproute2 also always prints the /128 suffix
- test parsers need not special case the absence of /128

>--- a/extensions/libxt_MARK.c
>@@ -242,7 +242,9 @@ static void mark_tg_save(const void *ip, const struct xt_entry_target *target)
> {
> 	const struct xt_mark_tginfo2 *info = (const void *)target->data;
> 
>-	printf(" --set-xmark 0x%x/0x%x", info->mark, info->mask);
>+	printf(" --set-xmark 0x%x", info->mark);
>+	if (info->mask != 0xffffffffU)
>+		printf("/0x%x", info->mask);

if (info->mask != UINT32_MAX)

Re: [iptables PATCH 01/12] tests: iptables-test: Implement fast test mode

[Phil Sutter]

On Thu, Oct 06, 2022 at 08:13:44AM +0200, Jan Engelhardt wrote:
> On Thursday 2022-10-06 02:27, Phil Sutter wrote:
> 
> >+def run_test_file_fast(filename, netns):
> >...
> >+    elif "libarpt_" in filename:
> >+        # only supported with nf_tables backend
> >+        if EXECUTABLE != "xtables-nft-multi":
> >+           return 0
> 
> Should this particular check for executable be part of fast_run_possible
> instead? (Or somewhere else completely - if not running under x-n-m,
> even slow mode is not possible ;)

Ah, you caught me c'n'p programming. ;)
I'll move the run_test_file_fast() call to after the same code in
run_test_file() and pass 'iptables' variable as parameter. The -save and
-restore commands may be constructed by simply appending the suffix.

Thanks, Phil

Re: [iptables PATCH 11/12] extensions: Do not print all-one's netmasks

[Phil Sutter]

Hi,

On Thu, Oct 06, 2022 at 08:27:33AM +0200, Jan Engelhardt wrote:
> 
> On Thursday 2022-10-06 02:28, Phil Sutter wrote:
> 
> >All one's netmasks are a trivial default, no point in printing them.
> >
> >@@ -64,7 +64,7 @@ static void __NETMAP_print(const void *ip, const struct xt_entry_target *target,
> > 	bits = xtables_ip6mask_to_cidr(&a);
> > 	if (bits < 0)
> > 		printf("/%s", xtables_ip6addr_to_numeric(&a));
> >-	else
> >+	else if (bits < sizeof(a) * 8)
> > 		printf("/%d", bits);
> 
> I would rather see it stay.
> - iproute2 also always prints the /128 suffix
> - test parsers need not special case the absence of /128

I get your point. Screen-scraping is also not uncommon among iptables
users, so care has to be taken when "optimizing" output.

OTOH we're a bit inconsistent: xtables_ip(6)mask_to_numeric() explicitly
omits output if it would print "/32" or "/128".

Maybe I'll just leave the code as-is and adjust only the test cases
instead?

> >--- a/extensions/libxt_MARK.c
> >@@ -242,7 +242,9 @@ static void mark_tg_save(const void *ip, const struct xt_entry_target *target)
> > {
> > 	const struct xt_mark_tginfo2 *info = (const void *)target->data;
> > 
> >-	printf(" --set-xmark 0x%x/0x%x", info->mark, info->mask);
> >+	printf(" --set-xmark 0x%x", info->mark);
> >+	if (info->mask != 0xffffffffU)
> >+		printf("/0x%x", info->mask);
> 
> if (info->mask != UINT32_MAX)

ACK. I copied from mark_tg_print(), so that's a useful fix unrelated to
the discussion above.

Thanks, Phil

Re: [iptables PATCH 11/12] extensions: Do not print all-one's netmasks

[Jan Engelhardt]

On Thursday 2022-10-06 13:54, Phil Sutter wrote:

>> >-	else
>> >+	else if (bits < sizeof(a) * 8)
>> > 		printf("/%d", bits);
>> 
>> I would rather see it stay.
>> - iproute2 also always prints the /128 suffix
>> - test parsers need not special case the absence of /128
>>   [as in, when something reads e.g. iptables-save output]
>
>[...] OTOH we're a bit inconsistent: xtables_ip(6)mask_to_numeric() explicitly
>omits output if it would print "/32" or "/128".

We could consider just changing those two functions.
Any third-party program that interprets our output already has to deal
with a "/N" suffix in the same spot anyway.