Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [iptables PATCH 1/2] nft: Merge nft_*_rule_find() functions
@ 2020-05-11 13:38 Phil Sutter
  2020-05-11 13:38 ` [iptables PATCH 2/2] nft: Drop save_counters callback from family_ops Phil Sutter
  2020-05-18 15:15 ` [iptables PATCH 1/2] nft: Merge nft_*_rule_find() functions Pablo Neira Ayuso
  0 siblings, 2 replies; 4+ messages in thread
From: Phil Sutter @ 2020-05-11 13:38 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

Both ebtables and arptables are fine with using nft_ipv46_rule_find()
instead of their own implementations. Take the chance and move the
former into nft.c as a static helper since it is used in a single place,
only. Then get rid of the callback from family_ops.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-arp.c    | 28 ----------------------------
 iptables/nft-bridge.c | 38 --------------------------------------
 iptables/nft-ipv4.c   |  1 -
 iptables/nft-ipv6.c   |  1 -
 iptables/nft-shared.c | 39 ---------------------------------------
 iptables/nft-shared.h |  4 ----
 iptables/nft.c        | 41 ++++++++++++++++++++++++++++++++++++++++-
 7 files changed, 40 insertions(+), 112 deletions(-)

diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
index 9a831efd07a28..23ab73cba649e 100644
--- a/iptables/nft-arp.c
+++ b/iptables/nft-arp.c
@@ -635,33 +635,6 @@ static bool nft_arp_is_same(const void *data_a,
 				  (unsigned char *)b->arp.outiface_mask);
 }
 
-static bool nft_arp_rule_find(struct nft_handle *h, struct nftnl_rule *r,
-			      struct nftnl_rule *rule)
-{
-	struct iptables_command_state _cs = {}, *cs = &_cs;
-	struct iptables_command_state this = {};
-	bool ret = false;
-
-	/* Delete by matching rule case */
-	nft_rule_to_iptables_command_state(h, r, &this);
-	nft_rule_to_iptables_command_state(h, rule, cs);
-
-	if (!nft_arp_is_same(&cs->arp, &this.arp))
-		goto out;
-
-	if (!compare_targets(cs->target, this.target))
-		goto out;
-
-	if (this.jumpto && strcmp(cs->jumpto, this.jumpto) != 0)
-		goto out;
-
-	ret = true;
-out:
-	h->ops->clear_cs(&this);
-	h->ops->clear_cs(cs);
-	return ret;
-}
-
 static void nft_arp_save_chain(const struct nftnl_chain *c, const char *policy)
 {
 	const char *chain = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
@@ -684,6 +657,5 @@ struct nft_family_ops nft_family_ops_arp = {
 	.post_parse		= NULL,
 	.rule_to_cs		= nft_rule_to_iptables_command_state,
 	.clear_cs		= nft_clear_iptables_command_state,
-	.rule_find		= nft_arp_rule_find,
 	.parse_target		= nft_ipv46_parse_target,
 };
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 39a2f704000c7..18f5e78f1b3a2 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -756,43 +756,6 @@ static bool nft_bridge_is_same(const void *data_a, const void *data_b)
 	return strcmp(a->in, b->in) == 0 && strcmp(a->out, b->out) == 0;
 }
 
