[iptables PATCH 00/12] Misc fixes (more or less)

[iptables PATCH 00/12] Misc fixes (more or less)

[Phil Sutter]

This is early fallout from working on a merge of ebtables commandline
parsers with the shared one. It is a mix of actual bug fixes, small
improvements and an implementaion of --change-counters command for
ebtables-nft.

Phil Sutter (12):
  Makefile: Install arptables-translate link and man page
  nft-bridge: nft_bridge_add() uses wrong flags
  xshared: struct xt_cmd_parse::xlate is unused
  xshared: All variants support -v, update OPTSTRING_COMMON
  xshared: Drop needless assignment in --help case
  xshared: Drop pointless CMD_REPLACE check
  tests: xlate: Print failing command line
  ebtables: Drop append_entry() wrapper
  ebtables: Make ebt_load_match_extensions() static
  ebtables: Align line number formatting with legacy
  xshared: do_parse: Ignore '-j CONTINUE'
  ebtables: Implement --change-counters command

 iptables/Makefile.am                          |  11 +-
 iptables/nft-bridge.c                         |   6 +-
 iptables/nft-bridge.h                         |   1 -
 iptables/nft-cmd.c                            |  20 ++++
 iptables/nft-cmd.h                            |  12 +++
 iptables/nft.c                                |  65 +++++++++++
 iptables/nft.h                                |   1 +
 .../testcases/ebtables/0010-change-counters_0 |  45 ++++++++
 iptables/xshared.c                            |  11 +-
 iptables/xshared.h                            |   9 +-
 iptables/xtables-eb.c                         | 102 ++++++++++--------
 iptables/xtables-translate.8                  |  12 ++-
 iptables/xtables-translate.c                  |   1 -
 xlate-test.py                                 |   5 +-
 14 files changed, 229 insertions(+), 72 deletions(-)
 create mode 100755 iptables/tests/shell/testcases/ebtables/0010-change-counters_0

-- 
2.41.0

[iptables PATCH 01/12] Makefile: Install arptables-translate link and man page

[Phil Sutter]

While at it, introduce a variable to hold the various semantic links to
xtables-translate.8 man page.

Fixes: 5b7324e0675e3 ("nft-arp: add arptables-translate")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/Makefile.am         | 11 +++++++----
 iptables/xtables-translate.8 | 12 ++++++++----
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/iptables/Makefile.am b/iptables/Makefile.am
index 8a7227024987f..0f8b430c20213 100644
--- a/iptables/Makefile.am
+++ b/iptables/Makefile.am
@@ -69,10 +69,12 @@ man_MANS         = iptables.8 iptables-restore.8 iptables-save.8 \
 dist_sbin_SCRIPTS = iptables-apply
 dist_pkgdata_DATA = iptables.xslt
 
+xlate_man_links = iptables-translate.8 ip6tables-translate.8 \
+		  iptables-restore-translate.8 ip6tables-restore-translate.8 \
+		  ebtables-translate.8 arptables-translate.8
+
 if ENABLE_NFTABLES
-man_MANS	+= iptables-translate.8 ip6tables-translate.8 \
-		   iptables-restore-translate.8 ip6tables-restore-translate.8 \
-		   xtables-monitor.8 ebtables-translate.8
+man_MANS	+= ${xlate_man_links} xtables-monitor.8
 
 dist_man_MANS	 = xtables-nft.8 xtables-translate.8 xtables-legacy.8 \
 		   arptables-nft.8 arptables-nft-restore.8 arptables-nft-save.8 \
@@ -97,6 +99,7 @@ x_sbin_links  = iptables-nft iptables-nft-restore iptables-nft-save \
 		arptables-nft arptables \
 		arptables-nft-restore arptables-restore \
 		arptables-nft-save arptables-save \
+		arptables-translate \
 		ebtables-nft ebtables \
 		ebtables-nft-restore ebtables-restore \
 		ebtables-nft-save ebtables-save \
@@ -108,7 +111,7 @@ iptables-extensions.8: iptables-extensions.8.tmpl ../extensions/matches.man ../e
 		-e '/@MATCH@/ r ../extensions/matches.man' \
 		-e '/@TARGET@/ r ../extensions/targets.man' $< >$@;
 
-iptables-translate.8 ip6tables-translate.8 iptables-restore-translate.8 ip6tables-restore-translate.8 ebtables-translate.8:
+${xlate_man_links}:
 	${AM_VERBOSE_GEN} echo '.so man8/xtables-translate.8' >$@
 
 ip6tables.8 ip6tables-apply.8 ip6tables-restore.8 ip6tables-save.8:
diff --git a/iptables/xtables-translate.8 b/iptables/xtables-translate.8
index ba16c5257c4a5..fe1278874b4da 100644
--- a/iptables/xtables-translate.8
+++ b/iptables/xtables-translate.8
@@ -30,10 +30,12 @@ iptables-translate \(em translation tool to migrate from iptables to nftables
 ip6tables-translate \(em translation tool to migrate from ip6tables to nftables
 .P
 ebtables-translate \(em translation tool to migrate from ebtables to nftables
+.P
+arptables-translate \(em translation tool to migrate from arptables to nftables
 .SH DESCRIPTION
 There is a set of tools to help the system administrator translate a given
-ruleset from \fBiptables(8)\fP, \fBip6tables(8)\fP and \fBebtables(8)\fP to
-\fBnftables(8)\fP.
+ruleset from \fBiptables(8)\fP, \fBip6tables(8)\fP, \fBebtables(8)\fP and
+\fBarptables(8)\fP to \fBnftables(8)\fP.
 
 The available commands are:
 
@@ -47,11 +49,13 @@ ip6tables\-translate
 ip6tables\-restore\-translate
 .IP \[bu] 2
 ebtables\-translate
+.IP \[bu] 2
+arptables\-translate
 
 .SH USAGE
 They take as input the original
-\fBiptables(8)\fP/\fBip6tables(8)\fP/\fBebtables(8)\fP syntax and
-output the native \fBnftables(8)\fP syntax.
+\fBiptables(8)\fP/\fBip6tables(8)\fP/\fBebtables(8)\fP/\fBarptables(8)\fP
+syntax and output the native \fBnftables(8)\fP syntax.
 
 The \fBiptables-restore-translate\fP tool reads a ruleset in the syntax
 produced by \fBiptables-save(8)\fP. Likewise, the
-- 
2.41.0

[iptables PATCH 02/12] nft-bridge: nft_bridge_add() uses wrong flags

[Phil Sutter]

When checking whether -s or -d was given, invflags were used by
accident. This change has no functional effect since the values remain
the same, but this way it's clear where the previously assigned flags
are used.

Fixes: 07f4ca9681688 ("xtables-compat: ebtables: allow checking for zero-mac")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-bridge.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index d9a8ad2b0f373..772525e1b45a9 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -134,14 +134,14 @@ static int nft_bridge_add(struct nft_handle *h, struct nft_rule_ctx *ctx,
 	struct ebt_entry *fw = &cs->eb;
 	uint32_t op;
 
-	if (fw->bitmask & EBT_ISOURCE) {
+	if (fw->bitmask & EBT_SOURCEMAC) {
 		op = nft_invflags2cmp(fw->invflags, EBT_ISOURCE);
 		add_addr(h, r, NFT_PAYLOAD_LL_HEADER,
 			 offsetof(struct ethhdr, h_source),
 			 fw->sourcemac, fw->sourcemsk, ETH_ALEN, op);
 	}
 
-	if (fw->bitmask & EBT_IDEST) {
+	if (fw->bitmask & EBT_DESTMAC) {
 		op = nft_invflags2cmp(fw->invflags, EBT_IDEST);
 		add_addr(h, r, NFT_PAYLOAD_LL_HEADER,
 			 offsetof(struct ethhdr, h_dest),
-- 
2.41.0

[iptables PATCH 03/12] xshared: struct xt_cmd_parse::xlate is unused

[Phil Sutter]

Drop the boolean, it was meant to disable some existence checks in
do_parse() prior to the caching rework. Now that do_parse() runs before
any caching is done, the checks in question don't exist anymore so drop
this relict.

Fixes: a7f1e208cdf9c ("nft: split parsing from netlink commands")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xshared.h           | 1 -
 iptables/xtables-translate.c | 1 -
 2 files changed, 2 deletions(-)

diff --git a/iptables/xshared.h b/iptables/xshared.h
index 5586385456a4d..c77556a1987dc 100644
--- a/iptables/xshared.h
+++ b/iptables/xshared.h
@@ -284,7 +284,6 @@ struct xt_cmd_parse {
 	bool				restore;
 	int				line;
 	int				verbose;
-	bool				xlate;
 	struct xt_cmd_parse_ops		*ops;
 };
 
diff --git a/iptables/xtables-translate.c b/iptables/xtables-translate.c
index ea9dce204dfc9..ad44311230323 100644
--- a/iptables/xtables-translate.c
+++ b/iptables/xtables-translate.c
@@ -259,7 +259,6 @@ static int do_command_xlate(struct nft_handle *h, int argc, char *argv[],
 		.table		= *table,
 		.restore	= restore,
 		.line		= line,
-		.xlate		= true,
 		.ops		= &h->ops->cmd_parse,
 	};
 	struct iptables_command_state cs = {
-- 
2.41.0

[iptables PATCH 04/12] xshared: All variants support -v

[Phil Sutter]

Update OPTSTRING_COMMON and remove the flag from *_OPTSTRING defines.

Fixes: 51d9d9e081344 ("ebtables: Support verbose mode")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xshared.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/iptables/xshared.h b/iptables/xshared.h
index c77556a1987dc..815b9d3e98726 100644
--- a/iptables/xshared.h
+++ b/iptables/xshared.h
@@ -75,10 +75,10 @@ struct xtables_globals;
 struct xtables_rule_match;
 struct xtables_target;
 
-#define OPTSTRING_COMMON "-:A:C:D:E:F::I:L::M:N:P:VX::Z::" "c:d:i:j:o:p:s:t:"
-#define IPT_OPTSTRING	OPTSTRING_COMMON "R:S::W::" "46bfg:h::m:nvw::x"
-#define ARPT_OPTSTRING	OPTSTRING_COMMON "R:S::" "h::l:nvx" /* "m:" */
-#define EBT_OPTSTRING	OPTSTRING_COMMON "hv"
+#define OPTSTRING_COMMON "-:A:C:D:E:F::I:L::M:N:P:VX::Z::" "c:d:i:j:o:p:s:t:v"
+#define IPT_OPTSTRING	OPTSTRING_COMMON "R:S::W::" "46bfg:h::m:nw::x"
+#define ARPT_OPTSTRING	OPTSTRING_COMMON "R:S::" "h::l:nx" /* "m:" */
+#define EBT_OPTSTRING	OPTSTRING_COMMON "h"
 
 /* define invflags which won't collide with IPT ones.
  * arptables-nft does NOT use the legacy ARPT_INV_* defines.
-- 
2.41.0

[iptables PATCH 05/12] xshared: Drop needless assignment in --help case

[Phil Sutter]

Help text printing code does not refer to optarg, so there is no need to
assign to it if unset.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xshared.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/iptables/xshared.c b/iptables/xshared.c
index 5f75a0a57a023..53e72b7abb1e8 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -1527,9 +1527,6 @@ void do_parse(int argc, char *argv[],
 			break;
 
 		case 'h':
-			if (!optarg)
-				optarg = argv[optind];
-
 			/* iptables -p icmp -h */
 			if (!cs->matches && cs->protocol)
 				xtables_find_match(cs->protocol,
-- 
2.41.0

[iptables PATCH 06/12] xshared: Drop pointless CMD_REPLACE check

[Phil Sutter]

All current users set default source and destination addresses in their
post_parse callbacks, so legacy variants are safe and nft variants don't
have this restriction anyway.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xshared.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/iptables/xshared.c b/iptables/xshared.c
index 53e72b7abb1e8..c4d7a266fed5e 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -1836,11 +1836,6 @@ void do_parse(int argc, char *argv[],
 	if (p->ops->post_parse)
 		p->ops->post_parse(p->command, cs, args);
 
-	if (p->command == CMD_REPLACE &&
-	    (args->s.naddrs != 1 || args->d.naddrs != 1))
-		xtables_error(PARAMETER_PROBLEM, "Replacement rule does not "
-			   "specify a unique address");
-
 	generic_opt_check(p->command, cs->options);
 
 	if (p->chain != NULL && strlen(p->chain) >= XT_EXTENSION_MAXNAMELEN)
-- 
2.41.0

[iptables PATCH 07/12] tests: xlate: Print failing command line

[Phil Sutter]

If the command segfaults, 'error' variable is empty and the resulting
error message is even misleading as the called program may not have been
iptables-translate.

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

diff --git a/xlate-test.py b/xlate-test.py
index ddd68b91d3a7e..b6a78bb250e27 100755
--- a/xlate-test.py
+++ b/xlate-test.py
@@ -41,9 +41,10 @@ xtables_nft_multi = 'xtables-nft-multi'
 
 
 def test_one_xlate(name, sourceline, expected, result):
-    rc, output, error = run_proc([xtables_nft_multi] + shlex.split(sourceline))
+    cmd = [xtables_nft_multi] + shlex.split(sourceline)
+    rc, output, error = run_proc(cmd)
     if rc != 0:
-        result.append(name + ": " + red("Error: ") + "iptables-translate failure")
+        result.append(name + ": " + red("Error: ") + "Call failed: " + " ".join(cmd))
         result.append(error)
         return False
 
-- 
2.41.0

[iptables PATCH 08/12] ebtables: Drop append_entry() wrapper

[Phil Sutter]

There is no point in having it when there is no code to share.

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

diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index a8ad57c735cc5..3fa5c179ba4b1 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -112,24 +112,6 @@ static int parse_rule_number(const char *rule)
 	return rule_nr;
 }
 
-static int
-append_entry(struct nft_handle *h,
-	     const char *chain,
-	     const char *table,
-	     struct iptables_command_state *cs,
-	     int rule_nr,
-	     bool verbose, bool append)
-{
-	int ret = 1;
-
-	if (append)
-		ret = nft_cmd_rule_append(h, chain, table, cs, verbose);
-	else
-		ret = nft_cmd_rule_insert(h, chain, table, cs, rule_nr, verbose);
-
-	return ret;
-}
-
 static int
 delete_entry(struct nft_handle *h,
 	     const char *chain,
@@ -1178,11 +1160,11 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table,
 	} else if (command == 'F') {
 		ret = nft_cmd_rule_flush(h, chain, *table, flags & OPT_VERBOSE);
 	} else if (command == 'A') {
-		ret = append_entry(h, chain, *table, &cs, 0,
-				   flags & OPT_VERBOSE, true);
+		ret = nft_cmd_rule_append(h, chain, *table, &cs,
+					  flags & OPT_VERBOSE);
 	} else if (command == 'I') {
-		ret = append_entry(h, chain, *table, &cs, rule_nr - 1,
-				   flags & OPT_VERBOSE, false);
+		ret = nft_cmd_rule_insert(h, chain, *table, &cs,
+					  rule_nr - 1, flags & OPT_VERBOSE);
 	} else if (command == 'D') {
 		ret = delete_entry(h, chain, *table, &cs, rule_nr - 1,
 				   rule_nr_end, flags & OPT_VERBOSE);
-- 
2.41.0

[iptables PATCH 09/12] ebtables: Make ebt_load_match_extensions() static

[Phil Sutter]

The function is not used outside of xtables-eb.c.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-bridge.h | 1 -
 iptables/xtables-eb.c | 2 +-
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/iptables/nft-bridge.h b/iptables/nft-bridge.h
index eb1b3928b6543..0e6a29650acca 100644
--- a/iptables/nft-bridge.h
+++ b/iptables/nft-bridge.h
@@ -115,7 +115,6 @@ static inline const char *ebt_target_name(unsigned int verdict)
 })								\
 
 void ebt_cs_clean(struct iptables_command_state *cs);
-void ebt_load_match_extensions(void);
 void ebt_add_match(struct xtables_match *m,
 			  struct iptables_command_state *cs);
 void ebt_add_watcher(struct xtables_target *watcher,
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index 3fa5c179ba4b1..cd45e0495ebcb 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -504,7 +504,7 @@ static void ebt_load_watcher(const char *name)
 		xtables_error(OTHER_PROBLEM, "Can't alloc memory");
 }
 
-void ebt_load_match_extensions(void)
+static void ebt_load_match_extensions(void)
 {
 	opts = ebt_original_options;
 	ebt_load_match("802_3");
-- 
2.41.0

[iptables PATCH 10/12] ebtables: Align line number formatting with legacy

[Phil Sutter]

Legacy ebtables appends a dot to the number printed in first column if
--Ln flag was given.

Fixes: da871de2a6efb ("nft: bootstrap ebtables-compat")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 772525e1b45a9..1fcdeaf2cad68 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -354,7 +354,7 @@ static void nft_bridge_print_rule(struct nft_handle *h, struct nftnl_rule *r,
 	struct iptables_command_state cs = {};
 
 	if (format & FMT_LINENUMBERS)
-		printf("%d ", num);
+		printf("%d. ", num);
 
 	nft_rule_to_ebtables_command_state(h, r, &cs);
 	__nft_bridge_save_rule(&cs, format);
-- 
2.41.0

[iptables PATCH 11/12] xshared: do_parse: Ignore '-j CONTINUE'

[Phil Sutter]

While iptables does not support his NOP, arptables man page claims it
does (although legacy arptables rejects it) and ebtables prefers to
print it instead of omitting the '-j' option.

Accept and ignore the target when parsing to at least fix for
arptables-nft and prepare for ebtables-nft using do_parse() as well.

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

diff --git a/iptables/xshared.c b/iptables/xshared.c
index c4d7a266fed5e..1b02f35a9de3a 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -1634,7 +1634,8 @@ void do_parse(int argc, char *argv[],
 		case 'j':
 			set_option(&cs->options, OPT_JUMP, &args->invflags,
 				   invert);
-			command_jump(cs, optarg);
+			if (strcmp(optarg, "CONTINUE"))
+				command_jump(cs, optarg);
 			break;
 
 		case 'i':
-- 
2.41.0

[iptables PATCH 12/12] ebtables: Implement --change-counters command

[Phil Sutter]

Treat it like --replace against the same rule with changed counters.
The operation is obviously not atomic, so rule counters may change in
kernel while the rule is fetched, modified and replaced.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-cmd.c                            | 20 +++++
 iptables/nft-cmd.h                            | 12 +++
 iptables/nft.c                                | 65 ++++++++++++++++
 iptables/nft.h                                |  1 +
 .../testcases/ebtables/0010-change-counters_0 | 45 +++++++++++
 iptables/xtables-eb.c                         | 74 ++++++++++++++-----
 6 files changed, 197 insertions(+), 20 deletions(-)
 create mode 100755 iptables/tests/shell/testcases/ebtables/0010-change-counters_0

diff --git a/iptables/nft-cmd.c b/iptables/nft-cmd.c
index 8a824586ad8c3..8372d171b00c4 100644
--- a/iptables/nft-cmd.c
+++ b/iptables/nft-cmd.c
@@ -400,3 +400,23 @@ int ebt_cmd_user_chain_policy(struct nft_handle *h, const char *table,
 
 	return 1;
 }
+
+int nft_cmd_rule_change_counters(struct nft_handle *h,
+				 const char *chain, const char *table,
+				 struct iptables_command_state *cs,
+				 int rule_nr, uint8_t counter_op, bool verbose)
+{
+	struct nft_cmd *cmd;
+
+	cmd = nft_cmd_new(h, NFT_COMPAT_RULE_CHANGE_COUNTERS, table, chain,
+			  rule_nr == -1 ? cs : NULL, rule_nr, verbose);
+	if (!cmd)
+		return 0;
+
+	cmd->counter_op = counter_op;
+	cmd->counters = cs->counters;
+
+	nft_cache_level_set(h, NFT_CL_RULES, cmd);
+
+	return 1;
+}
diff --git a/iptables/nft-cmd.h b/iptables/nft-cmd.h
index ae5908d8d596b..8163b82c3511f 100644
--- a/iptables/nft-cmd.h
+++ b/iptables/nft-cmd.h
@@ -7,6 +7,13 @@
 
 struct nftnl_rule;
 
+enum {
+	CTR_OP_INC_PKTS = 1 << 0,
+	CTR_OP_DEC_PKTS = 1 << 1,
+	CTR_OP_INC_BYTES = 1 << 2,
+	CTR_OP_DEC_BYTES = 1 << 3,
+};
+
 struct nft_cmd {
 	struct list_head		head;
 	int				command;
@@ -22,6 +29,7 @@ struct nft_cmd {
 	} obj;
 	const char			*policy;
 	struct xt_counters		counters;
+	uint8_t				counter_op;
 	const char			*rename;
 	int				counters_save;
 	struct {
@@ -77,6 +85,10 @@ int nft_cmd_rule_list_save(struct nft_handle *h, const char *chain,
 			   const char *table, int rulenum, int counters);
 int ebt_cmd_user_chain_policy(struct nft_handle *h, const char *table,
 			      const char *chain, const char *policy);
+int nft_cmd_rule_change_counters(struct nft_handle *h,
+				 const char *chain, const char *table,
+				 struct iptables_command_state *cs,
+				 int rule_nr, uint8_t counter_op, bool verbose);
 void nft_cmd_table_new(struct nft_handle *h, const char *table);
 
 #endif /* _NFT_CMD_H_ */
diff --git a/iptables/nft.c b/iptables/nft.c
index 97fd4f49fdb4c..f536857829cd2 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -337,6 +337,7 @@ static int mnl_append_error(const struct nft_handle *h,
 	case NFT_COMPAT_RULE_REPLACE:
 	case NFT_COMPAT_RULE_DELETE:
 	case NFT_COMPAT_RULE_FLUSH:
+	case NFT_COMPAT_RULE_CHANGE_COUNTERS:
 		snprintf(tcr, sizeof(tcr), "rule in chain %s",
 			 nftnl_rule_get_str(o->rule, NFTNL_RULE_CHAIN));
 #if 0
@@ -2641,6 +2642,58 @@ int nft_rule_replace(struct nft_handle *h, const char *chain,
 	return ret;
 }
 
+static int nft_rule_change_counters(struct nft_handle *h, const char *table,
+				    const char *chain, struct nftnl_rule *rule,
+				    int rulenum, struct xt_counters *counters,
+				    uint8_t counter_op, bool verbose)
+{
+	struct iptables_command_state cs = {};
+	struct nftnl_rule *r, *new_rule;
+	struct nft_rule_ctx ctx = {
+		.command = NFT_COMPAT_RULE_APPEND,
+	};
+	struct nft_chain *c;
+
+	nft_fn = nft_rule_change_counters;
+
+	c = nft_chain_find(h, table, chain);
+	if (!c) {
+		errno = ENOENT;
+		return 0;
+	}
+
+	r = nft_rule_find(h, c, rule, rulenum);
+	if (!r) {
+		errno = E2BIG;
+		return 0;
+	}
+
+	DEBUGP("changing counters of rule with handle=%llu\n",
+		(unsigned long long)
+		nftnl_rule_get_u64(r, NFTNL_RULE_HANDLE));
+
+	h->ops->rule_to_cs(h, r, &cs);
+
+	if (counter_op & CTR_OP_INC_PKTS)
+		cs.counters.pcnt += counters->pcnt;
+	else if (counter_op & CTR_OP_DEC_PKTS)
+		cs.counters.pcnt -= counters->pcnt;
+	else
+		cs.counters.pcnt = counters->pcnt;
+
+	if (counter_op & CTR_OP_INC_BYTES)
+		cs.counters.bcnt += counters->bcnt;
+	else if (counter_op & CTR_OP_DEC_BYTES)
+		cs.counters.bcnt -= counters->bcnt;
+	else
+		cs.counters.bcnt = counters->bcnt;
+
+	new_rule = nft_rule_new(h, &ctx, chain, table, &cs);
+	h->ops->clear_cs(&cs);
+
+	return nft_rule_append(h, chain, table, new_rule, r, verbose);
+}
+
 static int
 __nft_rule_list(struct nft_handle *h, struct nftnl_chain *c,
 		int rulenum, unsigned int format,
@@ -3031,6 +3084,7 @@ static void batch_obj_del(struct nft_handle *h, struct obj_update *o)
 	case NFT_COMPAT_RULE_APPEND:
 	case NFT_COMPAT_RULE_INSERT:
 	case NFT_COMPAT_RULE_REPLACE:
+	case NFT_COMPAT_RULE_CHANGE_COUNTERS:
 		break;
 	case NFT_COMPAT_RULE_DELETE:
 	case NFT_COMPAT_RULE_FLUSH:
@@ -3118,6 +3172,7 @@ static void nft_refresh_transaction(struct nft_handle *h)
 		case NFT_COMPAT_RULE_APPEND:
 		case NFT_COMPAT_RULE_INSERT:
 		case NFT_COMPAT_RULE_REPLACE:
+		case NFT_COMPAT_RULE_CHANGE_COUNTERS:
 		case NFT_COMPAT_RULE_DELETE:
 		case NFT_COMPAT_SET_ADD:
 		case NFT_COMPAT_RULE_LIST:
@@ -3208,6 +3263,7 @@ static int nft_action(struct nft_handle *h, int action)
 						  n->rule);
 			break;
 		case NFT_COMPAT_RULE_REPLACE:
+		case NFT_COMPAT_RULE_CHANGE_COUNTERS:
 			nft_compat_rule_batch_add(h, NFT_MSG_NEWRULE,
 						  NLM_F_CREATE | NLM_F_REPLACE,
 						  n->seq, n->rule);
@@ -3510,6 +3566,15 @@ static int nft_prepare(struct nft_handle *h)
 		case NFT_COMPAT_CHAIN_ADD:
 			assert(0);
 			return 0;
+		case NFT_COMPAT_RULE_CHANGE_COUNTERS:
+			ret = nft_rule_change_counters(h, cmd->table,
+						       cmd->chain,
+						       cmd->obj.rule,
+						       cmd->rulenum,
+						       &cmd->counters,
+						       cmd->counter_op,
+						       cmd->verbose);
+			break;
 		}
 
 		nft_cmd_free(cmd);
diff --git a/iptables/nft.h b/iptables/nft.h
index 5acbbf82e2c29..79f1e037cd6d3 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -72,6 +72,7 @@ enum obj_update_type {
 	NFT_COMPAT_RULE_SAVE,
 	NFT_COMPAT_RULE_ZERO,
 	NFT_COMPAT_BRIDGE_USER_CHAIN_UPDATE,
+	NFT_COMPAT_RULE_CHANGE_COUNTERS,
 };
 
 struct cache_chain {
diff --git a/iptables/tests/shell/testcases/ebtables/0010-change-counters_0 b/iptables/tests/shell/testcases/ebtables/0010-change-counters_0
new file mode 100755
index 0000000000000..4f783819d10eb
--- /dev/null
+++ b/iptables/tests/shell/testcases/ebtables/0010-change-counters_0
@@ -0,0 +1,45 @@
+#!/bin/sh
+
+case "$XT_MULTI" in
+*xtables-nft-multi)
+	;;
+*)
+	echo "skip $XT_MULTI"
+	exit 0
+	;;
+esac
+
+set -e
+set -x
+
+check_rule() { # (pcnt, bcnt)
+	$XT_MULTI ebtables -L FORWARD --Lc --Ln | \
+		grep -q "^1. -o eth0 -j CONTINUE , pcnt = $1 -- bcnt = $2$"
+}
+
+$XT_MULTI ebtables -A FORWARD -o eth0 -c 10 20
+check_rule 10 20
+
+$XT_MULTI ebtables -C FORWARD 1 100 200
+check_rule 100 200
+
+$XT_MULTI ebtables -C FORWARD 101 201 -o eth0
+check_rule 101 201
+
+$XT_MULTI ebtables -C FORWARD 1 +10 -20
+check_rule 111 181
+
+$XT_MULTI ebtables -C FORWARD -10 +20 -o eth0
+check_rule 101 201
+
+$XT_MULTI ebtables -A FORWARD -o eth1 -c 111 211
+$XT_MULTI ebtables -A FORWARD -o eth2 -c 121 221
+
+$XT_MULTI ebtables -C FORWARD 2:3 +100 -200
+
+EXPECT='1. -o eth0 -j CONTINUE , pcnt = 101 -- bcnt = 201
+2. -o eth1 -j CONTINUE , pcnt = 211 -- bcnt = 11
+3. -o eth2 -j CONTINUE , pcnt = 221 -- bcnt = 21'
+diff -u <(echo "$EXPECT") \
+	<($XT_MULTI ebtables -L FORWARD --Lc --Ln | grep -- '-o eth')
+
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index cd45e0495ebcb..ddbe1b5a3adc0 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -136,6 +136,29 @@ delete_entry(struct nft_handle *h,
 	return ret;
 }
 
+static int
+change_entry_counters(struct nft_handle *h,
+		      const char *chain, const char *table,
+		      struct iptables_command_state *cs,
+		      int rule_nr, int rule_nr_end, uint8_t counter_op,
+		      bool verbose)
+{
+	int ret = 1;
+
+	if (rule_nr == -1)
+		return nft_cmd_rule_change_counters(h, chain, table, cs,
+						    rule_nr, counter_op,
+						    verbose);
+	do {
+		ret = nft_cmd_rule_change_counters(h, chain, table, cs,
+						   rule_nr, counter_op,
+						   verbose);
+		rule_nr++;
+	} while (rule_nr < rule_nr_end);
+
+	return ret;
+}
+
 int ebt_get_current_chain(const char *chain)
 {
 	if (!chain)
@@ -391,51 +414,62 @@ static int parse_rule_range(const char *argv, int *rule_nr, int *rule_nr_end)
 /* Incrementing or decrementing rules in daemon mode is not supported as the
  * involved code overload is not worth it (too annoying to take the increased
  * counters in the kernel into account). */
-static int parse_change_counters_rule(int argc, char **argv, int *rule_nr, int *rule_nr_end, struct iptables_command_state *cs)
+static uint8_t parse_change_counters_rule(int argc, char **argv,
+					  int *rule_nr, int *rule_nr_end,
+					  struct iptables_command_state *cs)
 {
+	uint8_t ret = 0;
 	char *buffer;
-	int ret = 0;
 
-	if (optind + 1 >= argc || argv[optind][0] == '-' || argv[optind + 1][0] == '-')
+	if (optind + 1 >= argc ||
+	    (argv[optind][0] == '-' && !isdigit(argv[optind][1])) ||
+	    (argv[optind + 1][0] == '-' && !isdigit(argv[optind + 1][1])))
 		xtables_error(PARAMETER_PROBLEM,
 			      "The command -C needs at least 2 arguments");
-	if (optind + 2 < argc && (argv[optind + 2][0] != '-' || (argv[optind + 2][1] >= '0' && argv[optind + 2][1] <= '9'))) {
+	if (optind + 2 < argc &&
+	    (argv[optind + 2][0] != '-' || isdigit(argv[optind + 2][1]))) {
 		if (optind + 3 != argc)
 			xtables_error(PARAMETER_PROBLEM,
 				      "No extra options allowed with -C start_nr[:end_nr] pcnt bcnt");
 		if (parse_rule_range(argv[optind], rule_nr, rule_nr_end))
 			xtables_error(PARAMETER_PROBLEM,
-				      "Something is wrong with the rule number specification '%s'", argv[optind]);
+				      "Something is wrong with the rule number specification '%s'",
+				      argv[optind]);
 		optind++;
 	}
 
 	if (argv[optind][0] == '+') {
-		ret += 1;
+		ret |= CTR_OP_INC_PKTS;
 		cs->counters.pcnt = strtoull(argv[optind] + 1, &buffer, 10);
 	} else if (argv[optind][0] == '-') {
-		ret += 2;
+		ret |= CTR_OP_DEC_PKTS;
 		cs->counters.pcnt = strtoull(argv[optind] + 1, &buffer, 10);
-	} else
+	} else {
 		cs->counters.pcnt = strtoull(argv[optind], &buffer, 10);
-
+	}
 	if (*buffer != '\0')
 		goto invalid;
+
 	optind++;
+
 	if (argv[optind][0] == '+') {
-		ret += 3;
+		ret |= CTR_OP_INC_BYTES;
 		cs->counters.bcnt = strtoull(argv[optind] + 1, &buffer, 10);
 	} else if (argv[optind][0] == '-') {
-		ret += 6;
+		ret |= CTR_OP_DEC_BYTES;
 		cs->counters.bcnt = strtoull(argv[optind] + 1, &buffer, 10);
-	} else
+	} else {
 		cs->counters.bcnt = strtoull(argv[optind], &buffer, 10);
-
+	}
 	if (*buffer != '\0')
 		goto invalid;
+
 	optind++;
+
 	return ret;
 invalid:
-	xtables_error(PARAMETER_PROBLEM,"Packet counter '%s' invalid", argv[optind]);
+	xtables_error(PARAMETER_PROBLEM,
+		      "Packet counter '%s' invalid", argv[optind]);
 }
 
 static void ebtables_parse_interface(const char *arg, char *vianame)
@@ -695,7 +729,7 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table,
 {
 	char *buffer;
 	int c, i;
-	int chcounter = 0; /* Needed for -C */
+	uint8_t chcounter = 0; /* Needed for -C */
 	int rule_nr = 0;
 	int rule_nr_end = 0;
 	int ret = 0;
@@ -1171,11 +1205,11 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table,
 	} else if (command == 14) {
 		ret = nft_cmd_rule_check(h, chain, *table,
 					 &cs, flags & OPT_VERBOSE);
-	} /*else if (replace->command == 'C') {
-		ebt_change_counters(replace, new_entry, rule_nr, rule_nr_end, &(new_entry->cnt_surplus), chcounter);
-		if (ebt_errormsg[0] != '\0')
-			return -1;
-	}*/
+	} else if (command == 'C') {
+		ret = change_entry_counters(h, chain, *table, &cs,
+					    rule_nr - 1, rule_nr_end, chcounter,
+					    flags & OPT_VERBOSE);
+	}
 
 	ebt_cs_clean(&cs);
 	return ret;
-- 
2.41.0

Re: [iptables PATCH 00/12] Misc fixes (more or less)

[Phil Sutter]

On Wed, Nov 22, 2023 at 02:02:10PM +0100, Phil Sutter wrote:
> This is early fallout from working on a merge of ebtables commandline
> parsers with the shared one. It is a mix of actual bug fixes, small
> improvements and an implementaion of --change-counters command for
> ebtables-nft.
> 
> Phil Sutter (12):
>   Makefile: Install arptables-translate link and man page
>   nft-bridge: nft_bridge_add() uses wrong flags
>   xshared: struct xt_cmd_parse::xlate is unused
>   xshared: All variants support -v, update OPTSTRING_COMMON
>   xshared: Drop needless assignment in --help case
>   xshared: Drop pointless CMD_REPLACE check
>   tests: xlate: Print failing command line
>   ebtables: Drop append_entry() wrapper
>   ebtables: Make ebt_load_match_extensions() static
>   ebtables: Align line number formatting with legacy
>   xshared: do_parse: Ignore '-j CONTINUE'
>   ebtables: Implement --change-counters command

Series applied.