[iptables PATCH v2 0/3] iptables-save: Avoid /etc/protocols lookups

[iptables PATCH v2 0/3] iptables-save: Avoid /etc/protocols lookups

[Phil Sutter]

V1 was a tad too simple: The revert is fine, but the (now) third patch
changes iptables-save output and thus potentially breaks test cases. To
avoid that, add patch 2 which enables "dccp" and "ipcomp" protocol names
in output. Apart from that, a single shell test case expected '-p gre'
in the dump. Replace the actually printed '-p 47' using sed in there.

Phil Sutter (3):
  Revert "xshared: Print protocol numbers if --numeric was given"
  libxtables: Add dccp and ipcomp to xtables_chain_protos
  iptables-save: Avoid /etc/protocols lookups

 .../shell/testcases/ip6tables/0002-verbose-output_0    | 10 +++++-----
 .../testcases/ipt-restore/0011-noflush-empty-line_0    |  2 +-
 .../tests/shell/testcases/ipt-save/0001load-dumps_0    |  1 +
 .../shell/testcases/iptables/0002-verbose-output_0     |  4 ++--
 iptables/xshared.c                                     |  8 ++++----
 libxtables/xtables.c                                   |  2 ++
 6 files changed, 15 insertions(+), 12 deletions(-)

-- 
2.43.0

[iptables PATCH v2 1/3] Revert "xshared: Print protocol numbers if --numeric was given"

[Phil Sutter]

This reverts commit da8ecc62dd765b15df84c3aa6b83dcb7a81d4ffa.

