[iptables-nftables PATCH 0/5] Centralizes rule parsing

[iptables-nftables PATCH 0/5] Centralizes rule parsing

[Tomasz Bursztyka]

Hi,

Here are the patches that refactors how rules are parsed. So now it's done in one unique place for all operations.

And it adds a function to reset the counters with -Z since it's trivial to do so with such parsing strategy.

Tomasz Bursztyka (5):
  nft: Parse fully and properly at once a rule into a cs
  nft: Refactor firewall printing so it reuses already parsed cs struct
  nft: Refactor rule deletion so it compares both cs structure
  xtables: nft: Complete refactoring on how rules are saved
  nft: Add a function to reset the counters of an existing rule

 iptables/nft-ipv4.c       |  99 ++++-----
 iptables/nft-ipv6.c       |  85 +++-----
 iptables/nft-shared.c     | 267 ++++++++++++------------
 iptables/nft-shared.h     |  16 +-
 iptables/nft.c            | 513 ++++++++++------------------------------------
 iptables/nft.h            |   5 +-
 iptables/xtables-events.c |  19 +-
 iptables/xtables.c        |  15 +-
 8 files changed, 342 insertions(+), 677 deletions(-)

-- 
1.8.3.2

[iptables-nftables PATCH 1/5] nft: Parse fully and properly at once a rule into a cs

[Tomasz Bursztyka]

This will help reducing code complexity in printing, saving, deleting
etc...

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 iptables/nft-shared.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 66 insertions(+), 5 deletions(-)

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index dd4766b..842523f 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -334,6 +334,57 @@ const char *nft_parse_target(struct nft_rule *r, const void **targinfo,
 	return targname;
 }
 
+static void
+_nft_parse_target(struct nft_rule_expr *e, struct nft_rule_expr_iter *iter,
+		 struct iptables_command_state *cs)
+{
+	size_t target_len;
+	const char *targname = nft_rule_expr_get_str(e, NFT_EXPR_TG_NAME);
+	const void *targinfo = nft_rule_expr_get(e,
+					NFT_EXPR_TG_INFO, &target_len);
+	struct xtables_target *target;
+	struct xt_entry_target *t;
+
+	target = xtables_find_target(targname, XTF_TRY_LOAD);
+	if (target == NULL)
+		return;
+
+	t = calloc(1, sizeof(struct xt_entry_target) + target_len);
+	memcpy(&t->data, targinfo, target_len);
+	t->u.target_size = target_len +
+				XT_ALIGN(sizeof(struct xt_entry_target));
+	t->u.user.revision = nft_rule_expr_get_u32(e, NFT_EXPR_TG_REV);
+	strcpy(t->u.user.name, target->name);
+
+	target->t = t;
+	cs->target = target;
+}
+
+static void
+nft_parse_match(struct nft_rule_expr *e, struct nft_rule_expr_iter *iter,
+		struct iptables_command_state *cs)
+{
+	size_t match_len;
+	const char *match_name = nft_rule_expr_get_str(e, NFT_EXPR_MT_NAME);
+	const void *match_info = nft_rule_expr_get(e,
+						NFT_EXPR_MT_INFO, &match_len);
+	struct xtables_match *match;
+	struct xt_entry_match *m;
+
+	match = xtables_find_match(match_name, XTF_TRY_LOAD, &cs->matches);
+	if (match == NULL)
+		return;
+
+	m = calloc(1, sizeof(struct xt_entry_match) + match_len);
+
+	memcpy(&m->data, match_info, match_len);
+	m->u.match_size = match_len + XT_ALIGN(sizeof(struct xt_entry_match));
+	m->u.user.revision = nft_rule_expr_get_u32(e, NFT_EXPR_TG_REV);
+	strcpy(m->u.user.name, match->name);
+
+	match->m = m;
+}
+
 void print_proto(uint16_t proto, int invert)
 {
 	const struct protoent *pent = getprotobynumber(proto);
@@ -460,20 +511,30 @@ void nft_rule_to_iptables_command_state(struct nft_rule *r,
 		const char *name =
 			nft_rule_expr_get_str(expr, NFT_RULE_EXPR_ATTR_NAME);
 
-		if (strcmp(name, "counter") == 0) {
+		if (strcmp(name, "counter") == 0)
 			nft_parse_counter(expr, iter, &cs->counters);
-		} else if (strcmp(name, "payload") == 0) {
+		else if (strcmp(name, "payload") == 0)
 			nft_parse_payload(expr, iter, family, cs);
-		} else if (strcmp(name, "meta") == 0) {
+		else if (strcmp(name, "meta") == 0)
 			nft_parse_meta(expr, iter, family, cs);
-		} else if (strcmp(name, "immediate") == 0) {
+		else if (strcmp(name, "immediate") == 0)
 			nft_parse_immediate(expr, iter, family, cs);
-		}
+		else if (strcmp(name, "target") == 0)
+			_nft_parse_target(expr, iter, cs);
+		else if (strcmp(name, "match") == 0)
+			nft_parse_match(expr, iter, cs);
 
 		expr = nft_rule_expr_iter_next(iter);
 	}
 
 	nft_rule_expr_iter_destroy(iter);
+
+	if (cs->target != NULL)
+		cs->jumpto = cs->target->name;
+	else if (cs->jumpto != NULL)
+		cs->target = xtables_find_target(cs->jumpto, XTF_TRY_LOAD);
+	else
+		cs->jumpto = "";
 }
 
 static void
-- 
1.8.3.2

[iptables-nftables PATCH 2/5] nft: Refactor firewall printing so it reuses already parsed cs struct

[Tomasz Bursztyka]

Now that we parse properly, in one place and at once, the rule back into
a command structure, it's now easier to print the rule from that command
structure.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 iptables/nft-ipv4.c   |  21 +------
 iptables/nft-ipv6.c   |  13 +----
 iptables/nft-shared.c | 156 +++++++-------------------------------------------
 iptables/nft-shared.h |   7 +--
 4 files changed, 26 insertions(+), 171 deletions(-)

diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index 81be9f4..a47f10b 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -121,14 +121,6 @@ static void get_frag(struct nft_rule_expr_iter *iter, bool *inv)
 		*inv = false;
 }
 