-static bool nft_bridge_rule_find(struct nft_handle *h, struct nftnl_rule *r,
-				 struct nftnl_rule *rule)
-{
-	struct iptables_command_state _cs = {}, *cs = &_cs;
-	struct iptables_command_state this = {};
-	bool ret = false;
-
-	nft_rule_to_ebtables_command_state(h, r, &this);
-	nft_rule_to_ebtables_command_state(h, rule, cs);
-
-	DEBUGP("comparing with... ");
-
-	if (!nft_bridge_is_same(cs, &this))
-		goto out;
-
-	if (!compare_matches(cs->matches, this.matches)) {
-		DEBUGP("Different matches\n");
-		goto out;
-	}
-
-	if (!compare_targets(cs->target, this.target)) {
-		DEBUGP("Different target\n");
-		goto out;
-	}
-
-	if (cs->jumpto != NULL && strcmp(cs->jumpto, this.jumpto) != 0) {
-		DEBUGP("Different verdict\n");
-		goto out;
-	}
-
-	ret = true;
-out:
-	h->ops->clear_cs(&this);
-	h->ops->clear_cs(cs);
-	return ret;
-}
-
 static int xlate_ebmatches(const struct iptables_command_state *cs, struct xt_xlate *xl)
 {
 	int ret = 1, numeric = cs->options & OPT_NUMERIC;
@@ -974,6 +937,5 @@ struct nft_family_ops nft_family_ops_bridge = {
 	.post_parse		= NULL,
 	.rule_to_cs		= nft_rule_to_ebtables_command_state,
 	.clear_cs		= ebt_cs_clean,
-	.rule_find		= nft_bridge_rule_find,
 	.xlate			= nft_bridge_xlate,
 };
diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index 69691fe28cf80..ba789da0c5973 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -457,6 +457,5 @@ struct nft_family_ops nft_family_ops_ipv4 = {
 	.parse_target		= nft_ipv46_parse_target,
 	.rule_to_cs		= nft_rule_to_iptables_command_state,
 	.clear_cs		= nft_clear_iptables_command_state,
-	.rule_find		= nft_ipv46_rule_find,
 	.xlate			= nft_ipv4_xlate,
 };
diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
index 76f2613d95c6a..84bcf1c53f48c 100644
--- a/iptables/nft-ipv6.c
+++ b/iptables/nft-ipv6.c
@@ -409,6 +409,5 @@ struct nft_family_ops nft_family_ops_ipv6 = {
 	.parse_target		= nft_ipv46_parse_target,
 	.rule_to_cs		= nft_rule_to_iptables_command_state,
 	.clear_cs		= nft_clear_iptables_command_state,
-	.rule_find		= nft_ipv46_rule_find,
 	.xlate			= nft_ipv6_xlate,
 };
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index bfc7bc2203239..53cd4cae9ef7c 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -989,45 +989,6 @@ void nft_ipv46_parse_target(struct xtables_target *t, void *data)
 	cs->target = t;
 }
 