The patch's original intention is not entirely clear anymore. If it was
to reduce delays involved by calling getprotobynumber() though, commit
b6196c7504d4d ("xshared: Prefer xtables_chain_protos lookup over
getprotoent") avoids those if --numeric flag was given already. Also,
this numeric protocol output did not cover iptables-save which is a more
relevant candidate for such optimizations anyway.

Closes: https://bugzilla.netfilter.org/show_bug.cgi?id=1729
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 .../shell/testcases/ip6tables/0002-verbose-output_0    | 10 +++++-----
 .../testcases/ipt-restore/0011-noflush-empty-line_0    |  2 +-
 .../shell/testcases/iptables/0002-verbose-output_0     |  4 ++--
 iptables/xshared.c                                     |  6 +++---
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/iptables/tests/shell/testcases/ip6tables/0002-verbose-output_0 b/iptables/tests/shell/testcases/ip6tables/0002-verbose-output_0
index cc18a94b96986..45fab83026cb6 100755
--- a/iptables/tests/shell/testcases/ip6tables/0002-verbose-output_0
+++ b/iptables/tests/shell/testcases/ip6tables/0002-verbose-output_0
@@ -33,11 +33,11 @@ EXPECT='Chain INPUT (policy ACCEPT 0 packets, 0 bytes)
 
 Chain FORWARD (policy ACCEPT 0 packets, 0 bytes)
  pkts bytes target     prot opt in     out     source               destination
-    0     0 ACCEPT     0    --  eth2   eth3    feed:babe::1         feed:babe::2
-    0     0 ACCEPT     0    --  eth2   eth3    feed:babe::4         feed:babe::5
-    0     0            58   --  *      *       ::/0                 ::/0                 ipv6-icmptype 1 code 0
-    0     0            0    --  *      *       ::/0                 ::/0                 dst length:42 rt type:23
-    0     0 LOG        0    --  *      *       ::/0                 ::/0                 frag id:1337 LOG flags 0 level 4
+    0     0 ACCEPT     all  --  eth2   eth3    feed:babe::1         feed:babe::2
+    0     0 ACCEPT     all  --  eth2   eth3    feed:babe::4         feed:babe::5
+    0     0            ipv6-icmp --  *      *       ::/0                 ::/0                 ipv6-icmptype 1 code 0
+    0     0            all  --  *      *       ::/0                 ::/0                 dst length:42 rt type:23
+    0     0 LOG        all  --  *      *       ::/0                 ::/0                 frag id:1337 LOG flags 0 level 4
 
 Chain OUTPUT (policy ACCEPT 0 packets, 0 bytes)
  pkts bytes target     prot opt in     out     source               destination'
diff --git a/iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0 b/iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0
index 1a3af46fc4756..bea1a690bb624 100755
--- a/iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0
+++ b/iptables/tests/shell/testcases/ipt-restore/0011-noflush-empty-line_0
@@ -12,5 +12,5 @@ EOF
 
 EXPECT='Chain FORWARD (policy ACCEPT)
 target     prot opt source               destination         
-ACCEPT     0    --  0.0.0.0/0            0.0.0.0/0           '
+ACCEPT     all  --  0.0.0.0/0            0.0.0.0/0           '
 diff -u <(echo "$EXPECT") <($XT_MULTI iptables -n -L FORWARD)
diff --git a/iptables/tests/shell/testcases/iptables/0002-verbose-output_0 b/iptables/tests/shell/testcases/iptables/0002-verbose-output_0
index 15c72af309186..5d2af4c8d2ab2 100755
--- a/iptables/tests/shell/testcases/iptables/0002-verbose-output_0
+++ b/iptables/tests/shell/testcases/iptables/0002-verbose-output_0
@@ -21,8 +21,8 @@ EXPECT='Chain INPUT (policy ACCEPT 0 packets, 0 bytes)
 
 Chain FORWARD (policy ACCEPT 0 packets, 0 bytes)
  pkts bytes target     prot opt in     out     source               destination
-    0     0 ACCEPT     0    --  eth2   eth3    10.0.0.1             10.0.0.2
-    0     0 ACCEPT     0    --  eth2   eth3    10.0.0.4             10.0.0.5
+    0     0 ACCEPT     all  --  eth2   eth3    10.0.0.1             10.0.0.2
+    0     0 ACCEPT     all  --  eth2   eth3    10.0.0.4             10.0.0.5
 
 Chain OUTPUT (policy ACCEPT 0 packets, 0 bytes)
  pkts bytes target     prot opt in     out     source               destination'
diff --git a/iptables/xshared.c b/iptables/xshared.c
index 43fa929df7676..68f70277388a6 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -1080,10 +1080,10 @@ void print_rule_details(unsigned int linenum, const struct xt_counters *ctrs,
 
 	fputc(invflags & XT_INV_PROTO ? '!' : ' ', stdout);
 
-	if (((format & (FMT_NUMERIC | FMT_NOTABLE)) == FMT_NUMERIC) || !pname)
-		printf(FMT("%-4hu ", "%hu "), proto);
-	else
+	if (pname)
 		printf(FMT("%-4s ", "%s "), pname);
+	else
+		printf(FMT("%-4hu ", "%hu "), proto);
 }
 
 void save_rule_details(const char *iniface, const char *outiface,
-- 
2.43.0

[iptables PATCH v2 2/3] libxtables: Add dccp and ipcomp to xtables_chain_protos

[Phil Sutter]

There are "protocol extensions" for both just like with TCP or UDP.
Caching their values allows for implicit extension lookup after '-p'
flag, for instance:

| iptables -A FORWARD -p dccp --dport 1
| iptables -A FORWARD -p ipcomp --ipcompspi 18

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 libxtables/xtables.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index b4339e8d31275..38b50f7aca93e 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -2201,6 +2201,8 @@ const struct xtables_pprot xtables_chain_protos[] = {
 	{"mobility-header", IPPROTO_MH},
 	{"ipv6-mh",   IPPROTO_MH},
 	{"mh",        IPPROTO_MH},
+	{"dccp",      IPPROTO_DCCP},
+	{"ipcomp",    IPPROTO_COMP},
 	{"all",       0},
 	{NULL},
 };
-- 
2.43.0

[iptables PATCH v2 3/3] iptables-save: Avoid /etc/protocols lookups

[Phil Sutter]

Instrument proto_to_name() to abort if given protocol number is not
among the well-known ones in xtables_chain_protos. Along with
xtables_parse_protocol() preferring said array for lookups as well, this
ensures reliable dump'n'restore regardless of /etc/protocols contents.

Another benefit is rule dump performance. A simple test-case dumping
100k rules matching on dccp protocol shows an 8s delta (2s vs. 10s for
legacy, 0.5s vs. 8s for nft) with this patch applied. For reference:

| for variant in nft legacy; do
| 	(
| 		echo "*filter"
| 		for ((i = 0; i < 100000; i++)); do
| 		        echo "-A FORWARD -p dccp -j ACCEPT"
| 		done
| 		echo "COMMIT"
| 	) | iptables-${variant}-restore
| 	time iptables-${variant}-save | wc -l
| 	iptables-${variant} -F
| done

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/tests/shell/testcases/ipt-save/0001load-dumps_0 | 1 +
 iptables/xshared.c                                       | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/iptables/tests/shell/testcases/ipt-save/0001load-dumps_0 b/iptables/tests/shell/testcases/ipt-save/0001load-dumps_0
index 4e0be51c071c8..48f5f7b423379 100755
--- a/iptables/tests/shell/testcases/ipt-save/0001load-dumps_0
+++ b/iptables/tests/shell/testcases/ipt-save/0001load-dumps_0
@@ -39,6 +39,7 @@ do_simple()
 
 	$XT_MULTI ${iptables}-restore < "$dumpfile"
 	$XT_MULTI ${iptables}-save | grep -v "^#" > "$tmpfile"
+	sed -i -e 's/-p 47 /-p gre /' "$tmpfile"
 	do_diff $dumpfile "$tmpfile"
 	if [ $? -ne 0 ]; then
 		# cp "$tmpfile" "$dumpfile.got"
diff --git a/iptables/xshared.c b/iptables/xshared.c
index 68f70277388a6..51283d7afad1e 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -1097,7 +1097,7 @@ void save_rule_details(const char *iniface, const char *outiface,
 	}
 
 	if (proto > 0) {
-		const char *pname = proto_to_name(proto, 0);
+		const char *pname = proto_to_name(proto, true);
 
 		if (invflags & XT_INV_PROTO)
 			printf(" !");
-- 
2.43.0

Re: [iptables PATCH v2 0/3] iptables-save: Avoid /etc/protocols lookups

[Phil Sutter]

On Wed, Jan 10, 2024 at 11:41:33PM +0100, Phil Sutter wrote:
> V1 was a tad too simple: The revert is fine, but the (now) third patch
> changes iptables-save output and thus potentially breaks test cases. To
> avoid that, add patch 2 which enables "dccp" and "ipcomp" protocol names
> in output. Apart from that, a single shell test case expected '-p gre'
> in the dump. Replace the actually printed '-p 47' using sed in there.
> 
> Phil Sutter (3):
>   Revert "xshared: Print protocol numbers if --numeric was given"
>   libxtables: Add dccp and ipcomp to xtables_chain_protos
>   iptables-save: Avoid /etc/protocols lookups

Series applied.