[iptables PATCH v2 00/11] Larger xtables-save review

[iptables PATCH v2 00/11] Larger xtables-save review

[Phil Sutter]

This series started as a fix to program names mentioned in *-save
outputs and ended in merging ebtables-save and arptables-save code into
xtables_save_main used by ip{6,}tables-nft-save.

The first patch is actually unrelated but was discovered when testing
counter output - depending on environment, ebtables-nft might segfault.

The second patch fixes option '-c' of ebtables-nft-save which enables
counter prefixes in dumped rules but failed to disable the classical
ebtables-style counters.

Patch three unifies the header/footer comments in all the *-save tools
and also drops the extra newline printed in ebtables- and arptables-save
output, so test scripts need adjustments beyond dropping the new comment
lines from output.

Patch four fixes the table compatibility check in ip{6,}tables-nft-save.

Patches five and seven to nine prepare for integrating arptables- and
ebtables-save into the xtables-save code.

Patch six merely fixes a minor coding-style issue.

Patches ten and eleven finally perform the actual merge.

Changes since v1:
- Rebased onto current master branch.
- Improved commit message in patch eight.

Phil Sutter (11):
  ebtables: Fix error message for invalid parameters
  ebtables-save: Fix counter formatting
  xtables-save: Unify *-save header/footer comments
  xtables-save: Fix table compatibility check
  nft: Make nft_for_each_table() more versatile
  xtables-save: Avoid mixed code and declarations
  xtables-save: Pass optstring/longopts to xtables_save_main()
  xtables-save: Make COMMIT line optional
  xtables-save: Pass format flags to do_output()
  arptables-save: Merge into xtables_save_main()
  ebtables-save: Merge into xtables_save_main()

 iptables/nft-bridge.c                         |  39 +--
 iptables/nft.c                                |   6 +-
 iptables/nft.h                                |   2 +-
 .../arptables/0001-arptables-save-restore_0   |   7 +-
 .../0002-arptables-restore-defaults_0         |   6 +-
 .../arptables/0003-arptables-verbose-output_0 |   5 +-
 .../ebtables/0002-ebtables-save-restore_0     |   4 +-
 .../ebtables/0003-ebtables-restore-defaults_0 |   6 +-
 .../testcases/ebtables/0004-save-counters_0   |  64 +++++
 iptables/xtables-eb.c                         |   4 +-
 iptables/xtables-save.c                       | 237 ++++--------------
 11 files changed, 143 insertions(+), 237 deletions(-)
 create mode 100755 iptables/tests/shell/testcases/ebtables/0004-save-counters_0

-- 
2.22.0

[iptables PATCH v2 01/11] ebtables: Fix error message for invalid parameters

[Phil Sutter]

With empty ruleset, ebtables-nft would report the wrong argv:

| % sudo ./install/sbin/ebtables-nft -vnL
| ebtables v1.8.3 (nf_tables): Unknown argument: './install/sbin/ebtables-nft'
| Try `ebtables -h' or 'ebtables --help' for more information.

After a (successful) call to 'ebtables-nft -L', this would even
segfault:

| % sudo ./install/sbin/ebtables-nft -vnL
| zsh: segmentation fault  sudo ./install/sbin/ebtables-nft -vnL

Fixes: acde6be32036f ("ebtables-translate: Fix segfault while parsing extension options")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-eb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index 171f41b0f616e..b8d89ad974a42 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -1180,7 +1180,7 @@ print_zero:
 			if (ebt_command_default(&cs))
 				xtables_error(PARAMETER_PROBLEM,
 					      "Unknown argument: '%s'",
-					      argv[optind - 1]);
+					      argv[optind]);
 
 			if (command != 'A' && command != 'I' &&
 			    command != 'D' && command != 'C')
-- 
2.22.0

[iptables PATCH v2 02/11] ebtables-save: Fix counter formatting

[Phil Sutter]

The initial problem was 'ebtables-save -c' printing iptables-style
counters but at the same time not disabling ebtables-style counter
output (which was even printed in wrong format for ebtables-save).

The code around counter output was complicated enough to motivate a
larger rework:

* Make FMT_C_COUNTS indicate the appended counter style for ebtables.

* Use FMT_EBT_SAVE to distinguish between '-c' style counters and the
  legacy pcnt/bcnt ones.

Consequently, ebtables-save sets format to:

FMT_NOCOUNTS			- for no counters
FMT_EBT_SAVE			- for iptables-style counters
FMT_EBT_SAVE | FMT_C_COUNTS	- for '-c' style counters

For regular ebtables, list_rules() always sets FMT_C_COUNTS
(iptables-style counters are never used there) and FMT_NOCOUNTS if no
counters are requested.

The big plus is if neither FMT_NOCOUNTS nor FMT_C_COUNTS is set,
iptables-style counters are to be printed - both in iptables and
ebtables. This allows to drop the ebtables-specific 'save_counters'
callback.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-bridge.c                         | 39 +++--------
 .../testcases/ebtables/0004-save-counters_0   | 67 +++++++++++++++++++
 iptables/xtables-eb.c                         |  2 +-
 iptables/xtables-save.c                       |  3 +-
 4 files changed, 81 insertions(+), 30 deletions(-)
 create mode 100755 iptables/tests/shell/testcases/ebtables/0004-save-counters_0

diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index ddfbee165da93..2e4b309b86135 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -21,8 +21,6 @@
 #include "nft-bridge.h"
 #include "nft.h"
 
-static bool ebt_legacy_counter_fmt;
-
 void ebt_cs_clean(struct iptables_command_state *cs)
 {
 	struct ebt_match *m, *nm;
@@ -422,22 +420,6 @@ static void print_protocol(uint16_t ethproto, bool invert, unsigned int bitmask)
 		printf("%s ", ent->e_name);
 }
 
-static void nft_bridge_save_counters(const void *data)
-{
-	const char *ctr;
-
-	if (ebt_legacy_counter_fmt)
-		return;
-
-	ctr = getenv("EBTABLES_SAVE_COUNTER");
-	if (ctr) {
-		ebt_legacy_counter_fmt = true;
-		return;
-	}
-
-	save_counters(data);
-}
-
 static void nft_bridge_save_rule(const void *data, unsigned int format)
 {
 	const struct iptables_command_state *cs = data;
@@ -474,15 +456,16 @@ static void nft_bridge_save_rule(const void *data, unsigned int format)
 		cs->target->print(&cs->fw, cs->target->t, format & FMT_NUMERIC);
 	}
 
-	if (format & FMT_EBT_SAVE)
-		printf(" -c %"PRIu64" %"PRIu64"",
-		       (uint64_t)cs->counters.pcnt,
-		       (uint64_t)cs->counters.bcnt);
-
-	if (!(format & FMT_NOCOUNTS))
-		printf(" , pcnt = %"PRIu64" -- bcnt = %"PRIu64"",
-		       (uint64_t)cs->counters.pcnt,
-		       (uint64_t)cs->counters.bcnt);
+	if ((format & (FMT_NOCOUNTS | FMT_C_COUNTS)) == FMT_C_COUNTS) {
+		if (format & FMT_EBT_SAVE)
+			printf(" -c %"PRIu64" %"PRIu64"",
+			       (uint64_t)cs->counters.pcnt,
+			       (uint64_t)cs->counters.bcnt);
+		else
+			printf(" , pcnt = %"PRIu64" -- bcnt = %"PRIu64"",
+			       (uint64_t)cs->counters.pcnt,
+			       (uint64_t)cs->counters.bcnt);
+	}
 
 	if (!(format & FMT_NONEWLINE))
 		fputc('\n', stdout);
@@ -763,7 +746,7 @@ struct nft_family_ops nft_family_ops_bridge = {
 	.print_header		= nft_bridge_print_header,
 	.print_rule		= nft_bridge_print_rule,
 	.save_rule		= nft_bridge_save_rule,
-	.save_counters		= nft_bridge_save_counters,
+	.save_counters		= save_counters,
 	.save_chain		= nft_bridge_save_chain,
 	.post_parse		= NULL,
 	.rule_to_cs		= nft_rule_to_ebtables_command_state,
diff --git a/iptables/tests/shell/testcases/ebtables/0004-save-counters_0 b/iptables/tests/shell/testcases/ebtables/0004-save-counters_0
new file mode 100755
index 0000000000000..8348dc7ee231f
--- /dev/null
+++ b/iptables/tests/shell/testcases/ebtables/0004-save-counters_0
@@ -0,0 +1,67 @@
+#!/bin/bash
+
+set -e
+
+# there is no legacy backend to test
+[[ $XT_MULTI == */xtables-nft-multi ]] || { echo "skip $XT_MULTI"; exit 0; }
+
+$XT_MULTI ebtables --init-table
+$XT_MULTI ebtables -A FORWARD -i nodev123 -o nodev432 -j ACCEPT
+$XT_MULTI ebtables -A FORWARD -i nodev432 -o nodev123 -j ACCEPT
+
+EXPECT='Bridge table: filter
+
+Bridge chain: FORWARD, entries: 2, policy: ACCEPT
+-i nodev123 -o nodev432 -j ACCEPT
+-i nodev432 -o nodev123 -j ACCEPT'
+
+echo "ebtables -L FORWARD"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables -L FORWARD)
+
+EXPECT='Bridge table: filter
+
+Bridge chain: FORWARD, entries: 2, policy: ACCEPT
+-i nodev123 -o nodev432 -j ACCEPT , pcnt = 0 -- bcnt = 0
+-i nodev432 -o nodev123 -j ACCEPT , pcnt = 0 -- bcnt = 0'
+
+echo "ebtables -L FORWARD --Lc"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables -L FORWARD --Lc)
+
+EXPECT='*filter
+:INPUT ACCEPT
+:FORWARD ACCEPT
+:OUTPUT ACCEPT
+-A FORWARD -i nodev123 -o nodev432 -j ACCEPT
+-A FORWARD -i nodev432 -o nodev123 -j ACCEPT
+'
+
+echo "ebtables-save"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables-save | grep -v '^#')
+
+EXPECT='*filter
+:INPUT ACCEPT
+:FORWARD ACCEPT
+:OUTPUT ACCEPT
+[0:0] -A FORWARD -i nodev123 -o nodev432 -j ACCEPT
+[0:0] -A FORWARD -i nodev432 -o nodev123 -j ACCEPT
+'
+
+echo "ebtables-save -c"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables-save -c | grep -v '^#')
+
+export EBTABLES_SAVE_COUNTER=yes
+
+# -c flag overrides EBTABLES_SAVE_COUNTER variable
+echo "EBTABLES_SAVE_COUNTER=yes ebtables-save -c"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables-save -c | grep -v '^#')
+
+EXPECT='*filter
+:INPUT ACCEPT
+:FORWARD ACCEPT
+:OUTPUT ACCEPT
+-A FORWARD -i nodev123 -o nodev432 -j ACCEPT -c 0 0
+-A FORWARD -i nodev432 -o nodev123 -j ACCEPT -c 0 0
+'
+
+echo "EBTABLES_SAVE_COUNTER=yes ebtables-save"
+diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables-save | grep -v '^#')
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index b8d89ad974a42..121ecbecd0b64 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -410,7 +410,7 @@ static int list_rules(struct nft_handle *h, const char *chain, const char *table
 {
 	unsigned int format;
 
-	format = FMT_OPTIONS;
+	format = FMT_OPTIONS | FMT_C_COUNTS;
 	if (verbose)
 		format |= FMT_VIA;
 
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index b8d19705771ed..491122f39bbb0 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -274,7 +274,8 @@ static int __ebt_save(struct nft_handle *h, const char *tablename, bool counters
 	printf("*%s\n", tablename);
 
 	if (counters)
-		format = ebt_legacy_counter_format ? FMT_EBT_SAVE : 0;
+		format = FMT_EBT_SAVE |
+			(ebt_legacy_counter_format ? FMT_C_COUNTS : 0);
 
 	/* Dump out chain names first,
 	 * thereby preventing dependency conflicts */
-- 
2.22.0

[iptables PATCH v2 03/11] xtables-save: Unify *-save header/footer comments

[Phil Sutter]

Make eb- and arptables-save print both header and footer comments, too.
Also print them for each table separately - the timing information is
worth the extra lines in output.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 .../arptables/0001-arptables-save-restore_0   |  7 +++----
 .../0002-arptables-restore-defaults_0         |  6 ++----
 .../arptables/0003-arptables-verbose-output_0 |  5 ++---
 .../ebtables/0002-ebtables-save-restore_0     |  4 +---
 .../ebtables/0003-ebtables-restore-defaults_0 |  6 ++----
 .../testcases/ebtables/0004-save-counters_0   |  9 +++------
 iptables/xtables-save.c                       | 19 ++++++++++---------
 7 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0 b/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0
index e10f61cc8f95b..bf04dc0a3e15a 100755
--- a/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0
+++ b/iptables/tests/shell/testcases/arptables/0001-arptables-save-restore_0
@@ -50,13 +50,12 @@ DUMP='*filter
 -A foo -j MARK --set-mark 12345
 -A foo -j ACCEPT --opcode 1
 -A foo -j ACCEPT --proto-type 0x800
--A foo -j ACCEPT -i lo --opcode 1 --proto-type 0x800
-'
+-A foo -j ACCEPT -i lo --opcode 1 --proto-type 0x800'
 
-diff -u <(echo -e "$DUMP") <($XT_MULTI arptables-save)
+diff -u <(echo -e "$DUMP") <($XT_MULTI arptables-save | grep -v "^#")
 
 # make sure dump can be restored and check it didn't change
 
 $XT_MULTI arptables -F
 $XT_MULTI arptables-restore <<<$DUMP
-diff -u <(echo -e "$DUMP") <($XT_MULTI arptables-save)
+diff -u <(echo -e "$DUMP") <($XT_MULTI arptables-save | grep -v "^#")
diff --git a/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0 b/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0
index b2ed95e87bb40..38d387f327ebb 100755
--- a/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0
+++ b/iptables/tests/shell/testcases/arptables/0002-arptables-restore-defaults_0
@@ -11,8 +11,7 @@ set -e
 DUMP='*filter
 :OUTPUT ACCEPT
 -A OUTPUT -j mangle --mangle-ip-s 10.0.0.1
--A OUTPUT -j mangle --mangle-ip-d 10.0.0.2
-'
+-A OUTPUT -j mangle --mangle-ip-d 10.0.0.2'
 
 # note how mangle-ip-s is unset in second rule
 
@@ -20,8 +19,7 @@ EXPECT='*filter
 :INPUT ACCEPT
 :OUTPUT ACCEPT
 -A OUTPUT -j mangle --mangle-ip-s 10.0.0.1
--A OUTPUT -j mangle --mangle-ip-d 10.0.0.2
-'
+-A OUTPUT -j mangle --mangle-ip-d 10.0.0.2'
 
 $XT_MULTI arptables -F
 $XT_MULTI arptables-restore <<<$DUMP
diff --git a/iptables/tests/shell/testcases/arptables/0003-arptables-verbose-output_0 b/iptables/tests/shell/testcases/arptables/0003-arptables-verbose-output_0
index 3a9807a1cfe0b..10c5ec33ada2c 100755
--- a/iptables/tests/shell/testcases/arptables/0003-arptables-verbose-output_0
+++ b/iptables/tests/shell/testcases/arptables/0003-arptables-verbose-output_0
@@ -58,7 +58,6 @@ EXPECT='*filter
 -A INPUT -j MARK -i eth23 --set-mark 42
 -A OUTPUT -j CLASSIFY -o eth23 --set-class 23:42
 -A OUTPUT -j foo -o eth23
--A foo -j mangle -o eth23 --mangle-ip-s 10.0.0.1
-'
+-A foo -j mangle -o eth23 --mangle-ip-s 10.0.0.1'
 
-diff -u -Z <(echo -e "$EXPECT") <($XT_MULTI arptables-save)
+diff -u -Z <(echo -e "$EXPECT") <($XT_MULTI arptables-save | grep -v '^#')
diff --git a/iptables/tests/shell/testcases/ebtables/0002-ebtables-save-restore_0 b/iptables/tests/shell/testcases/ebtables/0002-ebtables-save-restore_0
index 080ba49a4974d..e18d46551509d 100755
--- a/iptables/tests/shell/testcases/ebtables/0002-ebtables-save-restore_0
+++ b/iptables/tests/shell/testcases/ebtables/0002-ebtables-save-restore_0
@@ -99,7 +99,6 @@ DUMP='*filter
 -A foo --802_3-sap 0x23 --limit 100/sec --limit-burst 5 -j ACCEPT
 -A foo --pkttype-type multicast --log-level notice --log-prefix "" -j CONTINUE
 -A foo --pkttype-type multicast --limit 100/sec --limit-burst 5 -j ACCEPT
-
 *nat
 :PREROUTING ACCEPT
 :OUTPUT DROP
@@ -107,8 +106,7 @@ DUMP='*filter
 :nat_foo DROP
 -A PREROUTING -j redirect 
 -A OUTPUT -j ACCEPT
--A POSTROUTING -j ACCEPT
-'
+-A POSTROUTING -j ACCEPT'
 
 diff -u <(echo -e "$DUMP") <($XT_MULTI ebtables-save | grep -v '^#')
 
diff --git a/iptables/tests/shell/testcases/ebtables/0003-ebtables-restore-defaults_0 b/iptables/tests/shell/testcases/ebtables/0003-ebtables-restore-defaults_0
index c858054764d70..62d224134456b 100755
--- a/iptables/tests/shell/testcases/ebtables/0003-ebtables-restore-defaults_0
+++ b/iptables/tests/shell/testcases/ebtables/0003-ebtables-restore-defaults_0
@@ -13,8 +13,7 @@ DUMP='*filter
 -A FORWARD --limit 100 --limit-burst 42 -j ACCEPT
 -A FORWARD --limit 1000 -j ACCEPT
 -A FORWARD --log --log-prefix "foobar"
--A FORWARD --log
-'
+-A FORWARD --log'
 
 # note how limit-burst is 5 in second rule and log-prefix empty in fourth one
 
@@ -25,8 +24,7 @@ EXPECT='*filter
 -A FORWARD --limit 100/sec --limit-burst 42 -j ACCEPT
 -A FORWARD --limit 1000/sec --limit-burst 5 -j ACCEPT
 -A FORWARD --log-level notice --log-prefix "foobar" -j CONTINUE
--A FORWARD --log-level notice --log-prefix "" -j CONTINUE
-'
+-A FORWARD --log-level notice --log-prefix "" -j CONTINUE'
 
 $XT_MULTI ebtables --init-table
 $XT_MULTI ebtables-restore <<<$DUMP
diff --git a/iptables/tests/shell/testcases/ebtables/0004-save-counters_0 b/iptables/tests/shell/testcases/ebtables/0004-save-counters_0
index 8348dc7ee231f..46966f433139a 100755
--- a/iptables/tests/shell/testcases/ebtables/0004-save-counters_0
+++ b/iptables/tests/shell/testcases/ebtables/0004-save-counters_0
@@ -32,8 +32,7 @@ EXPECT='*filter
 :FORWARD ACCEPT
 :OUTPUT ACCEPT
 -A FORWARD -i nodev123 -o nodev432 -j ACCEPT
--A FORWARD -i nodev432 -o nodev123 -j ACCEPT
-'
+-A FORWARD -i nodev432 -o nodev123 -j ACCEPT'
 
 echo "ebtables-save"
 diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables-save | grep -v '^#')
@@ -43,8 +42,7 @@ EXPECT='*filter
 :FORWARD ACCEPT
 :OUTPUT ACCEPT
 [0:0] -A FORWARD -i nodev123 -o nodev432 -j ACCEPT
-[0:0] -A FORWARD -i nodev432 -o nodev123 -j ACCEPT
-'
+[0:0] -A FORWARD -i nodev432 -o nodev123 -j ACCEPT'
 
 echo "ebtables-save -c"
 diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables-save -c | grep -v '^#')
@@ -60,8 +58,7 @@ EXPECT='*filter
 :FORWARD ACCEPT
 :OUTPUT ACCEPT
 -A FORWARD -i nodev123 -o nodev432 -j ACCEPT -c 0 0
--A FORWARD -i nodev432 -o nodev123 -j ACCEPT -c 0 0
-'
+-A FORWARD -i nodev432 -o nodev123 -j ACCEPT -c 0 0'
 
 echo "EBTABLES_SAVE_COUNTER=yes ebtables-save"
 diff -u <(echo -e "$EXPECT") <($XT_MULTI ebtables-save | grep -v '^#')
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 491122f39bbb0..0cf11f998cc77 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -250,7 +250,6 @@ static int __ebt_save(struct nft_handle *h, const char *tablename, bool counters
 {
 	struct nftnl_chain_list *chain_list;
 	unsigned int format = FMT_NOCOUNTS;
-	static bool first = true;
 	time_t now;
 
 	if (!nft_table_find(h, tablename)) {
@@ -265,12 +264,9 @@ static int __ebt_save(struct nft_handle *h, const char *tablename, bool counters
 
 	chain_list = nft_chain_list_get(h, tablename);
 
-	if (first) {
-		now = time(NULL);
-		printf("# Generated by %s v%s on %s", prog_name,
-		       prog_vers, ctime(&now));
-		first = false;
-	}
+	now = time(NULL);
+	printf("# Generated by %s v%s on %s", prog_name,
+	       prog_vers, ctime(&now));
 	printf("*%s\n", tablename);
 
 	if (counters)
@@ -281,7 +277,8 @@ static int __ebt_save(struct nft_handle *h, const char *tablename, bool counters
 	 * thereby preventing dependency conflicts */
 	nft_chain_save(h, chain_list);
 	nft_rule_save(h, tablename, format);
-	printf("\n");
+	now = time(NULL);
+	printf("# Completed on %s", ctime(&now));
 	return 0;
 }
 
@@ -361,6 +358,7 @@ int xtables_arp_save_main(int argc, char **argv)
 	struct nft_handle h = {
 		.family	= NFPROTO_ARP,
 	};
+	time_t now;
 	int c;
 
 	xtables_globals.program_name = basename(*argv);;
@@ -407,10 +405,13 @@ int xtables_arp_save_main(int argc, char **argv)
 		return 0;
 	}
 
+	printf("# Generated by %s v%s on %s", prog_name,
+	       prog_vers, ctime(&now));
 	printf("*filter\n");
 	nft_chain_save(&h, nft_chain_list_get(&h, "filter"));
 	nft_rule_save(&h, "filter", show_counters ? 0 : FMT_NOCOUNTS);
-	printf("\n");
+	now = time(NULL);
+	printf("# Completed on %s", ctime(&now));
 	nft_fini(&h);
 	return 0;
 }
-- 
2.22.0

[iptables PATCH v2 04/11] xtables-save: Fix table compatibility check

[Phil Sutter]

The builtin table check guarding the 'is incompatible' warning was
wrong: The idea was to print the warning only for incompatible tables
which are builtin, not for others. Yet the code would print the warning
only for non-builtin ones.

Also reorder the checks: nft_table_builtin_find() is fast and therefore
a quick way to bail for uninteresting tables. The compatibility check is
needed for the remaining tables, only.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-save.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 0cf11f998cc77..811ec6330a4cb 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -67,11 +67,12 @@ __do_output(struct nft_handle *h, const char *tablename, bool counters)
 {
 	struct nftnl_chain_list *chain_list;
 
+	if (!nft_table_builtin_find(h, tablename))
+		return 0;
 
 	if (!nft_is_table_compatible(h, tablename)) {
-		if (!nft_table_builtin_find(h, tablename))
-			printf("# Table `%s' is incompatible, use 'nft' tool.\n",
-			       tablename);
+		printf("# Table `%s' is incompatible, use 'nft' tool.\n",
+		       tablename);
 		return 0;
 	}
 