-bool nft_ipv46_rule_find(struct nft_handle *h, struct nftnl_rule *r,
-			 struct nftnl_rule *rule)
-{
-	struct iptables_command_state _cs = {}, this = {}, *cs = &_cs;
-	bool ret = false;
-
-	nft_rule_to_iptables_command_state(h, r, &this);
-	nft_rule_to_iptables_command_state(h, rule, cs);
-
-	DEBUGP("comparing with... ");
-#ifdef DEBUG_DEL
-	nft_rule_print_save(h, r, NFT_RULE_APPEND, 0);
-#endif
-	if (!h->ops->is_same(cs, &this))
-		goto out;
-
-	if (!compare_matches(cs->matches, this.matches)) {
-		DEBUGP("Different matches\n");
-		goto out;
-	}
-
-	if (!compare_targets(cs->target, this.target)) {
-		DEBUGP("Different target\n");
-		goto out;
-	}
-
-	if ((!cs->target || !this.target) &&
-	    strcmp(cs->jumpto, this.jumpto) != 0) {
-		DEBUGP("Different verdict\n");
-		goto out;
-	}
-
-	ret = true;
-out:
-	h->ops->clear_cs(&this);
-	h->ops->clear_cs(cs);
-	return ret;
-}
-
 void nft_check_xt_legacy(int family, bool is_ipt_save)
 {
 	static const char tables6[] = "/proc/net/ip6_tables_names";
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index 89e9d0b9be335..cb60e685872dd 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -109,8 +109,6 @@ struct nft_family_ops {
 	void (*rule_to_cs)(struct nft_handle *h, const struct nftnl_rule *r,
 			   struct iptables_command_state *cs);
 	void (*clear_cs)(struct iptables_command_state *cs);
-	bool (*rule_find)(struct nft_handle *h, struct nftnl_rule *r,
-			  struct nftnl_rule *rule);
 	int (*xlate)(const void *data, struct xt_xlate *xl);
 };
 
@@ -171,8 +169,6 @@ void save_matches_and_target(const struct iptables_command_state *cs,
 struct nft_family_ops *nft_family_ops_lookup(int family);
 
 void nft_ipv46_parse_target(struct xtables_target *t, void *data);
-bool nft_ipv46_rule_find(struct nft_handle *h, struct nftnl_rule *r,
-			 struct nftnl_rule *rule);
 
 bool compare_matches(struct xtables_rule_match *mt1, struct xtables_rule_match *mt2);
 bool compare_targets(struct xtables_target *tg1, struct xtables_target *tg2);
diff --git a/iptables/nft.c b/iptables/nft.c
index 3c0daa8d42529..e65eb91c1c504 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -2120,6 +2120,45 @@ static int __nft_rule_del(struct nft_handle *h, struct nftnl_rule *r)
 	return 1;
 }
 
+static bool nft_rule_cmp(struct nft_handle *h, struct nftnl_rule *r,
+			 struct nftnl_rule *rule)
+{
+	struct iptables_command_state _cs = {}, this = {}, *cs = &_cs;
+	bool ret = false;
+
+	h->ops->rule_to_cs(h, r, &this);
+	h->ops->rule_to_cs(h, rule, cs);
+
+	DEBUGP("comparing with... ");
+#ifdef DEBUG_DEL
+	nft_rule_print_save(h, r, NFT_RULE_APPEND, 0);
+#endif
+	if (!h->ops->is_same(cs, &this))
+		goto out;
+
+	if (!compare_matches(cs->matches, this.matches)) {
+		DEBUGP("Different matches\n");
+		goto out;
+	}
+
+	if (!compare_targets(cs->target, this.target)) {
+		DEBUGP("Different target\n");
+		goto out;
+	}
+
+	if ((!cs->target || !this.target) &&
+	    strcmp(cs->jumpto, this.jumpto) != 0) {
+		DEBUGP("Different verdict\n");
+		goto out;
+	}
+
+	ret = true;
+out:
+	h->ops->clear_cs(&this);
+	h->ops->clear_cs(cs);
+	return ret;
+}
+
 static struct nftnl_rule *
 nft_rule_find(struct nft_handle *h, struct nftnl_chain *c,
 	      struct nftnl_rule *rule, int rulenum)
@@ -2138,7 +2177,7 @@ nft_rule_find(struct nft_handle *h, struct nftnl_chain *c,
 
 	r = nftnl_rule_iter_next(iter);
 	while (r != NULL) {
-		found = h->ops->rule_find(h, r, rule);
+		found = nft_rule_cmp(h, r, rule);
 		if (found)
 			break;
 		r = nftnl_rule_iter_next(iter);
-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [iptables PATCH 2/2] nft: Drop save_counters callback from family_ops
  2020-05-11 13:38 [iptables PATCH 1/2] nft: Merge nft_*_rule_find() functions Phil Sutter
@ 2020-05-11 13:38 ` Phil Sutter
  2020-05-18 15:15   ` Pablo Neira Ayuso
  2020-05-18 15:15 ` [iptables PATCH 1/2] nft: Merge nft_*_rule_find() functions Pablo Neira Ayuso
  1 sibling, 1 reply; 4+ messages in thread
From: Phil Sutter @ 2020-05-11 13:38 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

All families use the same callback function, just fold it into the sole
place it's called.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-arp.c    | 1 -
 iptables/nft-bridge.c | 1 -
 iptables/nft-ipv4.c   | 1 -
 iptables/nft-ipv6.c   | 1 -
 iptables/nft-shared.c | 8 --------
 iptables/nft-shared.h | 2 --
 iptables/nft.c        | 5 +++--
 7 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
index 23ab73cba649e..67f4529d93652 100644
--- a/iptables/nft-arp.c
+++ b/iptables/nft-arp.c
@@ -652,7 +652,6 @@ struct nft_family_ops nft_family_ops_arp = {
 	.print_header		= nft_arp_print_header,
 	.print_rule		= nft_arp_print_rule,
 	.save_rule		= nft_arp_save_rule,
-	.save_counters		= save_counters,
 	.save_chain		= nft_arp_save_chain,
 	.post_parse		= NULL,
 	.rule_to_cs		= nft_rule_to_iptables_command_state,
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 18f5e78f1b3a2..dbf11eb5e1fa8 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -932,7 +932,6 @@ 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		= save_counters,
 	.save_chain		= nft_bridge_save_chain,
 	.post_parse		= NULL,
 	.rule_to_cs		= nft_rule_to_ebtables_command_state,
diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index ba789da0c5973..afdecf9711e64 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -450,7 +450,6 @@ struct nft_family_ops nft_family_ops_ipv4 = {
 	.print_header		= print_header,
 	.print_rule		= nft_ipv4_print_rule,
 	.save_rule		= nft_ipv4_save_rule,
-	.save_counters		= save_counters,
 	.save_chain		= nft_ipv46_save_chain,
 	.proto_parse		= nft_ipv4_proto_parse,
 	.post_parse		= nft_ipv4_post_parse,
diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
index 84bcf1c53f48c..4008b7eab4f2a 100644
--- a/iptables/nft-ipv6.c
+++ b/iptables/nft-ipv6.c
@@ -402,7 +402,6 @@ struct nft_family_ops nft_family_ops_ipv6 = {
 	.print_header		= print_header,
 	.print_rule		= nft_ipv6_print_rule,
 	.save_rule		= nft_ipv6_save_rule,
-	.save_counters		= save_counters,
 	.save_chain		= nft_ipv46_save_chain,
 	.proto_parse		= nft_ipv6_proto_parse,
 	.post_parse		= nft_ipv6_post_parse,
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 53cd4cae9ef7c..c5a8f3fcc051d 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -831,14 +831,6 @@ void save_rule_details(const struct iptables_command_state *cs,
 	}
 }
 
-void save_counters(const void *data)
-{
-	const struct iptables_command_state *cs = data;
-
-	printf("[%llu:%llu] ", (unsigned long long)cs->counters.pcnt,
-			       (unsigned long long)cs->counters.bcnt);
-}
-
 void nft_ipv46_save_chain(const struct nftnl_chain *c, const char *policy)
 {
 	const char *chain = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index cb60e685872dd..94437ffe7990c 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -98,7 +98,6 @@ struct nft_family_ops {
 	void (*print_rule)(struct nft_handle *h, struct nftnl_rule *r,
 			   unsigned int num, unsigned int format);
 	void (*save_rule)(const void *data, unsigned int format);
-	void (*save_counters)(const void *data);
 	void (*save_chain)(const struct nftnl_chain *c, const char *policy);
 	void (*proto_parse)(struct iptables_command_state *cs,
 			    struct xtables_args *args);
@@ -160,7 +159,6 @@ void save_rule_details(const struct iptables_command_state *cs,
 		       unsigned const char *iniface_mask,
 		       const char *outiface,
 		       unsigned const char *outiface_mask);
-void save_counters(const void *data);
 void nft_ipv46_save_chain(const struct nftnl_chain *c, const char *policy);
 void save_matches_and_target(const struct iptables_command_state *cs,
 			     bool goto_flag, const void *fw,
diff --git a/iptables/nft.c b/iptables/nft.c
index e65eb91c1c504..0c5a74fc232c6 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1453,8 +1453,9 @@ nft_rule_print_save(struct nft_handle *h, const struct nftnl_rule *r,
 
 	ops->rule_to_cs(h, r, &cs);
 
-	if (!(format & (FMT_NOCOUNTS | FMT_C_COUNTS)) && ops->save_counters)
-		ops->save_counters(&cs);
+	if (!(format & (FMT_NOCOUNTS | FMT_C_COUNTS)))
+		printf("[%llu:%llu] ", (unsigned long long)cs.counters.pcnt,
+				       (unsigned long long)cs.counters.bcnt);
 
 	/* print chain name */
 	switch(type) {
-- 
2.25.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [iptables PATCH 1/2] nft: Merge nft_*_rule_find() functions
  2020-05-11 13:38 [iptables PATCH 1/2] nft: Merge nft_*_rule_find() functions Phil Sutter
  2020-05-11 13:38 ` [iptables PATCH 2/2] nft: Drop save_counters callback from family_ops Phil Sutter
@ 2020-05-18 15:15 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2020-05-18 15:15 UTC (permalink / raw)
  To: Phil Sutter; +Cc: netfilter-devel

On Mon, May 11, 2020 at 03:38:16PM +0200, Phil Sutter wrote:
> Both ebtables and arptables are fine with using nft_ipv46_rule_find()
> instead of their own implementations. Take the chance and move the
> former into nft.c as a static helper since it is used in a single place,
> only. Then get rid of the callback from family_ops.

LGTM.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [iptables PATCH 2/2] nft: Drop save_counters callback from family_ops
  2020-05-11 13:38 ` [iptables PATCH 2/2] nft: Drop save_counters callback from family_ops Phil Sutter
@ 2020-05-18 15:15   ` Pablo Neira Ayuso
  0 siblings, 0 replies; 4+ messages in thread
From: Pablo Neira Ayuso @ 2020-05-18 15:15 UTC (permalink / raw)
  To: Phil Sutter; +Cc: netfilter-devel

On Mon, May 11, 2020 at 03:38:17PM +0200, Phil Sutter wrote:
> All families use the same callback function, just fold it into the sole
> place it's called.

LGTM.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-11 13:38 [iptables PATCH 1/2] nft: Merge nft_*_rule_find() functions Phil Sutter
2020-05-11 13:38 ` [iptables PATCH 2/2] nft: Drop save_counters callback from family_ops Phil Sutter
2020-05-18 15:15   ` Pablo Neira Ayuso
2020-05-18 15:15 ` [iptables PATCH 1/2] nft: Merge nft_*_rule_find() functions Pablo Neira Ayuso

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org
	public-inbox-index netfilter-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git