-static void print_frag(bool inv)
-{
-	if (inv)
-		printf("! -f ");
-	else
-		printf("-f ");
-}
-
 static const char *mask_to_str(uint32_t mask)
 {
 	static char mask_str[sizeof("255.255.255.255")];
@@ -288,15 +280,10 @@ static void nft_ipv4_print_firewall(struct nft_rule *r, unsigned int num,
 				    unsigned int format)
 {
 	struct iptables_command_state cs = {};
-	const char *targname = NULL;
-	const void *targinfo = NULL;
-	size_t target_len = 0;
 
 	nft_rule_to_iptables_command_state(r, &cs);
 
-	targname = nft_parse_target(r, &targinfo, &target_len);
-
-	print_firewall_details(&cs, targname, cs.fw.ip.flags,
+	print_firewall_details(&cs, cs.jumpto, cs.fw.ip.flags,
 			       cs.fw.ip.invflags, cs.fw.ip.proto,
 			       cs.fw.ip.iniface, cs.fw.ip.outiface,
 			       num, format);
@@ -311,11 +298,7 @@ static void nft_ipv4_print_firewall(struct nft_rule *r, unsigned int num,
 		printf("[goto] ");
 #endif
 
-	if (print_matches(r, format) != 0)
-		return;
-
-	if (print_target(targname, targinfo, target_len, format) != 0)
-		return;
+	print_matches_and_target(&cs, format);
 
 	if (!(format & FMT_NONEWLINE))
 		fputc('\n', stdout);
diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
index 0214dcf..deea451 100644
--- a/iptables/nft-ipv6.c
+++ b/iptables/nft-ipv6.c
@@ -198,15 +198,10 @@ static void nft_ipv6_print_firewall(struct nft_rule *r, unsigned int num,
 				    unsigned int format)
 {
 	struct iptables_command_state cs = {};
-	const char *targname = NULL;
-	const void *targinfo = NULL;
-	size_t target_len = 0;
 
 	nft_rule_to_iptables_command_state(r, &cs);
 
-	targname = nft_parse_target(r, &targinfo, &target_len);
-
-	print_firewall_details(&cs, targname, cs.fw6.ipv6.flags,
+	print_firewall_details(&cs, cs.jumpto, cs.fw6.ipv6.flags,
 			       cs.fw6.ipv6.invflags, cs.fw6.ipv6.proto,
 			       cs.fw6.ipv6.iniface, cs.fw6.ipv6.outiface,
 			       num, format);
@@ -221,11 +216,7 @@ static void nft_ipv6_print_firewall(struct nft_rule *r, unsigned int num,
 		printf("[goto] ");
 #endif
 
-	if (print_matches(r, format) != 0)
-		return;
-
-	if (print_target(targname, targinfo, target_len, format) != 0)
-		return;
+	print_matches_and_target(&cs, format);
 
 	if (!(format & FMT_NONEWLINE))
 		fputc('\n', stdout);
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 842523f..87de236 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -281,61 +281,8 @@ void parse_meta(struct nft_rule_expr *e, uint8_t key, char *iniface,
 	}
 }
 
-const char *nft_parse_target(struct nft_rule *r, const void **targinfo,
-			     size_t *target_len)
-{
-	struct nft_rule_expr_iter *iter;
-	struct nft_rule_expr *expr;
-	const char *targname = NULL;
-
-	iter = nft_rule_expr_iter_create(r);
-	if (iter == NULL)
-		return NULL;
-
-	expr = nft_rule_expr_iter_next(iter);
-	while (expr != NULL) {
-		const char *name =
-			nft_rule_expr_get_str(expr, NFT_RULE_EXPR_ATTR_NAME);
-
-		if (strcmp(name, "target") == 0) {
-			targname = nft_rule_expr_get_str(expr,
-							NFT_EXPR_TG_NAME);
-			*targinfo = nft_rule_expr_get(expr, NFT_EXPR_TG_INFO,
-								target_len);
-			break;
-		} else if (strcmp(name, "immediate") == 0) {
-			uint32_t verdict =
-			nft_rule_expr_get_u32(expr, NFT_EXPR_IMM_VERDICT);
-
-			switch(verdict) {
-			case NF_ACCEPT:
-				targname = "ACCEPT";
-				break;
-			case NF_DROP:
-				targname = "DROP";
-				break;
-			case NFT_RETURN:
-				targname = "RETURN";
-				break;
-			case NFT_GOTO:
-				targname = nft_rule_expr_get_str(expr,
-							NFT_EXPR_IMM_CHAIN);
-				break;
-			case NFT_JUMP:
-				targname = nft_rule_expr_get_str(expr,
-							NFT_EXPR_IMM_CHAIN);
-			break;
-			}
-		}
-		expr = nft_rule_expr_iter_next(iter);
-	}
-	nft_rule_expr_iter_destroy(iter);
-
-	return targname;
-}
-
 static void
-_nft_parse_target(struct nft_rule_expr *e, struct nft_rule_expr_iter *iter,
+nft_parse_target(struct nft_rule_expr *e, struct nft_rule_expr_iter *iter,
 		 struct iptables_command_state *cs)
 {
 	size_t target_len;
@@ -520,7 +467,7 @@ void nft_rule_to_iptables_command_state(struct nft_rule *r,
 		else if (strcmp(name, "immediate") == 0)
 			nft_parse_immediate(expr, iter, family, cs);
 		else if (strcmp(name, "target") == 0)
-			_nft_parse_target(expr, iter, cs);
+			nft_parse_target(expr, iter, cs);
 		else if (strcmp(name, "match") == 0)
 			nft_parse_match(expr, iter, cs);
 
@@ -537,87 +484,6 @@ void nft_rule_to_iptables_command_state(struct nft_rule *r,
 		cs->jumpto = "";
 }
 
-static void
-print_match(struct nft_rule_expr *expr, int numeric)
-{
-	size_t len;
-	const char *match_name = nft_rule_expr_get_str(expr, NFT_EXPR_MT_NAME);
-	const void *match_info = nft_rule_expr_get(expr, NFT_EXPR_MT_INFO, &len);
-	const struct xtables_match *match =
-		xtables_find_match(match_name, XTF_TRY_LOAD, NULL);
-	struct xt_entry_match *m =
-		calloc(1, sizeof(struct xt_entry_match) + len);
-
-	/* emulate struct xt_entry_match since ->print needs it */
-	memcpy((void *)&m->data, match_info, len);
-
-	if (match) {
-		if (match->print)
-			/* FIXME missing first parameter */
-			match->print(NULL, m, numeric);
-		else
-			printf("%s ", match_name);
-	} else {
-		if (match_name[0])
-			printf("UNKNOWN match `%s' ", match_name);
-	}
-
-	free(m);
-}
-
-int print_matches(struct nft_rule *r, int format)
-{
-	struct nft_rule_expr_iter *iter;
-	struct nft_rule_expr *expr;
-
-	iter = nft_rule_expr_iter_create(r);
-	if (iter == NULL)
-		return -ENOMEM;
-
-	expr = nft_rule_expr_iter_next(iter);
-	while (expr != NULL) {
-		const char *name =
-			nft_rule_expr_get_str(expr, NFT_RULE_EXPR_ATTR_NAME);
-
-		if (strcmp(name, "match") == 0)
-			print_match(expr, format & FMT_NUMERIC);
-
-		expr = nft_rule_expr_iter_next(iter);
-	}
-	nft_rule_expr_iter_destroy(iter);
-
-	return 0;
-}
-
-int print_target(const char *targname, const void *targinfo,
-		 size_t target_len, int format)
-{
-	struct xtables_target *target;
-	struct xt_entry_target *t;
-
-	if (targname == NULL)
-		return 0;
-
-	t = calloc(1, sizeof(struct xt_entry_target) + target_len);
-	if (t == NULL)
-		return -ENOMEM;
-
-	/* emulate struct xt_entry_target since ->print needs it */
-	memcpy((void *)&t->data, targinfo, target_len);
-
-	target = xtables_find_target(targname, XTF_TRY_LOAD);
-	if (target) {
-		if (target->print)
-			/* FIXME missing first parameter */
-			target->print(NULL, t, format & FMT_NUMERIC);
-	} else if (target_len > 0)
-		printf("[%ld bytes of unknown target data] ", target_len);
-
-	free(t);
-
-	return 0;
-}
-
 void print_num(uint64_t number, unsigned int format)
 {
 	if (format & FMT_KILOMEGAGIGA) {
@@ -707,6 +573,24 @@ void print_firewall_details(const struct iptables_command_state *cs,
 	}
 }
 
+void print_matches_and_target(struct iptables_command_state *cs,
+			      unsigned int format)
+{
+	struct xtables_rule_match *matchp;
+
+	for (matchp = cs->matches; matchp; matchp = matchp->next) {
+		if (matchp->match->print != NULL)
+			matchp->match->print(NULL, matchp->match->m,
+							format & FMT_NUMERIC);
+	}
+
+	if (cs->target != NULL) {
+		if (cs->target->print != NULL)
+			cs->target->print(NULL, cs->target->t,
+							format & FMT_NUMERIC);
+	}
+}
+
 struct nft_family_ops *nft_family_ops_lookup(int family)
 {
 	switch (family) {
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index 488ed63..a4eec04 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -79,22 +79,19 @@ bool is_same_interfaces(const char *a_iniface, const char *a_outiface,
 void parse_meta(struct nft_rule_expr *e, uint8_t key, char *iniface,
 		unsigned char *iniface_mask, char *outiface,
 		unsigned char *outiface_mask, uint8_t *invflags);
-const char *nft_parse_target(struct nft_rule *r, const void **targinfo,
-			     size_t *target_len);
 void print_proto(uint16_t proto, int invert);
 void get_cmp_data(struct nft_rule_expr_iter *iter,
 		  void *data, size_t dlen, bool *inv);
 void nft_rule_to_iptables_command_state(struct nft_rule *r,
 					struct iptables_command_state *cs);
-int print_matches(struct nft_rule *r, int format);
-int print_target(const char *targname, const void *targinfo,
-		 size_t target_len, int format);
 void print_num(uint64_t number, unsigned int format);
 void print_firewall_details(const struct iptables_command_state *cs,
 			    const char *targname, uint8_t flags,
 			    uint8_t invflags, uint8_t proto,
 			    const char *iniface, const char *outiface,
 			    unsigned int num, unsigned int format);
+void print_matches_and_target(struct iptables_command_state *cs,
+			      unsigned int format);
 
 struct nft_family_ops *nft_family_ops_lookup(int family);
 
-- 
1.8.3.2

[iptables-nftables PATCH 3/5] nft: Refactor rule deletion so it compares both cs structure

[Tomasz Bursztyka]

Now that we parse properly, in one place and at once, the rule back into a
command structure, it's now easier to compare the rule from that command
structure when deleting.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 iptables/nft.c | 195 +++++++++++----------------------------------------------
 1 file changed, 35 insertions(+), 160 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 28e71d8..e12a229 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1661,188 +1661,63 @@ next:
 	return 0;
 }
 
-static int matches_howmany(struct xtables_rule_match *matches)
-{
-	struct xtables_rule_match *matchp;
-	int matches_ctr = 0;
-
-	for (matchp = matches; matchp; matchp = matchp->next)
-		matches_ctr++;
-
-	return matches_ctr;
-}
-
 static bool
-__find_match(struct nft_rule_expr *expr, struct xtables_rule_match *matches)
+compare_matches(struct xtables_rule_match *matches_1,
+		struct xtables_rule_match *matches_2)
 {
-	const char *matchname = nft_rule_expr_get_str(expr, NFT_EXPR_MT_NAME);
-	/* Netlink aligns this match info, don't trust this length variable */
-	const char *data = nft_rule_expr_get_str(expr, NFT_EXPR_MT_INFO);
-	struct xtables_rule_match *matchp;
-	bool found = false;
+	struct xtables_rule_match *mp_1;
+	struct xtables_rule_match *mp_2;
 
-	for (matchp = matches; matchp; matchp = matchp->next) {
-		struct xt_entry_match *m = matchp->match->m;
+	for (mp_1 = matches_1, mp_2 = matches_2;
+			mp_1 && mp_2; mp_1 = mp_1->next, mp_2 = mp_2->next) {
+		struct xt_entry_match *m_1 = mp_1->match->m;
+		struct xt_entry_match *m_2 = mp_2->match->m;
 
-		if (strcmp(m->u.user.name, matchname) != 0) {
+		if (strcmp(m_1->u.user.name, m_2->u.user.name) != 0) {
 			DEBUGP("mismatching match name\n");
-			continue;
+			return false;
 		}
 
-		if (memcmp(data, m->data, m->u.user.match_size - sizeof(*m)) != 0) {
-			DEBUGP("mismatch match data\n");
-			continue;
+		if (m_1->u.user.match_size != m_2->u.user.match_size) {
+			DEBUGP("mismatching match size\n");
+			return false;
 		}
-		found = true;
-		break;
-	}
-
-	return found;
-}
-
-static bool find_matches(struct xtables_rule_match *matches, struct nft_rule *r)
-{
-	struct nft_rule_expr_iter *iter;
-	struct nft_rule_expr *expr;
-	int kernel_matches = 0;
-
-	iter = nft_rule_expr_iter_create(r);
-	if (iter == NULL)
-		return false;
 
-	expr = nft_rule_expr_iter_next(iter);
-	while (expr != NULL) {
-		const char *name =
-			nft_rule_expr_get_str(expr, NFT_RULE_EXPR_ATTR_NAME);
-
-		if (strcmp(name, "match") == 0) {
-			if (!__find_match(expr, matches))
-				return false;
-
-			kernel_matches++;
+		if (memcmp(m_1->data, m_2->data,
+				m_1->u.user.match_size - sizeof(*m_1)) != 0) {
+			DEBUGP("mismatch match data\n");
+			return false;
 		}
-		expr = nft_rule_expr_iter_next(iter);
 	}
-	nft_rule_expr_iter_destroy(iter);
 
-	/* same number of matches? */
-	if (matches_howmany(matches) != kernel_matches)
-		return false;
-
-	return true;
-}
-
-static bool __find_target(struct nft_rule_expr *expr, struct xt_entry_target *t)
-{
-	size_t len;
-	const char *tgname = nft_rule_expr_get_str(expr, NFT_EXPR_TG_NAME);
-	/* Netlink aligns this target info, don't trust this length variable */
-	const char *data = nft_rule_expr_get(expr, NFT_EXPR_TG_INFO, &len);
-
-	if (strcmp(t->u.user.name, tgname) != 0) {
-		DEBUGP("mismatching target name\n");
+	/* Both cursor should be NULL */
+	if (mp_1 != mp_2) {
+		DEBUGP("mismatch matches amount\n");
 		return false;
 	}
 
-	if (memcmp(data, t->data,  t->u.user.target_size - sizeof(*t)) != 0)
-		return false;
-
 	return true;
 }
 
-static int targets_howmany(struct xtables_target *target)
-{
-	return target != NULL ? 1 : 0;
-}
-
 static bool
-find_target(struct xtables_target *target, struct nft_rule *r)
+compare_targets(struct xtables_target *target_1,
+		struct xtables_target *target_2)
 {
-	struct nft_rule_expr_iter *iter;
-	struct nft_rule_expr *expr;
-	int kernel_targets = 0;
-
-	/* Special case: we use native immediate expressions to emulated
-	 * standard targets. Also, we don't want to crash with no targets.
-	 */
-	if (target == NULL || strcmp(target->name, "standard") == 0)
+	if (target_1 == NULL && target_2 == NULL)
 		return true;
 
-	iter = nft_rule_expr_iter_create(r);
-	if (iter == NULL)
+	if ((target_1 == NULL && target_2 != NULL) ||
+				(target_1 != NULL && target_2 == NULL))
 		return false;
 
-	expr = nft_rule_expr_iter_next(iter);
-	while (expr != NULL) {
-		const char *name =
-			nft_rule_expr_get_str(expr, NFT_RULE_EXPR_ATTR_NAME);
-
-		if (strcmp(name, "target") == 0) {
-			/* we may support several targets in the future */
-			if (!__find_target(expr, target->t))
-				return false;
-
-			kernel_targets++;
-		}
-		expr = nft_rule_expr_iter_next(iter);
-	}
-	nft_rule_expr_iter_destroy(iter);
-
-	/* same number of targets? */
-	if (targets_howmany(target) != kernel_targets) {
-		DEBUGP("kernel targets is %d but we passed %d\n",
-		kernel_targets, targets_howmany(target));
+	if (strcmp(target_1->t->u.user.name, target_2->t->u.user.name) != 0)
 		return false;
-	}
 
-	return true;
-}
-
-static bool
-find_immediate(struct nft_rule *r, const char *jumpto)
-{
-	struct nft_rule_expr_iter *iter;
-	struct nft_rule_expr *expr;
-
-	iter = nft_rule_expr_iter_create(r);
-	if (iter == NULL)
+	if (memcmp(target_1->t->data, target_2->t->data,
+					target_1->t->u.user.target_size -
+						sizeof(*target_1->t)) != 0)
 		return false;
 
-	expr = nft_rule_expr_iter_next(iter);
-	while (expr != NULL) {
-		const char *name =
-			nft_rule_expr_get_str(expr, NFT_RULE_EXPR_ATTR_NAME);
-
-		if (strcmp(name, "immediate") == 0) {
-			int verdict = nft_rule_expr_get_u32(expr, NFT_EXPR_IMM_VERDICT);
-			const char *verdict_name = NULL;
-
-			/* No target specified but immediate shows up, this
-			 * is not the rule we are looking for.
-			 */
-			if (strlen(jumpto) == 0)
-				return false;
-
-			switch(verdict) {
-			case NF_ACCEPT:
-				verdict_name = "ACCEPT";
-				break;
-			case NF_DROP:
-				verdict_name = "DROP";
-				break;
-			case NFT_RETURN:
-				verdict_name = "RETURN";
-				break;
-			}
-
-			/* Standard target? */
-			if (verdict_name && strcmp(jumpto, verdict_name) != 0)
-				return false;
-		}
-		expr = nft_rule_expr_iter_next(iter);
-	}
-	nft_rule_expr_iter_destroy(iter);
-
 	return true;
 }
 
@@ -1921,18 +1796,18 @@ nft_rule_find(struct nft_rule_list *list, const char *chain, const char *table,
 			if (!ops->is_same(cs, &this))
 				goto next;
 
-			if (!find_matches(cs->matches, r)) {
-				DEBUGP("matches not found\n");
+			if (!compare_matches(cs->matches, this.matches)) {
+				DEBUGP("Different matches\n");
 				goto next;
 			}
 
-			if (!find_target(cs->target, r)) {
-				DEBUGP("target not found\n");
+			if (!compare_targets(cs->target, this.target)) {
+				DEBUGP("Different target\n");
 				goto next;
 			}
 
-			if (!find_immediate(r, cs->jumpto)) {
-				DEBUGP("immediate not found\n");
+			if (strcmp(cs->jumpto, this.jumpto) != 0) {
+				DEBUGP("Different verdict\n");
 				goto next;
 			}
 
-- 
1.8.3.2

[iptables-nftables PATCH 4/5] xtables: nft: Complete refactoring on how rules are saved

[Tomasz Bursztyka]

Now that we parse properly, in one place and at once, the rule back into a
command structure, it's now easier to save the rule from that command
structure.

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 iptables/nft-ipv4.c       |  78 ++++++-------
 iptables/nft-ipv6.c       |  72 +++++-------
 iptables/nft-shared.c     |  60 ++++++++++
 iptables/nft-shared.h     |   9 ++
 iptables/nft.c            | 283 +++++++---------------------------------------
 iptables/nft.h            |   4 +-
 iptables/xtables-events.c |  19 +---
 7 files changed, 182 insertions(+), 343 deletions(-)

diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index a47f10b..68d3887 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -147,50 +147,6 @@ static const char *mask_to_str(uint32_t mask)
 	return mask_str;
 }
 
-static void nft_ipv4_print_payload(struct nft_rule_expr *e,
-				  struct nft_rule_expr_iter *iter)
-{
-	uint32_t offset;
-	bool inv;
-
-	offset = nft_rule_expr_get_u32(e, NFT_EXPR_PAYLOAD_OFFSET);
-
-	switch(offset) {
-	struct in_addr addr;
-	uint8_t proto;
-
-	case offsetof(struct iphdr, saddr):
-		get_cmp_data(iter, &addr, sizeof(addr), &inv);
-		if (inv)
-			printf("! -s %s/%s ", inet_ntoa(addr),
-						mask_to_str(0xffffffff));
-		else
-			printf("-s %s/%s ", inet_ntoa(addr),
-						mask_to_str(0xffffffff));
-		break;
-	case offsetof(struct iphdr, daddr):
-		get_cmp_data(iter, &addr, sizeof(addr), &inv);
-		if (inv)
-			printf("! -d %s/%s ", inet_ntoa(addr),
-						mask_to_str(0xffffffff));
-		else
-			printf("-d %s/%s ", inet_ntoa(addr),
-						mask_to_str(0xffffffff));
-		break;
-	case offsetof(struct iphdr, protocol):
-		get_cmp_data(iter, &proto, sizeof(proto), &inv);
-		print_proto(proto, inv);
-		break;
-	case offsetof(struct iphdr, frag_off):
-		get_frag(iter, &inv);
-		print_frag(inv);
-		break;
-	default:
-		DEBUGP("unknown payload offset %d\n", offset);
-		break;
-	}
-}
-
 static void nft_ipv4_parse_meta(struct nft_rule_expr *e, uint8_t key,
 				struct iptables_command_state *cs)
 {
@@ -304,6 +260,38 @@ static void nft_ipv4_print_firewall(struct nft_rule *r, unsigned int num,
 		fputc('\n', stdout);
 }
 
+static void save_ipv4_addr(char letter, const struct in_addr *addr,
+			   uint32_t mask, int invert)
+{
+	if (!mask && !invert && !addr->s_addr)
+		return;
+
+	printf("%s-%c %s/%s ", invert ? "! " : "", letter,
+			inet_ntoa(*addr), mask_to_str(mask));
+}
+
+static uint8_t nft_ipv4_save_firewall(const struct iptables_command_state *cs,
+				      unsigned int format)
+{
+	save_firewall_details(cs, cs->fw.ip.invflags, cs->fw.ip.proto,
+				cs->fw.ip.iniface, cs->fw.ip.iniface_mask,
+				cs->fw.ip.outiface, cs->fw.ip.outiface_mask,
+				format);
+
+	if (cs->fw.ip.flags & IPT_F_FRAG) {
+		if (cs->fw.ip.invflags & IPT_INV_FRAG)
+			printf("! ");
+		printf("-f ");
+	}
+
+	save_ipv4_addr('s', &cs->fw.ip.src, cs->fw.ip.smsk.s_addr,
+					cs->fw.ip.invflags & IPT_INV_SRCIP);
+	save_ipv4_addr('d', &cs->fw.ip.dst, cs->fw.ip.dmsk.s_addr,
+					cs->fw.ip.invflags & IPT_INV_DSTIP);
+
+	return cs->fw.ip.flags;
+}
+
 static void nft_ipv4_post_parse(int command,
 				struct iptables_command_state *cs,
 				struct xtables_args *args)
@@ -353,10 +341,10 @@ static void nft_ipv4_post_parse(int command,
 struct nft_family_ops nft_family_ops_ipv4 = {
 	.add			= nft_ipv4_add,
 	.is_same		= nft_ipv4_is_same,
-	.print_payload		= nft_ipv4_print_payload,
 	.parse_meta		= nft_ipv4_parse_meta,
 	.parse_payload		= nft_ipv4_parse_payload,
 	.parse_immediate	= nft_ipv4_parse_immediate,
 	.print_firewall		= nft_ipv4_print_firewall,
+	.save_firewall		= nft_ipv4_save_firewall,
 	.post_parse		= nft_ipv4_post_parse,
 };
diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
index deea451..1f2e3c8 100644
--- a/iptables/nft-ipv6.c
+++ b/iptables/nft-ipv6.c
@@ -69,48 +69,6 @@ static bool nft_ipv6_is_same(const struct iptables_command_state *a,
 				  b->fw6.ipv6.outiface_mask);
 }
 
-static void nft_ipv6_print_payload(struct nft_rule_expr *e,
-				   struct nft_rule_expr_iter *iter)
-{
-	uint32_t offset;
-	bool inv;
-
-	offset = nft_rule_expr_get_u32(e, NFT_EXPR_PAYLOAD_OFFSET);
-
-	switch (offset) {
-	char addr_str[INET6_ADDRSTRLEN];
-	struct in6_addr addr;
-	uint8_t proto;
-	case offsetof(struct ip6_hdr, ip6_src):
-		get_cmp_data(iter, &addr, sizeof(addr), &inv);
-		inet_ntop(AF_INET6, &addr, addr_str, INET6_ADDRSTRLEN);
-
-		if (inv)
-			printf("! -s %s ", addr_str);
-		else
-			printf("-s %s ", addr_str);
-
-		break;
-	case offsetof(struct ip6_hdr, ip6_dst):
-		get_cmp_data(iter, &addr, sizeof(addr), &inv);
-		inet_ntop(AF_INET6, &addr, addr_str, INET6_ADDRSTRLEN);
-
-		if (inv)
-			printf("! -d %s ", addr_str);
-		else
-			printf("-d %s ", addr_str);
-
-		break;
-	case offsetof(struct ip6_hdr, ip6_nxt):
-		get_cmp_data(iter, &proto, sizeof(proto), &inv);
-		print_proto(proto, inv);
-		break;
-	default:
-		DEBUGP("unknown payload offset %d\n", offset);
-		break;
-	}
-}
-
 static void nft_ipv6_parse_meta(struct nft_rule_expr *e, uint8_t key,
 				struct iptables_command_state *cs)
 {
@@ -222,6 +180,34 @@ static void nft_ipv6_print_firewall(struct nft_rule *r, unsigned int num,
 		fputc('\n', stdout);
 }
 
+static void save_ipv6_addr(char letter, const struct in6_addr *addr,
+			   int invert)
+{
+	char addr_str[INET6_ADDRSTRLEN];
+
+	if (!invert && !IN6_IS_ADDR_UNSPECIFIED(addr))
+		return;
+
+	inet_ntop(AF_INET6, &addr, addr_str, INET6_ADDRSTRLEN);
+	printf("%s-%c %s ", invert ? "! " : "", letter, addr_str);
+}
+
+static uint8_t nft_ipv6_save_firewall(const struct iptables_command_state *cs,
+				      unsigned int format)
+{
+	save_firewall_details(cs, cs->fw6.ipv6.invflags, cs->fw6.ipv6.proto,
+			cs->fw6.ipv6.iniface, cs->fw6.ipv6.iniface_mask,
+			cs->fw6.ipv6.outiface, cs->fw6.ipv6.outiface_mask,
+			format);
+
+	save_ipv6_addr('s', &cs->fw6.ipv6.src,
+				cs->fw6.ipv6.invflags & IPT_INV_SRCIP);
+	save_ipv6_addr('d', &cs->fw6.ipv6.dst,
+				cs->fw6.ipv6.invflags & IPT_INV_DSTIP);
+
+	return cs->fw6.ipv6.flags;
+}
+
 /* These are invalid numbers as upper layer protocol */
 static int is_exthdr(uint16_t proto)
 {
@@ -291,10 +277,10 @@ static void nft_ipv6_post_parse(int command, struct iptables_command_state *cs,
 struct nft_family_ops nft_family_ops_ipv6 = {
 	.add			= nft_ipv6_add,
 	.is_same		= nft_ipv6_is_same,
-	.print_payload		= nft_ipv6_print_payload,
 	.parse_meta		= nft_ipv6_parse_meta,
 	.parse_payload		= nft_ipv6_parse_payload,
 	.parse_immediate	= nft_ipv6_parse_immediate,
 	.print_firewall		= nft_ipv6_print_firewall,
+	.save_firewall		= nft_ipv6_save_firewall,
 	.post_parse		= nft_ipv6_post_parse,
 };
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 87de236..e58ca87 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -573,6 +573,66 @@ void print_firewall_details(const struct iptables_command_state *cs,
 	}
 }
 
+static void
+print_iface(char letter, const char *iface, const unsigned char *mask,
+            int invert)
+{
+	unsigned int i;
+
+	if (mask[0] == 0)
+		return;
+
+	printf("%s-%c ", invert ? "! " : "", letter);
+
+	for (i = 0; i < IFNAMSIZ; i++) {
+		if (mask[i] != 0) {
+			if (iface[i] != '\0')
+				printf("%c", iface[i]);
+			} else {
+				if (iface[i-1] != '\0')
+					printf("+");
+				break;
+		}
+	}
+
+	printf(" ");
+}
+
+void save_firewall_details(const struct iptables_command_state *cs,
+			   uint8_t invflags, uint16_t proto,
+			   const char *iniface,
+			   unsigned const char *iniface_mask,
+			   const char *outiface,
+			   unsigned const char *outiface_mask,
+			   unsigned int format)
+{
+	if (!(format & FMT_NOCOUNTS)) {
+		printf("-c ");
+		print_num(cs->counters.pcnt, format);
+		print_num(cs->counters.bcnt, format);
+	}
+
+	if (iniface != NULL)
+		print_iface('i', iniface, iniface_mask,
+					invflags & IPT_INV_VIA_IN);
+
+	if (outiface != NULL)
+		print_iface('o', outiface, outiface_mask,
+					invflags & IPT_INV_VIA_OUT);
+
+	if (proto > 0) {
+		const struct protoent *pent = getprotobynumber(proto);
+
+		if (invflags & XT_INV_PROTO)
+			printf("! ");
+
+		if (pent)
+			printf("-p %s ", pent->p_name);
+		else
+			printf("-p %u ", proto);
+	}
+}
+
 void print_matches_and_target(struct iptables_command_state *cs,
 			      unsigned int format)
 {
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index a4eec04..e77b303 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -50,6 +50,8 @@ struct nft_family_ops {
 	void (*parse_immediate)(struct iptables_command_state *cs);
 	void (*print_firewall)(struct nft_rule *r, unsigned int num,
 			       unsigned int format);
+	uint8_t (*save_firewall)(const struct iptables_command_state *cs,
+				 unsigned int format);
 	void (*post_parse)(int command, struct iptables_command_state *cs,
 			   struct xtables_args *args);
 };
@@ -92,6 +94,13 @@ void print_firewall_details(const struct iptables_command_state *cs,
 			    unsigned int num, unsigned int format);
 void print_matches_and_target(struct iptables_command_state *cs,
 			      unsigned int format);
+void save_firewall_details(const struct iptables_command_state *cs,
+			   uint8_t invflags, uint16_t proto,
+			   const char *iniface,
+			   unsigned const char *iniface_mask,
+			   const char *outiface,
+			   unsigned const char *outiface_mask,
+			   unsigned int format);
 
 struct nft_family_ops *nft_family_ops_lookup(int family);
 
diff --git a/iptables/nft.c b/iptables/nft.c
index e12a229..c9d9e40 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -782,224 +782,27 @@ err:
 	return ret == 0 ? 1 : 0;
 }
 
-static void nft_match_save(struct nft_rule_expr *expr)
-{
-	const char *name;
-	const struct xtables_match *match;
-	struct xt_entry_match *emu;
-	const void *mtinfo;
-	size_t len;
-
-	name = nft_rule_expr_get_str(expr, NFT_EXPR_MT_NAME);
-
-	match = xtables_find_match(name, XTF_TRY_LOAD, NULL);
-	if (match == NULL)
-		return;
-
-	mtinfo = nft_rule_expr_get(expr, NFT_EXPR_MT_INFO, &len);
-	if (mtinfo == NULL)
-		return;
-
-	emu = calloc(1, sizeof(struct xt_entry_match) + len);
-	if (emu == NULL)
-		return;
-
-	memcpy(&emu->data, mtinfo, len);
-
-	if (match->alias)
-		printf("-m %s", match->alias(emu));
-	else
-		printf("-m %s", match->name);
-
-	/* FIXME missing parameter */
-	if (match->save)
-		match->save(NULL, emu);
-
-	printf(" ");
-
-	free(emu);
-}
-
-static void nft_target_save(struct nft_rule_expr *expr)
-{
-	const char *name;
-	const struct xtables_target *target;
-	struct xt_entry_target *emu;
-	const void *tginfo;
-	size_t len;
-
-	name = nft_rule_expr_get_str(expr, NFT_EXPR_TG_NAME);
-
-	/* Standard target not supported, we use native immediate expression */
-	if (strcmp(name, "") == 0) {
-		printf("ERROR: standard target seen, should not happen\n");
-		return;
-	}
-
-	target = xtables_find_target(name, XTF_TRY_LOAD);
-	if (target == NULL)
-		return;
-
-	tginfo = nft_rule_expr_get(expr, NFT_EXPR_TG_INFO, &len);
-	if (tginfo == NULL)
-		return;
-
-	emu = calloc(1, sizeof(struct xt_entry_match) + len);
-	if (emu == NULL)
-		return;
-
-	memcpy(emu->data, tginfo, len);
-
-	if (target->alias)
-		printf("-j %s", target->alias(emu));
-	else
-		printf("-j %s", target->name);
-
-	/* FIXME missing parameter */
-	if (target->save)
-		target->save(NULL, emu);
-
-	free(emu);
-}
-
-static void nft_immediate_save(struct nft_rule_expr *expr)
-{
-	uint32_t verdict;
-
-	verdict = nft_rule_expr_get_u32(expr, NFT_EXPR_IMM_VERDICT);
-
-	switch(verdict) {
-	case NF_ACCEPT:
-		printf("-j ACCEPT");
-		break;
-	case NF_DROP:
-		printf("-j DROP");
-		break;
-	case NFT_RETURN:
-		printf("-j RETURN");
-		break;
-	case NFT_GOTO:
-		printf("-g %s",
-			nft_rule_expr_get_str(expr, NFT_EXPR_IMM_CHAIN));
-		break;
-	case NFT_JUMP:
-		printf("-j %s",
-			nft_rule_expr_get_str(expr, NFT_EXPR_IMM_CHAIN));
-		break;
-	}
-}
-
-static void
-nft_print_meta(struct nft_rule_expr *e, struct nft_rule_expr_iter *iter)
+void
+nft_rule_print_save(const struct iptables_command_state *cs,
+		    struct nft_rule *r, enum nft_rule_print type,
+		    unsigned int format)
 {
-	uint8_t key = nft_rule_expr_get_u8(e, NFT_EXPR_META_KEY);
-	uint32_t value;
-	const char *name;
-	char ifname[IFNAMSIZ];
-	const char *ifname_ptr;
-	size_t len;
-
-	e = nft_rule_expr_iter_next(iter);
-	if (e == NULL)
-		return;
-
-	name = nft_rule_expr_get_str(e, NFT_RULE_EXPR_ATTR_NAME);
-	/* meta should be followed by cmp */
-	if (strcmp(name, "cmp") != 0) {
-		DEBUGP("skipping no cmp after meta\n");
-		return;
-	}
-
-	switch(key) {
-	case NFT_META_IIF:
-		value = nft_rule_expr_get_u32(e, NFT_EXPR_CMP_DATA);
-		if_indextoname(value, ifname);
-
-		switch(nft_rule_expr_get_u8(e, NFT_EXPR_CMP_OP)) {
-		case NFT_CMP_EQ:
-			printf("-i %s ", ifname);
-			break;
-		case NFT_CMP_NEQ:
-			printf("! -i %s ", ifname);
-			break;
-		}
-		break;
-	case NFT_META_OIF:
-		value = nft_rule_expr_get_u32(e, NFT_EXPR_CMP_DATA);
-		if_indextoname(value, ifname);
-
-		switch(nft_rule_expr_get_u8(e, NFT_EXPR_CMP_OP)) {
-		case NFT_CMP_EQ:
-			printf("-o %s ", ifname);
-			break;
-		case NFT_CMP_NEQ:
-			printf("! -o %s ", ifname);
-			break;
-		}
-		break;
-	case NFT_META_IIFNAME:
-		ifname_ptr = nft_rule_expr_get(e, NFT_EXPR_CMP_DATA, &len);
-		memcpy(ifname, ifname_ptr, len);
-		ifname[len] = '\0';
-
-		/* if this is zero, then assume this is a interface mask */
-		if (if_nametoindex(ifname) == 0) {
-			ifname[len] = '+';
-			ifname[len+1] = '\0';
-		}
+	const char *chain = nft_rule_attr_get_str(r, NFT_RULE_ATTR_CHAIN);
+	int family = nft_rule_attr_get_u8(r, NFT_RULE_ATTR_FAMILY);
+	struct xtables_rule_match *matchp;
+	struct nft_family_ops *ops;
+	int ip_flags = 0;
 
-		switch(nft_rule_expr_get_u8(e, NFT_EXPR_CMP_OP)) {
-		case NFT_CMP_EQ:
-			printf("-i %s ", ifname);
-			break;
-		case NFT_CMP_NEQ:
-			printf("! -i %s ", ifname);
-			break;
-		}
+	switch(family) {
+	case AF_INET:
+		printf("-4 ");
 		break;
-	case NFT_META_OIFNAME:
-		ifname_ptr = nft_rule_expr_get(e, NFT_EXPR_CMP_DATA, &len);
-		memcpy(ifname, ifname_ptr, len);
-		ifname[len] = '\0';
-
-		/* if this is zero, then assume this is a interface mask */
-		if (if_nametoindex(ifname) == 0) {
-			ifname[len] = '+';
-			ifname[len+1] = '\0';
-		}
-
-		switch(nft_rule_expr_get_u8(e, NFT_EXPR_CMP_OP)) {
-		case NFT_CMP_EQ:
-			printf("-o %s ", ifname);
-			break;
-		case NFT_CMP_NEQ:
-			printf("! -o %s ", ifname);
-			break;
-		}
+	case AF_INET6:
+		printf("-6 ");
 		break;
 	default:
-		DEBUGP("unknown meta key %d\n", key);
 		break;
 	}
-}
-
-static void
-nft_print_counters(struct nft_rule_expr *e, struct nft_rule_expr_iter *iter,
-		   bool counters)
-{
-	if (counters) {
-		printf("-c %"PRIu64" %"PRIu64" ",
-			nft_rule_expr_get_u64(e, NFT_EXPR_CTR_PACKETS),
-			nft_rule_expr_get_u64(e, NFT_EXPR_CTR_BYTES));
-	}
-}
-
-void
-nft_rule_print_save(struct nft_rule *r, enum nft_rule_print type, bool counters)
-{
-	struct nft_rule_expr_iter *iter;
-	struct nft_rule_expr *expr;
-	const char *chain = nft_rule_attr_get_str(r, NFT_RULE_ATTR_CHAIN);
 
 	/* print chain name */
 	switch(type) {
@@ -1011,33 +814,24 @@ nft_rule_print_save(struct nft_rule *r, enum nft_rule_print type, bool counters)
 		break;
 	}
 
-	iter = nft_rule_expr_iter_create(r);
-	if (iter == NULL)
-		return;
+	ops = nft_family_ops_lookup(family);
+	ip_flags = ops->save_firewall(cs, format);
 
-	expr = nft_rule_expr_iter_next(iter);
-	while (expr != NULL) {
-		const char *name =
-			nft_rule_expr_get_str(expr, NFT_RULE_EXPR_ATTR_NAME);
+	for (matchp = cs->matches; matchp; matchp = matchp->next) {
+		printf("-m %s", matchp->match->name);
+		if (matchp->match->save != NULL)
+			matchp->match->save(NULL, matchp->match->m);
+		printf(" ");
+	}
 
-		if (strcmp(name, "counter") == 0) {
-			nft_print_counters(expr, iter, counters);
-		} else if (strcmp(name, "payload") == 0) {
-			struct nft_family_ops *ops = nft_family_ops_lookup(
-				nft_rule_attr_get_u8(r, NFT_RULE_ATTR_FAMILY));
-			ops->print_payload(expr, iter);
-		} else if (strcmp(name, "meta") == 0) {
-			nft_print_meta(expr, iter);
-		} else if (strcmp(name, "match") == 0) {
-			nft_match_save(expr);
-		} else if (strcmp(name, "target") == 0) {
-			nft_target_save(expr);
-		} else if (strcmp(name, "immediate") == 0) {
-			nft_immediate_save(expr);
-		}
+	if (cs->target != NULL) {
+		printf("-j %s", cs->jumpto);
 
-		expr = nft_rule_expr_iter_next(iter);
-	}
+		if (cs->target->save != NULL)
+			cs->target->save(NULL, cs->target->t);
+	} else if (strlen(cs->jumpto) > 0)
+		printf("-%c %s", ip_flags & IPT_F_GOTO ? 'g' : 'j',
+								cs->jumpto);
 
 	printf("\n");
 }
@@ -1219,11 +1013,15 @@ int nft_rule_save(struct nft_handle *h, const char *table, bool counters)
 	while (r != NULL) {
 		const char *rule_table =
 			nft_rule_attr_get_str(r, NFT_RULE_ATTR_TABLE);
+		struct iptables_command_state cs = {};
 
 		if (strcmp(table, rule_table) != 0)
 			goto next;
 
-		nft_rule_print_save(r, NFT_RULE_APPEND, counters);
+		nft_rule_to_iptables_command_state(r, &cs);
+
+		nft_rule_print_save(&cs, r, NFT_RULE_APPEND,
+						counters ? 0 : FMT_NOCOUNTS);
 
 next:
 		r = nft_rule_list_iter_next(iter);
@@ -1786,13 +1584,12 @@ nft_rule_find(struct nft_rule_list *list, const char *chain, const char *table,
 			break;
 		} else {
 			/* Delete by matching rule case */
+			nft_rule_to_iptables_command_state(r, &this);
+
 			DEBUGP("comparing with... ");
 #ifdef DEBUG_DEL
-			nft_rule_print_save(r, NFT_RULE_APPEND, 0);
+			nft_rule_print_save(&this, r, NFT_RULE_APPEND, 0);
 #endif
-
-			nft_rule_to_iptables_command_state(r, &this);
-
 			if (!ops->is_same(cs, &this))
 				goto next;
 
@@ -2199,7 +1996,11 @@ err:
 static void
 list_save(struct nft_rule *r, unsigned int num, unsigned int format)
 {
-	nft_rule_print_save(r, NFT_RULE_APPEND, !(format & FMT_NOCOUNTS));
+	struct iptables_command_state cs = {};
+
+	nft_rule_to_iptables_command_state(r, &cs);
+
+	nft_rule_print_save(&cs, r, NFT_RULE_APPEND, !(format & FMT_NOCOUNTS));
 }
 
 static int
diff --git a/iptables/nft.h b/iptables/nft.h
index f3317c9..006c031 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -87,7 +87,9 @@ enum nft_rule_print {
 	NFT_RULE_DEL,
 };
 
-void nft_rule_print_save(struct nft_rule *r, enum nft_rule_print type, bool counters);
+void nft_rule_print_save(const struct iptables_command_state *cs,
+			 struct nft_rule *r, enum nft_rule_print type,
+			 unsigned int format);
 
 /*
  * global commit and abort
diff --git a/iptables/xtables-events.c b/iptables/xtables-events.c
index 64ae972..1a4805f 100644
--- a/iptables/xtables-events.c
+++ b/iptables/xtables-events.c
@@ -58,6 +58,7 @@ static bool counters;
 
 static int rule_cb(const struct nlmsghdr *nlh, int type)
 {
+	struct iptables_command_state cs = {};
 	struct nft_rule *r;
 
 	r = nft_rule_alloc();
@@ -71,20 +72,12 @@ static int rule_cb(const struct nlmsghdr *nlh, int type)
 		goto err_free;
 	}
 
-	switch(nft_rule_attr_get_u8(r, NFT_RULE_ATTR_FAMILY)) {
-	case AF_INET:
-		printf("-4 ");
-		break;
-	case AF_INET6:
-		printf("-6 ");
-		break;
-	default:
-		break;
-	}
+	nft_rule_to_iptables_command_state(r, &cs);
 
-	nft_rule_print_save(r, type == NFT_MSG_NEWRULE ? NFT_RULE_APPEND :
-							 NFT_RULE_DEL,
-			    counters);
+	nft_rule_print_save(&cs, r,
+				type == NFT_MSG_NEWRULE ?
+					NFT_RULE_APPEND : NFT_RULE_DEL,
+				counters ? 0 : FMT_NOCOUNTS);
 err_free:
 	nft_rule_free(r);
 err:
-- 
1.8.3.2

[iptables-nftables PATCH 5/5] nft: Add a function to reset the counters of an existing rule

[Tomasz Bursztyka]

Now that we parse properly, in one place and at once, the rule back into a
command structure, it's now easier to reset its counters from that command
structure which we can pass again to nft_rule_append. (Thus the rule will
be replaced since we provide it's handle.)

Signed-off-by: Tomasz Bursztyka <tomasz.bursztyka@linux.intel.com>
---
 iptables/nft.c     | 35 +++++++++++++++++++++++++++++++++++
 iptables/nft.h     |  1 +
 iptables/xtables.c | 15 +++++++--------
 3 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index c9d9e40..abfe345 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -2098,6 +2098,41 @@ err:
 	return ret;
 }
 
+int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
+			   const char *table, int rulenum)
+{
+	struct iptables_command_state cs = {};
+	struct nft_rule_list *list;
+	struct nft_rule *r;
+	int ret = 0;
+
+	nft_fn = nft_rule_delete;
+
+	list = nft_rule_list_create(h);
+	if (list == NULL)
+		return 0;
+
+	r = nft_rule_find(list, chain, table, NULL, rulenum);
+	if (r == NULL) {
+		errno = ENOENT;
+		ret = 1;
+
+		goto error;
+	}
+
+	nft_rule_to_iptables_command_state(r, &cs);
+
+	cs.counters.pcnt = cs.counters.bcnt = 0;
+
+	ret =  nft_rule_append(h, chain, table, &cs,
+			nft_rule_attr_get_u64(r, NFT_RULE_ATTR_HANDLE), false);
+
+error:
+	nft_rule_list_destroy(list);
+
+	return ret;
+}
+
 static int nft_action(struct nft_handle *h, int type)
 {
 	char buf[MNL_SOCKET_BUFFER_SIZE];
diff --git a/iptables/nft.h b/iptables/nft.h
index 006c031..fe1b9c8 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -81,6 +81,7 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table, in
 int nft_rule_list_save(struct nft_handle *h, const char *chain, const char *table, int rulenum, int counters);
 int nft_rule_save(struct nft_handle *h, const char *table, bool counters);
 int nft_rule_flush(struct nft_handle *h, const char *chain, const char *table);
+int nft_rule_zero_counters(struct nft_handle *h, const char *chain, const char *table, int rulenum);
 
 enum nft_rule_print {
 	NFT_RULE_APPEND,
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 3e6092f..f47f9df 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -1173,8 +1173,7 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table)
 		ret = nft_chain_zero_counters(h, chain, *table);
 		break;
 	case CMD_ZERO_NUM:
-		/* FIXME */
-//		ret = iptc_zero_counter(chain, rulenum, *handle);
+		ret = nft_rule_zero_counters(h, chain, *table, rulenum - 1);
 		break;
 	case CMD_LIST:
 	case CMD_LIST|CMD_ZERO:
@@ -1187,9 +1186,9 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table)
 				   cs.options&OPT_LINENUMBERS);
 		if (ret && (command & CMD_ZERO))
 			ret = nft_chain_zero_counters(h, chain, *table);
-		/* FIXME */
-/*		if (ret && (command & CMD_ZERO_NUM))
-			ret = iptc_zero_counter(chain, rulenum, *handle); */
+		if (ret && (command & CMD_ZERO_NUM))
+			ret = nft_rule_zero_counters(h, chain,
+							*table, rulenum - 1);
 		break;
 	case CMD_LIST_RULES:
 	case CMD_LIST_RULES|CMD_ZERO:
@@ -1197,9 +1196,9 @@ int do_commandx(struct nft_handle *h, int argc, char *argv[], char **table)
 		ret = list_rules(h, chain, *table, rulenum, cs.options&OPT_VERBOSE);
 		if (ret && (command & CMD_ZERO))
 			ret = nft_chain_zero_counters(h, chain, *table);
-		/* FIXME */
-/*		if (ret && (command & CMD_ZERO_NUM))
-			ret = iptc_zero_counter(chain, rulenum, *handle); */
+		if (ret && (command & CMD_ZERO_NUM))
+			ret = nft_rule_zero_counters(h, chain,
+							*table, rulenum - 1);
 		break;
 	case CMD_NEW_CHAIN:
 		ret = nft_chain_user_add(h, chain, *table);
-- 
1.8.3.2

Re: [iptables-nftables PATCH 0/5] Centralizes rule parsing

[Pablo Neira Ayuso]

On Mon, Aug 19, 2013 at 03:04:01PM +0300, Tomasz Bursztyka wrote:
> Hi,
> 
> Here are the patches that refactors how rules are parsed. So now it's done in one unique place for all operations.
> 
> And it adds a function to reset the counters with -Z since it's trivial to do so with such parsing strategy.
> 
> Tomasz Bursztyka (5):
>   nft: Parse fully and properly at once a rule into a cs
>   nft: Refactor firewall printing so it reuses already parsed cs struct
>   nft: Refactor rule deletion so it compares both cs structure
>   xtables: nft: Complete refactoring on how rules are saved

I have collapsed these four patches in one single, we need that the
repository remains consistent between patches, that includes that new
functions need to have a client in the same patch.

The patch that I applied includes several things that I manually
fixed.

* IPv6 address printing was not working.
* Remove -4/-6 from the xtables-save output, we need exactly the same
  output like iptables-save. It is only shown in xtables-events.
* Fix match/target aliasing, this one was not so obvious, as it's a
  relatively new thing.
* Some coding style issue, this is prefered:

        function(a, b, c, d,
                 e, f, g);

rather than:

        function(a, b, c, d,
                        e, f, g);

I like that we saved 300 LOC with this. I have also applied one patch
to fix the wrong interpretation of the flags with IPv6.

>   nft: Add a function to reset the counters of an existing rule

Also applied this one.

Thanks.