-- 
2.22.0

[iptables PATCH v2 05/11] nft: Make nft_for_each_table() more versatile

[Phil Sutter]

Support passing arbitrary data (via void pointer) to the callback.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c          |  6 +++---
 iptables/nft.h          |  2 +-
 iptables/xtables-save.c | 27 +++++++++++++++++----------
 3 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 8f0d5e664eca6..cd42af70b54ef 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -2092,8 +2092,8 @@ err:
 }
 
 int nft_for_each_table(struct nft_handle *h,
-		       int (*func)(struct nft_handle *h, const char *tablename, bool counters),
-		       bool counters)
+		       int (*func)(struct nft_handle *h, const char *tablename, void *data),
+		       void *data)
 {
 	struct nftnl_table_list *list;
 	struct nftnl_table_list_iter *iter;
@@ -2112,7 +2112,7 @@ int nft_for_each_table(struct nft_handle *h,
 		const char *tablename =
 			nftnl_table_get(t, NFTNL_TABLE_NAME);
 
-		func(h, tablename, counters);
+		func(h, tablename, data);
 
 		t = nftnl_table_list_iter_next(iter);
 	}
diff --git a/iptables/nft.h b/iptables/nft.h
index dc1161840a38c..da078a44bab7b 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -81,7 +81,7 @@ void nft_build_cache(struct nft_handle *h);
 struct nftnl_table;
 struct nftnl_chain_list;
 
-int nft_for_each_table(struct nft_handle *h, int (*func)(struct nft_handle *h, const char *tablename, bool counters), bool counters);
+int nft_for_each_table(struct nft_handle *h, int (*func)(struct nft_handle *h, const char *tablename, void *data), void *data);
 bool nft_table_find(struct nft_handle *h, const char *tablename);
 int nft_table_purge_chains(struct nft_handle *h, const char *table, struct nftnl_chain_list *list);
 int nft_table_flush(struct nft_handle *h, const char *table);
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 811ec6330a4cb..484450f03354f 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -62,10 +62,15 @@ static const struct option ebt_save_options[] = {
 
 static bool ebt_legacy_counter_format;
 
+struct do_output_data {
+	bool counters;
+};
+
 static int
-__do_output(struct nft_handle *h, const char *tablename, bool counters)
+__do_output(struct nft_handle *h, const char *tablename, void *data)
 {
 	struct nftnl_chain_list *chain_list;
+	struct do_output_data *d = data;
 
 	if (!nft_table_builtin_find(h, tablename))
 		return 0;
@@ -89,7 +94,7 @@ __do_output(struct nft_handle *h, const char *tablename, bool counters)
 	/* Dump out chain names first,
 	 * thereby preventing dependency conflicts */
 	nft_chain_save(h, chain_list);
-	nft_rule_save(h, tablename, counters ? 0 : FMT_NOCOUNTS);
+	nft_rule_save(h, tablename, d->counters ? 0 : FMT_NOCOUNTS);
 
 	now = time(NULL);
 	printf("COMMIT\n");
@@ -98,12 +103,12 @@ __do_output(struct nft_handle *h, const char *tablename, bool counters)
 }
 
 static int
-do_output(struct nft_handle *h, const char *tablename, bool counters)
+do_output(struct nft_handle *h, const char *tablename, struct do_output_data *d)
 {
 	int ret;
 
 	if (!tablename) {
-		ret = nft_for_each_table(h, __do_output, counters);
+		ret = nft_for_each_table(h, __do_output, d);
 		nft_check_xt_legacy(h->family, true);
 		return !!ret;
 	}
@@ -114,7 +119,7 @@ do_output(struct nft_handle *h, const char *tablename, bool counters)
 		return 1;
 	}
 
-	ret = __do_output(h, tablename, counters);
+	ret = __do_output(h, tablename, d);
 	nft_check_xt_legacy(h->family, true);
 	return ret;
 }
@@ -128,6 +133,7 @@ xtables_save_main(int family, int argc, char *argv[])
 {
 	const struct builtin_table *tables;
 	const char *tablename = NULL;
+	struct do_output_data d = {};
 	bool dump = false;
 	struct nft_handle h = {
 		.family	= family,
@@ -150,7 +156,7 @@ xtables_save_main(int family, int argc, char *argv[])
 			fprintf(stderr, "-b/--binary option is not implemented\n");
 			break;
 		case 'c':
-			show_counters = true;
+			d.counters = true;
 			break;
 
 		case 't':
@@ -229,7 +235,7 @@ xtables_save_main(int family, int argc, char *argv[])
 		exit(EXIT_FAILURE);
 	}
 
-	ret = do_output(&h, tablename, show_counters);
+	ret = do_output(&h, tablename, &d);
 	nft_fini(&h);
 	if (dump)
 		exit(0);
@@ -247,10 +253,11 @@ int xtables_ip6_save_main(int argc, char *argv[])
 	return xtables_save_main(NFPROTO_IPV6, argc, argv);
 }
 
-static int __ebt_save(struct nft_handle *h, const char *tablename, bool counters)
+static int __ebt_save(struct nft_handle *h, const char *tablename, void *data)
 {
 	struct nftnl_chain_list *chain_list;
 	unsigned int format = FMT_NOCOUNTS;
+	bool *counters = data;
 	time_t now;
 
 	if (!nft_table_find(h, tablename)) {
@@ -286,9 +293,9 @@ static int __ebt_save(struct nft_handle *h, const char *tablename, bool counters
 static int ebt_save(struct nft_handle *h, const char *tablename, bool counters)
 {
 	if (!tablename)
-		return nft_for_each_table(h, __ebt_save, counters);
+		return nft_for_each_table(h, __ebt_save, &counters);
 
-	return __ebt_save(h, tablename, counters);
+	return __ebt_save(h, tablename, &counters);
 }
 
 int xtables_eb_save_main(int argc_, char *argv_[])
-- 
2.22.0

[iptables PATCH v2 06/11] xtables-save: Avoid mixed code and declarations

[Phil Sutter]

Also move time() calls to where they are used.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-save.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 484450f03354f..ac452f1dd6f14 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -71,6 +71,7 @@ __do_output(struct nft_handle *h, const char *tablename, void *data)
 {
 	struct nftnl_chain_list *chain_list;
 	struct do_output_data *d = data;
+	time_t now;
 
 	if (!nft_table_builtin_find(h, tablename))
 		return 0;
@@ -85,19 +86,18 @@ __do_output(struct nft_handle *h, const char *tablename, void *data)
 	if (!chain_list)
 		return 0;
 
-	time_t now = time(NULL);
-
+	now = time(NULL);
 	printf("# Generated by %s v%s on %s", prog_name,
 	       prog_vers, ctime(&now));
-	printf("*%s\n", tablename);
 
+	printf("*%s\n", tablename);
 	/* Dump out chain names first,
 	 * thereby preventing dependency conflicts */
 	nft_chain_save(h, chain_list);
 	nft_rule_save(h, tablename, d->counters ? 0 : FMT_NOCOUNTS);
+	printf("COMMIT\n");
 
 	now = time(NULL);
-	printf("COMMIT\n");
 	printf("# Completed on %s", ctime(&now));
 	return 0;
 }
-- 
2.22.0

[iptables PATCH v2 07/11] xtables-save: Pass optstring/longopts to xtables_save_main()

[Phil Sutter]

Introduce variables for the different optstrings so short and long
options live side-by-side.

In order to make xtables_save_main() more versatile, pass optstring and
longopts via parameter.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-save.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index ac452f1dd6f14..b4d14b5bcd016 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -33,7 +33,8 @@
 
 static bool show_counters = false;
 
-static const struct option options[] = {
+static const char *ipt_save_optstring = "bcdt:M:f:46V";
+static const struct option ipt_save_options[] = {
 	{.name = "counters", .has_arg = false, .val = 'c'},
 	{.name = "version",  .has_arg = false, .val = 'V'},
 	{.name = "dump",     .has_arg = false, .val = 'd'},
@@ -45,6 +46,7 @@ static const struct option options[] = {
 	{NULL},
 };
 
+static const char *arp_save_optstring = "cM:V";
 static const struct option arp_save_options[] = {
 	{.name = "counters", .has_arg = false, .val = 'c'},
 	{.name = "version",  .has_arg = false, .val = 'V'},
@@ -52,6 +54,7 @@ static const struct option arp_save_options[] = {
 	{NULL},
 };
 
+static const char *ebt_save_optstring = "ct:M:V";
 static const struct option ebt_save_options[] = {
 	{.name = "counters", .has_arg = false, .val = 'c'},
 	{.name = "version",  .has_arg = false, .val = 'V'},
@@ -129,7 +132,8 @@ do_output(struct nft_handle *h, const char *tablename, struct do_output_data *d)
  * rule
  */
 static int
-xtables_save_main(int family, int argc, char *argv[])
+xtables_save_main(int family, int argc, char *argv[],
+		  const char *optstring, const struct option *longopts)
 {
 	const struct builtin_table *tables;
 	const char *tablename = NULL;
@@ -150,7 +154,7 @@ xtables_save_main(int family, int argc, char *argv[])
 		exit(1);
 	}
 
-	while ((c = getopt_long(argc, argv, "bcdt:M:f:46V", options, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, optstring, longopts, NULL)) != -1) {
 		switch (c) {
 		case 'b':
 			fprintf(stderr, "-b/--binary option is not implemented\n");
@@ -245,12 +249,14 @@ xtables_save_main(int family, int argc, char *argv[])
 
 int xtables_ip4_save_main(int argc, char *argv[])
 {
-	return xtables_save_main(NFPROTO_IPV4, argc, argv);
+	return xtables_save_main(NFPROTO_IPV4, argc, argv,
+				 ipt_save_optstring, ipt_save_options);
 }
 
 int xtables_ip6_save_main(int argc, char *argv[])
 {
-	return xtables_save_main(NFPROTO_IPV6, argc, argv);
+	return xtables_save_main(NFPROTO_IPV6, argc, argv,
+				 ipt_save_optstring, ipt_save_options);
 }
 
 static int __ebt_save(struct nft_handle *h, const char *tablename, void *data)
@@ -323,7 +329,7 @@ int xtables_eb_save_main(int argc_, char *argv_[])
 		exit(1);
 	}
 
-	while ((c = getopt_long(argc_, argv_, "ct:M:V", ebt_save_options, NULL)) != -1) {
+	while ((c = getopt_long(argc_, argv_, ebt_save_optstring, ebt_save_options, NULL)) != -1) {
 		switch (c) {
 		case 'c':
 			unsetenv("EBTABLES_SAVE_COUNTER");
@@ -378,7 +384,7 @@ int xtables_arp_save_main(int argc, char **argv)
 		exit(1);
 	}
 
-	while ((c = getopt_long(argc, argv, "cM:V", arp_save_options, NULL)) != -1) {
+	while ((c = getopt_long(argc, argv, arp_save_optstring, arp_save_options, NULL)) != -1) {
 		switch (c) {
 		case 'c':
 			show_counters = true;
-- 
2.22.0

[iptables PATCH v2 08/11] xtables-save: Make COMMIT line optional

[Phil Sutter]

Explicit commits are not used by either arp- nor ebtables-save. In order
to share code between all the different *-save tools without inducing
changes to ruleset dump contents, allow for callers of do_output() to
turn COMMIT lines on or off.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-save.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index b4d14b5bcd016..249b396091af4 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -67,6 +67,7 @@ static bool ebt_legacy_counter_format;
 
 struct do_output_data {
 	bool counters;
+	bool commit;
 };
 
 static int
@@ -98,7 +99,8 @@ __do_output(struct nft_handle *h, const char *tablename, void *data)
 	 * thereby preventing dependency conflicts */
 	nft_chain_save(h, chain_list);
 	nft_rule_save(h, tablename, d->counters ? 0 : FMT_NOCOUNTS);
-	printf("COMMIT\n");
+	if (d->commit)
+		printf("COMMIT\n");
 
 	now = time(NULL);
 	printf("# Completed on %s", ctime(&now));
@@ -219,6 +221,7 @@ xtables_save_main(int family, int argc, char *argv[],
 		init_extensions4();
 #endif
 		tables = xtables_ipv4;
+		d.commit = true;
 		break;
 	case NFPROTO_ARP:
 		tables = xtables_arp;
-- 
2.22.0

[iptables PATCH v2 09/11] xtables-save: Pass format flags to do_output()

[Phil Sutter]

Let callers define the flags to pass to nft_rule_save() instead of just
setting the counters boolean.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-save.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 249b396091af4..980a80ff06f96 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -66,7 +66,7 @@ static const struct option ebt_save_options[] = {
 static bool ebt_legacy_counter_format;
 
 struct do_output_data {
-	bool counters;
+	unsigned int format;
 	bool commit;
 };
 
@@ -98,7 +98,7 @@ __do_output(struct nft_handle *h, const char *tablename, void *data)
 	/* Dump out chain names first,
 	 * thereby preventing dependency conflicts */
 	nft_chain_save(h, chain_list);
-	nft_rule_save(h, tablename, d->counters ? 0 : FMT_NOCOUNTS);
+	nft_rule_save(h, tablename, d->format);
 	if (d->commit)
 		printf("COMMIT\n");
 
@@ -139,7 +139,9 @@ xtables_save_main(int family, int argc, char *argv[],
 {
 	const struct builtin_table *tables;
 	const char *tablename = NULL;
-	struct do_output_data d = {};
+	struct do_output_data d = {
+		.format = FMT_NOCOUNTS,
+	};
 	bool dump = false;
 	struct nft_handle h = {
 		.family	= family,
@@ -162,7 +164,7 @@ xtables_save_main(int family, int argc, char *argv[],
 			fprintf(stderr, "-b/--binary option is not implemented\n");
 			break;
 		case 'c':
-			d.counters = true;
+			d.format &= ~FMT_NOCOUNTS;
 			break;
 
 		case 't':
-- 
2.22.0

[iptables PATCH v2 10/11] arptables-save: Merge into xtables_save_main()

[Phil Sutter]

With all preparations in place, xtables_save_main() can replace it with
not further changes.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-save.c | 63 ++---------------------------------------
 1 file changed, 3 insertions(+), 60 deletions(-)

diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 980a80ff06f96..0c294e056c7b5 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -372,65 +372,8 @@ int xtables_eb_save_main(int argc_, char *argv_[])
 	return 0;
 }
 
-int xtables_arp_save_main(int argc, char **argv)
+int xtables_arp_save_main(int argc, char *argv[])
 {
-	struct nft_handle h = {
-		.family	= NFPROTO_ARP,
-	};
-	time_t now;
-	int c;
-
-	xtables_globals.program_name = basename(*argv);;
-	c = xtables_init_all(&xtables_globals, h.family);
-	if (c < 0) {
-		fprintf(stderr, "%s/%s Failed to initialize xtables\n",
-				xtables_globals.program_name,
-				xtables_globals.program_version);
-		exit(1);
-	}
-
-	while ((c = getopt_long(argc, argv, arp_save_optstring, arp_save_options, NULL)) != -1) {
-		switch (c) {
-		case 'c':
-			show_counters = true;
-			break;
-		case 'M':
-			xtables_modprobe_program = optarg;
-			break;
-		case 'V':
-			printf("%s v%s (nf_tables)\n", prog_name, prog_vers);
-			exit(0);
-		default:
-			fprintf(stderr,
-				"Look at manual page `%s.8' for more information.\n",
-				prog_name);
-			exit(1);
-		}
-	}
-
-	if (nft_init(&h, xtables_arp) < 0) {
-		fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
-				xtables_globals.program_name,
-				xtables_globals.program_version,
-				strerror(errno));
-		exit(EXIT_FAILURE);
-	}
-
-	if (!nft_table_find(&h, "filter"))
-		return 0;
-
-	if (!nft_is_table_compatible(&h, "filter")) {
-		printf("# Table `filter' is incompatible, use 'nft' tool.\n");
-		return 0;
-	}
-
-	printf("# Generated by %s v%s on %s", prog_name,
-	       prog_vers, ctime(&now));
-	printf("*filter\n");
-	nft_chain_save(&h, nft_chain_list_get(&h, "filter"));
-	nft_rule_save(&h, "filter", show_counters ? 0 : FMT_NOCOUNTS);
-	now = time(NULL);
-	printf("# Completed on %s", ctime(&now));
-	nft_fini(&h);
-	return 0;
+	return xtables_save_main(NFPROTO_ARP, argc, argv,
+				 arp_save_optstring, arp_save_options);
 }
-- 
2.22.0

[iptables PATCH v2 11/11] ebtables-save: Merge into xtables_save_main()

[Phil Sutter]

The only thing missing was handling of EBTABLES_SAVE_COUNTER env var,
but that can be done after parsing parameters in bridge-specific code.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-save.c | 123 +++++-----------------------------------
 1 file changed, 13 insertions(+), 110 deletions(-)

diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 0c294e056c7b5..77b13f7ffbcdd 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -31,8 +31,6 @@
 #define prog_name xtables_globals.program_name
 #define prog_vers xtables_globals.program_version
 
-static bool show_counters = false;
-
 static const char *ipt_save_optstring = "bcdt:M:f:46V";
 static const struct option ipt_save_options[] = {
 	{.name = "counters", .has_arg = false, .val = 'c'},
@@ -63,8 +61,6 @@ static const struct option ebt_save_options[] = {
 	{NULL},
 };
 
-static bool ebt_legacy_counter_format;
-
 struct do_output_data {
 	unsigned int format;
 	bool commit;
@@ -228,9 +224,18 @@ xtables_save_main(int family, int argc, char *argv[],
 	case NFPROTO_ARP:
 		tables = xtables_arp;
 		break;
-	case NFPROTO_BRIDGE:
+	case NFPROTO_BRIDGE: {
+		const char *ctr = getenv("EBTABLES_SAVE_COUNTER");
+
+		if (!(d.format & FMT_NOCOUNTS)) {
+			d.format |= FMT_EBT_SAVE;
+		} else if (ctr && !strcmp(ctr, "yes")) {
+			d.format &= ~FMT_NOCOUNTS;
+			d.format |= FMT_C_COUNTS | FMT_EBT_SAVE;
+		}
 		tables = xtables_bridge;
 		break;
+	}
 	default:
 		fprintf(stderr, "Unknown family %d\n", family);
 		return 1;
@@ -264,112 +269,10 @@ int xtables_ip6_save_main(int argc, char *argv[])
 				 ipt_save_optstring, ipt_save_options);
 }
 
-static int __ebt_save(struct nft_handle *h, const char *tablename, void *data)
+int xtables_eb_save_main(int argc, char *argv[])
 {
-	struct nftnl_chain_list *chain_list;
-	unsigned int format = FMT_NOCOUNTS;
-	bool *counters = data;
-	time_t now;
-
-	if (!nft_table_find(h, tablename)) {
-		printf("Table `%s' does not exist\n", tablename);
-		return 1;
-	}
-
-	if (!nft_is_table_compatible(h, tablename)) {
-		printf("# Table `%s' is incompatible, use 'nft' tool.\n", tablename);
-		return 0;
-	}
-
-	chain_list = nft_chain_list_get(h, tablename);
-
-	now = time(NULL);
-	printf("# Generated by %s v%s on %s", prog_name,
-	       prog_vers, ctime(&now));
-	printf("*%s\n", tablename);
-
-	if (counters)
-		format = FMT_EBT_SAVE |
-			(ebt_legacy_counter_format ? FMT_C_COUNTS : 0);
-
-	/* Dump out chain names first,
-	 * thereby preventing dependency conflicts */
-	nft_chain_save(h, chain_list);
-	nft_rule_save(h, tablename, format);
-	now = time(NULL);
-	printf("# Completed on %s", ctime(&now));
-	return 0;
-}
-
-static int ebt_save(struct nft_handle *h, const char *tablename, bool counters)
-{
-	if (!tablename)
-		return nft_for_each_table(h, __ebt_save, &counters);
-
-	return __ebt_save(h, tablename, &counters);
-}
-
-int xtables_eb_save_main(int argc_, char *argv_[])
-{
-	const char *ctr = getenv("EBTABLES_SAVE_COUNTER");
-	const char *tablename = NULL;
-	struct nft_handle h = {
-		.family	= NFPROTO_BRIDGE,
-	};
-	int c;
-
-	if (ctr) {
-		if (strcmp(ctr, "yes") == 0) {
-			ebt_legacy_counter_format = true;
-			show_counters = true;
-		}
-	}
-
-	xtables_globals.program_name = basename(*argv_);
-	c = xtables_init_all(&xtables_globals, h.family);
-	if (c < 0) {
-		fprintf(stderr, "%s/%s Failed to initialize xtables\n",
-				xtables_globals.program_name,
-				xtables_globals.program_version);
-		exit(1);
-	}
-
-	while ((c = getopt_long(argc_, argv_, ebt_save_optstring, ebt_save_options, NULL)) != -1) {
-		switch (c) {
-		case 'c':
-			unsetenv("EBTABLES_SAVE_COUNTER");
-			show_counters = true;
-			ebt_legacy_counter_format = false;
-			break;
-		case 't':
-			/* Select specific table. */
-			tablename = optarg;
-			break;
-		case 'M':
-			xtables_modprobe_program = optarg;
-			break;
-		case 'V':
-			printf("%s v%s (nf_tables)\n", prog_name, prog_vers);
-			exit(0);
-		default:
-			fprintf(stderr,
-				"Look at manual page `%s.8' for more information.\n",
-				prog_name);
-			exit(1);
-		}
-	}
-
-	if (nft_init(&h, xtables_bridge) < 0) {
-		fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
-				xtables_globals.program_name,
-				xtables_globals.program_version,
-				strerror(errno));
-		exit(EXIT_FAILURE);
-	}
-
-	ebt_save(&h, tablename, show_counters);
-	nft_fini(&h);
-	return 0;
+	return xtables_save_main(NFPROTO_BRIDGE, argc, argv,
+				 ebt_save_optstring, ebt_save_options);
 }
 
 int xtables_arp_save_main(int argc, char *argv[])
-- 
2.22.0

Re: [iptables PATCH v2 00/11] Larger xtables-save review

[Florian Westphal]

Phil Sutter <phil@nwl.cc> wrote:
> This series started as a fix to program names mentioned in *-save
> outputs and ended in merging ebtables-save and arptables-save code into
> xtables_save_main used by ip{6,}tables-nft-save.
> 
> The first patch is actually unrelated but was discovered when testing
> counter output - depending on environment, ebtables-nft might segfault.
> 
> The second patch fixes option '-c' of ebtables-nft-save which enables
> counter prefixes in dumped rules but failed to disable the classical
> ebtables-style counters.
> 
> Patch three unifies the header/footer comments in all the *-save tools
> and also drops the extra newline printed in ebtables- and arptables-save
> output, so test scripts need adjustments beyond dropping the new comment
> lines from output.
> 
> Patch four fixes the table compatibility check in ip{6,}tables-nft-save.
> 
> Patches five and seven to nine prepare for integrating arptables- and
> ebtables-save into the xtables-save code.
> 
> Patch six merely fixes a minor coding-style issue.
> 
> Patches ten and eleven finally perform the actual merge.
> 
> Changes since v1:
> - Rebased onto current master branch.
> - Improved commit message in patch eight.

Thanks, this looks good to me.

Re: [iptables PATCH v2 00/11] Larger xtables-save review

[Pablo Neira Ayuso]

On Mon, Jul 22, 2019 at 12:16:17PM +0200, Phil Sutter wrote:
> This series started as a fix to program names mentioned in *-save
> outputs and ended in merging ebtables-save and arptables-save code into
> xtables_save_main used by ip{6,}tables-nft-save.

Applied, thanks Phil.