[iptables PATCH 00/14] Implement among match support

[iptables PATCH 00/14] Implement among match support

[Phil Sutter]

This series ultimately adds among match support to ebtables-nft. This
implementation merely shares the user interface with legacy one,
internally the code is distinct: libebt_among.c does not make use of the
wormhash data structure but a much simpler one for "temporary" storage
of data until being converted into an anonymous set and associated
lookup expression.

Patches 1 to 4 are basically unrelated fallout with misc fixes or minor
code improvements.

Patches 5 and 6 add core infrastructure to cache existing sets and
create new ones.

Patches 7 to 10 deal with the need for nft_handle access in all places
converting extension to expression and vice-versa, split to improve
readability as far as possible.

The remaining patches add some more (glue-) code and finally the actual
among match implementation.

Phil Sutter (14):
  nft: Fix typo in nft_parse_limit() error message
  nft: Get rid of NFT_COMPAT_EXPR_MAX define
  nft: Keep nft_handle pointer in nft_xt_ctx
  nft: Eliminate pointless calls to nft_family_ops_lookup()
  nft: Fetch sets when updating rule cache
  nft: Support NFT_COMPAT_SET_ADD
  nft: family_ops: Pass nft_handle to 'add' callback
  nft: family_ops: Pass nft_handle to 'rule_find' callback
  nft: family_ops: Pass nft_handle to 'print_rule' callback
  nft: family_ops: Pass nft_handle to 'rule_to_cs' callback
  nft: Bore up nft_parse_payload()
  nft: Embed rule's table name in nft_xt_ctx
  nft: Support parsing lookup expression
  nft: bridge: Rudimental among extension support

 extensions/libebt_among.c  | 278 ++++++++++++++++++++++++++
 extensions/libebt_among.t  |  16 ++
 iptables/ebtables-nft.8    |  66 ++++---
 iptables/nft-arp.c         |  14 +-
 iptables/nft-bridge.c      | 260 ++++++++++++++++++++++++-
 iptables/nft-bridge.h      |  21 ++
 iptables/nft-ipv4.c        |  10 +-
 iptables/nft-ipv6.c        |  10 +-
 iptables/nft-shared.c      |  72 +++----
 iptables/nft-shared.h      |  26 ++-
 iptables/nft.c             | 390 ++++++++++++++++++++++++++++++++-----
 iptables/nft.h             |  13 +-
 iptables/xtables-eb.c      |   1 +
 iptables/xtables-monitor.c |  17 +-
 iptables/xtables-save.c    |   3 +
 15 files changed, 1039 insertions(+), 158 deletions(-)
 create mode 100644 extensions/libebt_among.c
 create mode 100644 extensions/libebt_among.t

-- 
2.22.0

[iptables PATCH 01/14] nft: Fix typo in nft_parse_limit() error message

[Phil Sutter]

Seems like a trivial copy'n'paste bug.

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

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 1c09277d85fb5..904bc845e6a41 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -566,7 +566,7 @@ static void nft_parse_limit(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 		matches = &ctx->cs->matches;
 		break;
 	default:
-		fprintf(stderr, "BUG: nft_parse_match() unknown family %d\n",
+		fprintf(stderr, "BUG: nft_parse_limit() unknown family %d\n",
 			ctx->family);
 		exit(EXIT_FAILURE);
 	}
-- 
2.22.0

Re: [iptables PATCH 01/14] nft: Fix typo in nft_parse_limit() error message

[Pablo Neira Ayuso]

On Wed, Aug 21, 2019 at 11:25:49AM +0200, Phil Sutter wrote:
> Seems like a trivial copy'n'paste bug.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

[iptables PATCH 02/14] nft: Get rid of NFT_COMPAT_EXPR_MAX define

[Phil Sutter]

Instead simply use ARRAY_SIZE() macro to not overstep supported_exprs
array.

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

diff --git a/iptables/nft.c b/iptables/nft.c
index ae3740be6bed5..458dededaca29 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -3332,9 +3332,7 @@ uint32_t nft_invflags2cmp(uint32_t invflags, uint32_t flag)
 	return NFT_CMP_EQ;
 }
 
-#define NFT_COMPAT_EXPR_MAX     8
-
-static const char *supported_exprs[NFT_COMPAT_EXPR_MAX] = {
+static const char *supported_exprs[] = {
 	"match",
 	"target",
 	"payload",
@@ -3351,7 +3349,7 @@ static int nft_is_expr_compatible(struct nftnl_expr *expr, void *data)
 	const char *name = nftnl_expr_get_str(expr, NFTNL_EXPR_NAME);
 	int i;
 
-	for (i = 0; i < NFT_COMPAT_EXPR_MAX; i++) {
+	for (i = 0; i < ARRAY_SIZE(supported_exprs); i++) {
 		if (strcmp(supported_exprs[i], name) == 0)
 			return 0;
 	}
-- 
2.22.0

Re: [iptables PATCH 02/14] nft: Get rid of NFT_COMPAT_EXPR_MAX define

[Pablo Neira Ayuso]

On Wed, Aug 21, 2019 at 11:25:50AM +0200, Phil Sutter wrote:
> Instead simply use ARRAY_SIZE() macro to not overstep supported_exprs
> array.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

[iptables PATCH 03/14] nft: Keep nft_handle pointer in nft_xt_ctx

[Phil Sutter]

Instead of carrying the family value, carry the handle (which contains
the family value) and relieve expression parsers from having to call
nft_family_ops_lookup().

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-shared.c | 40 ++++++++++++++--------------------------
 iptables/nft-shared.h |  2 +-
 2 files changed, 15 insertions(+), 27 deletions(-)

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 904bc845e6a41..3ab21932f8fe6 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -310,7 +310,6 @@ static void nft_parse_target(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 	struct xtables_target *target;
 	struct xt_entry_target *t;
 	size_t size;
-	struct nft_family_ops *ops;
 	void *data = ctx->cs;
 
 	target = xtables_find_target(targname, XTF_TRY_LOAD);
@@ -327,8 +326,7 @@ static void nft_parse_target(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 
 	target->t = t;
 
-	ops = nft_family_ops_lookup(ctx->family);
-	ops->parse_target(target, data);
+	ctx->h->ops->parse_target(target, data);
 }
 
 static void nft_parse_match(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
@@ -339,9 +337,8 @@ static void nft_parse_match(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 	struct xtables_match *match;
 	struct xtables_rule_match **matches;
 	struct xt_entry_match *m;
-	struct nft_family_ops *ops;
 
-	switch (ctx->family) {
+	switch (ctx->h->family) {
 	case NFPROTO_IPV4:
 	case NFPROTO_IPV6:
 	case NFPROTO_BRIDGE:
@@ -349,7 +346,7 @@ static void nft_parse_match(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 		break;
 	default:
 		fprintf(stderr, "BUG: nft_parse_match() unknown family %d\n",
-			ctx->family);
+			ctx->h->family);
 		exit(EXIT_FAILURE);
 	}
 
@@ -365,9 +362,8 @@ static void nft_parse_match(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 
 	match->m = m;
 
-	ops = nft_family_ops_lookup(ctx->family);
-	if (ops->parse_match != NULL)
-		ops->parse_match(match, ctx->cs);
+	if (ctx->h->ops->parse_match != NULL)
+		ctx->h->ops->parse_match(match, ctx->cs);
 }
 
 void print_proto(uint16_t proto, int invert)
@@ -400,7 +396,6 @@ void get_cmp_data(struct nftnl_expr *e, void *data, size_t dlen, bool *inv)
 
 static void nft_meta_set_to_target(struct nft_xt_ctx *ctx)
 {
-	const struct nft_family_ops *ops;
 	struct xtables_target *target;
 	struct xt_entry_target *t;
 	unsigned int size;
@@ -429,8 +424,7 @@ static void nft_meta_set_to_target(struct nft_xt_ctx *ctx)
 
 	target->t = t;
 
-	ops = nft_family_ops_lookup(ctx->family);
-	ops->parse_target(target, ctx->cs);
+	ctx->h->ops->parse_target(target, ctx->cs);
 }
 
 static void nft_parse_meta(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
@@ -474,7 +468,6 @@ static void nft_parse_bitwise(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 
 static void nft_parse_cmp(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 {
-	struct nft_family_ops *ops = nft_family_ops_lookup(ctx->family);
 	void *data = ctx->cs;
 	uint32_t reg;
 
@@ -483,12 +476,12 @@ static void nft_parse_cmp(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 		return;
 
 	if (ctx->flags & NFT_XT_CTX_META) {
-		ops->parse_meta(ctx, e, data);
+		ctx->h->ops->parse_meta(ctx, e, data);
 		ctx->flags &= ~NFT_XT_CTX_META;
 	}
 	/* bitwise context is interpreted from payload */
 	if (ctx->flags & NFT_XT_CTX_PAYLOAD) {
-		ops->parse_payload(ctx, e, data);
+		ctx->h->ops->parse_payload(ctx, e, data);
 		ctx->flags &= ~NFT_XT_CTX_PAYLOAD;
 	}
 }
@@ -502,7 +495,6 @@ static void nft_parse_counter(struct nftnl_expr *e, struct xt_counters *counters
 static void nft_parse_immediate(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 {
 	const char *chain = nftnl_expr_get_str(e, NFTNL_EXPR_IMM_CHAIN);
-	struct nft_family_ops *ops;
 	const char *jumpto = NULL;
 	bool nft_goto = false;
 	void *data = ctx->cs;
@@ -544,8 +536,7 @@ static void nft_parse_immediate(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 		break;
 	}
 
-	ops = nft_family_ops_lookup(ctx->family);
-	ops->parse_immediate(jumpto, nft_goto, data);
+	ctx->h->ops->parse_immediate(jumpto, nft_goto, data);
 }
 
 static void nft_parse_limit(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
@@ -555,11 +546,10 @@ static void nft_parse_limit(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 	__u64 rate = nftnl_expr_get_u64(e, NFTNL_EXPR_LIMIT_RATE);
 	struct xtables_rule_match **matches;
 	struct xtables_match *match;
-	struct nft_family_ops *ops;
 	struct xt_rateinfo *rinfo;
 	size_t size;
 
-	switch (ctx->family) {
+	switch (ctx->h->family) {
 	case NFPROTO_IPV4:
 	case NFPROTO_IPV6:
 	case NFPROTO_BRIDGE:
@@ -567,7 +557,7 @@ static void nft_parse_limit(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 		break;
 	default:
 		fprintf(stderr, "BUG: nft_parse_limit() unknown family %d\n",
-			ctx->family);
+			ctx->h->family);
 		exit(EXIT_FAILURE);
 	}
 
@@ -586,9 +576,8 @@ static void nft_parse_limit(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 	rinfo->avg = XT_LIMIT_SCALE * unit / rate;
 	rinfo->burst = burst;
 
-	ops = nft_family_ops_lookup(ctx->family);
-	if (ops->parse_match != NULL)
-		ops->parse_match(match, ctx->cs);
+	if (ctx->h->ops->parse_match != NULL)
+		ctx->h->ops->parse_match(match, ctx->cs);
 }
 
 void nft_rule_to_iptables_command_state(const struct nftnl_rule *r,
@@ -596,10 +585,9 @@ void nft_rule_to_iptables_command_state(const struct nftnl_rule *r,
 {
 	struct nftnl_expr_iter *iter;
 	struct nftnl_expr *expr;
-	int family = nftnl_rule_get_u32(r, NFTNL_RULE_FAMILY);
 	struct nft_xt_ctx ctx = {
 		.cs = cs,
-		.family = family,
+		.h = h,
 	};
 
 	iter = nftnl_expr_iter_create(r);
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index de889ead7b605..ad5e8cde239f4 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -47,7 +47,7 @@ enum {
 struct nft_xt_ctx {
 	struct iptables_command_state *cs;
 	struct nftnl_expr_iter *iter;
-	int family;
+	struct nft_handle *h;
 	uint32_t flags;
 
 	uint32_t reg;
-- 
2.22.0

Re: [iptables PATCH 03/14] nft: Keep nft_handle pointer in nft_xt_ctx

[Pablo Neira Ayuso]

On Wed, Aug 21, 2019 at 11:25:51AM +0200, Phil Sutter wrote:
> Instead of carrying the family value, carry the handle (which contains
> the family value) and relieve expression parsers from having to call
> nft_family_ops_lookup().
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

Re: [iptables PATCH 03/14] nft: Keep nft_handle pointer in nft_xt_ctx

[Phil Sutter]

Hi,

On Sat, Aug 24, 2019 at 06:41:07PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Aug 21, 2019 at 11:25:51AM +0200, Phil Sutter wrote:
> > Instead of carrying the family value, carry the handle (which contains
> > the family value) and relieve expression parsers from having to call
> > nft_family_ops_lookup().
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> 
> Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

I broke things when reordering, this patch depends on later changes.
Hence I'll push only the first two and get the rest sorted for upcoming
v2.

Sorry, Phil

[iptables PATCH 04/14] nft: Eliminate pointless calls to nft_family_ops_lookup()

[Phil Sutter]

If nft_handle is available, use its 'ops' field instead of performing a
new lookup. For the same reason, there is no need to pass ops pointer to
__nft_print_header().

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/iptables/nft.c b/iptables/nft.c
index 458dededaca29..28e63aad15878 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1663,11 +1663,8 @@ static const char *policy_name[NF_ACCEPT+1] = {
 int nft_chain_save(struct nft_handle *h, struct nftnl_chain_list *list)
 {
 	struct nftnl_chain_list_iter *iter;
-	struct nft_family_ops *ops;
 	struct nftnl_chain *c;
 
-	ops = nft_family_ops_lookup(h->family);
-
 	iter = nftnl_chain_list_iter_create(list);
 	if (iter == NULL)
 		return 0;
@@ -1693,8 +1690,8 @@ int nft_chain_save(struct nft_handle *h, struct nftnl_chain_list *list)
 			}
 		}
 
-		if (ops->save_chain)
-			ops->save_chain(c, policy);
+		if (h->ops->save_chain)
+			h->ops->save_chain(c, policy);
 
 		c = nftnl_chain_list_iter_next(iter);
 	}
@@ -2484,7 +2481,6 @@ static int nft_rule_count(struct nft_handle *h, struct nftnl_chain *c)
 }
 
 static void __nft_print_header(struct nft_handle *h,
-			       const struct nft_family_ops *ops,
 			       struct nftnl_chain *c, unsigned int format)
 {
 	const char *chain_name = nftnl_chain_get_str(c, NFTNL_CHAIN_NAME);
@@ -2500,14 +2496,13 @@ static void __nft_print_header(struct nft_handle *h,
 	if (nftnl_chain_is_set(c, NFTNL_CHAIN_POLICY))
 		pname = policy_name[nftnl_chain_get_u32(c, NFTNL_CHAIN_POLICY)];
 
-	ops->print_header(format, chain_name, pname,
+	h->ops->print_header(format, chain_name, pname,
 			&ctrs, basechain, refs - entries, entries);
 }
 
 int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
 		  int rulenum, unsigned int format)
 {
-	const struct nft_family_ops *ops;
 	struct nftnl_chain_list *list;
 	struct nftnl_chain_list_iter *iter;
 	struct nftnl_chain *c;
@@ -2515,8 +2510,6 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
 
 	nft_xt_builtin_init(h, table);
 
-	ops = nft_family_ops_lookup(h->family);
-
 	if (!nft_is_table_compatible(h, table)) {
 		xtables_error(OTHER_PROBLEM, "table `%s' is incompatible, use 'nft' tool.\n", table);
 		return 0;
@@ -2532,11 +2525,11 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
 			return 0;
 
 		if (!rulenum) {
-			if (ops->print_table_header)
-				ops->print_table_header(table);
-			__nft_print_header(h, ops, c, format);
+			if (h->ops->print_table_header)
+				h->ops->print_table_header(table);
+			__nft_print_header(h, c, format);
 		}
-		__nft_rule_list(h, c, rulenum, format, ops->print_rule);
+		__nft_rule_list(h, c, rulenum, format, h->ops->print_rule);
 		return 1;
 	}
 
@@ -2544,16 +2537,16 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
 	if (iter == NULL)
 		return 0;
 
-	if (ops->print_table_header)
-		ops->print_table_header(table);
+	if (h->ops->print_table_header)
+		h->ops->print_table_header(table);
 
 	c = nftnl_chain_list_iter_next(iter);
 	while (c != NULL) {
 		if (found)
 			printf("\n");
 
-		__nft_print_header(h, ops, c, format);
-		__nft_rule_list(h, c, rulenum, format, ops->print_rule);
+		__nft_print_header(h, c, format);
+		__nft_rule_list(h, c, rulenum, format, h->ops->print_rule);
 
 		found = true;
 		c = nftnl_chain_list_iter_next(iter);
-- 
2.22.0

Re: [iptables PATCH 04/14] nft: Eliminate pointless calls to nft_family_ops_lookup()

[Pablo Neira Ayuso]

On Wed, Aug 21, 2019 at 11:25:52AM +0200, Phil Sutter wrote:
> If nft_handle is available, use its 'ops' field instead of performing a
> new lookup. For the same reason, there is no need to pass ops pointer to
> __nft_print_header().
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

[iptables PATCH 05/14] nft: Fetch sets when updating rule cache

[Phil Sutter]

In order to support anonymous sets, cache them along with their
elements.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft.c | 108 +++++++++++++++++++++++++++++++++++++++++++++++++
 iptables/nft.h |   7 ++++
 2 files changed, 115 insertions(+)

diff --git a/iptables/nft.c b/iptables/nft.c
index 28e63aad15878..633c33ddddb15 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -857,6 +857,14 @@ static int __flush_chain_cache(struct nftnl_chain *c, void *data)
 	return 0;
 }
 
+static int __flush_set_cache(struct nftnl_set *s, void *data)
+{
+	nftnl_set_list_del(s);
+	nftnl_set_free(s);
+
+	return 0;
+}
+
 static int flush_cache(struct nft_cache *c, const struct builtin_table *tables,
 		       const char *tablename)
 {
@@ -869,6 +877,8 @@ static int flush_cache(struct nft_cache *c, const struct builtin_table *tables,
 			return 0;
 		nftnl_chain_list_foreach(c->table[table->type].chains,
 					 __flush_chain_cache, NULL);
+		nftnl_set_list_foreach(c->table[table->type].sets,
+				       __flush_set_cache, NULL);
 		return 0;
 	}
 
@@ -881,6 +891,8 @@ static int flush_cache(struct nft_cache *c, const struct builtin_table *tables,
 
 		nftnl_chain_list_free(c->table[i].chains);
 		c->table[i].chains = NULL;
+		nftnl_set_list_free(c->table[i].sets);
+		c->table[i].sets = NULL;
 	}
 	nftnl_table_list_free(c->tables);
 	c->tables = NULL;
@@ -1414,6 +1426,86 @@ static int fetch_table_cache(struct nft_handle *h)
 	return 1;
 }
 
+static int nftnl_set_list_cb(const struct nlmsghdr *nlh, void *data)
+{
+	struct nft_handle *h = data;
+	const struct builtin_table *t;
+	struct nftnl_set *s;
+
+	s = nftnl_set_alloc();
+	if (s == NULL)
+		return MNL_CB_OK;
+
+	if (nftnl_set_nlmsg_parse(nlh, s) < 0)
+		goto out_free;
+
+	t = nft_table_builtin_find(h, nftnl_set_get_str(s, NFTNL_SET_TABLE));
+	if (!t)
+		goto out_free;
+
+	nftnl_set_list_add_tail(s, h->cache->table[t->type].sets);
+	return MNL_CB_OK;
+out_free:
+	nftnl_set_free(s);
+	return MNL_CB_OK;
+}
+
+static int set_elem_cb(const struct nlmsghdr *nlh, void *data)
+{
+	return nftnl_set_elems_nlmsg_parse(nlh, data) ? -1 : MNL_CB_OK;
+}
+
+static int set_list_fetch_elem_cb(struct nftnl_set *s, void *data)
+{
+        char buf[MNL_SOCKET_BUFFER_SIZE];
+	struct nft_handle *h = data;
+        struct nlmsghdr *nlh;
+
+	nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_GETSETELEM, h->family,
+				    NLM_F_DUMP, h->seq);
+        nftnl_set_elems_nlmsg_build_payload(nlh, s);
+
+	return mnl_talk(h, nlh, set_elem_cb, s);
+}
+
+static int fetch_set_cache(struct nft_handle *h)
+{
+	struct nlmsghdr *nlh;
+	char buf[16536];
+	int i, ret;
+
+	for (i = 0; i < NFT_TABLE_MAX; i++) {
+		enum nft_table_type type = h->tables[i].type;
+
+		if (!h->tables[i].name)
+			continue;
+
+		h->cache->table[type].sets = nftnl_set_list_alloc();
+		if (!h->cache->table[type].sets)
+			return -1;
+	}
+
+	nlh = nftnl_set_nlmsg_build_hdr(buf, NFT_MSG_GETSET, h->family,
+					NLM_F_DUMP, h->seq);
+
+	ret = mnl_talk(h, nlh, nftnl_set_list_cb, h);
+	if (ret < 0 && errno == EINTR) {
+		assert(nft_restart(h) >= 0);
+		return ret;
+	}
+
+	for (i = 0; i < NFT_TABLE_MAX; i++) {
+		enum nft_table_type type = h->tables[i].type;
+
+		if (!h->tables[i].name)
+			continue;
+
+		nftnl_set_list_foreach(h->cache->table[type].sets,
+				       set_list_fetch_elem_cb, h);
+	}
+	return ret;
+}
+
 static int fetch_chain_cache(struct nft_handle *h)
 {
 	char buf[16536];
@@ -1579,6 +1671,8 @@ static int fetch_rule_cache(struct nft_handle *h)
 {
 	int i;
 
+	fetch_set_cache(h);
+
 	for (i = 0; i < NFT_TABLE_MAX; i++) {
 		enum nft_table_type type = h->tables[i].type;
 
@@ -1655,6 +1749,20 @@ struct nftnl_chain_list *nft_chain_list_get(struct nft_handle *h,
 	return h->cache->table[t->type].chains;
 }
 
+struct nftnl_set_list *nft_set_list_get(struct nft_handle *h,
+					const char *table)
+{
+	const struct builtin_table *t;
+
+	t = nft_table_builtin_find(h, table);
+	if (!t)
+		return NULL;
+
+	nft_build_cache(h);
+
+	return h->cache->table[t->type].sets;
+}
+
 static const char *policy_name[NF_ACCEPT+1] = {
 	[NF_DROP] = "DROP",
 	[NF_ACCEPT] = "ACCEPT",
diff --git a/iptables/nft.h b/iptables/nft.h
index 5e5e765b0d043..2f929a15f98df 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -31,6 +31,7 @@ struct nft_cache {
 	struct nftnl_table_list		*tables;
 	struct {
 		struct nftnl_chain_list *chains;
+		struct nftnl_set_list	*sets;
 		bool			initialized;
 	} table[NFT_TABLE_MAX];
 };
@@ -88,6 +89,12 @@ int nft_table_flush(struct nft_handle *h, const char *table);
 void nft_table_new(struct nft_handle *h, const char *table);
 const struct builtin_table *nft_table_builtin_find(struct nft_handle *h, const char *table);
 
+/*
+ * Operations with sets.
+ */
+struct nftnl_set_list *nft_set_list_get(struct nft_handle *h,
+					const char *table);
+
 /*
  * Operations with chains.
  */
-- 
2.22.0

Re: [iptables PATCH 05/14] nft: Fetch sets when updating rule cache

[Pablo Neira Ayuso]

On Wed, Aug 21, 2019 at 11:25:53AM +0200, Phil Sutter wrote:
> diff --git a/iptables/nft.c b/iptables/nft.c
> index 28e63aad15878..633c33ddddb15 100644
> --- a/iptables/nft.c
> +++ b/iptables/nft.c
> +static int set_list_fetch_elem_cb(struct nftnl_set *s, void *data)
> +{
> +        char buf[MNL_SOCKET_BUFFER_SIZE];
> +	struct nft_handle *h = data;
> +        struct nlmsghdr *nlh;
> +
> +	nlh = nftnl_nlmsg_build_hdr(buf, NFT_MSG_GETSETELEM, h->family,
> +				    NLM_F_DUMP, h->seq);
> +        nftnl_set_elems_nlmsg_build_payload(nlh, s);
> +
> +	return mnl_talk(h, nlh, set_elem_cb, s);

Weird indentation here.

[iptables PATCH 06/14] nft: Support NFT_COMPAT_SET_ADD

[Phil Sutter]

Implement the required infrastructure to create sets as part of a batch
job commit.

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

diff --git a/iptables/nft.c b/iptables/nft.c
index 633c33ddddb15..ef43d4e5445b5 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -306,6 +306,7 @@ enum obj_update_type {
 	NFT_COMPAT_RULE_REPLACE,
 	NFT_COMPAT_RULE_DELETE,
 	NFT_COMPAT_RULE_FLUSH,
+	NFT_COMPAT_SET_ADD,
 };
 
 enum obj_action {
@@ -323,6 +324,7 @@ struct obj_update {
 		struct nftnl_table	*table;
 		struct nftnl_chain	*chain;
 		struct nftnl_rule	*rule;
+		struct nftnl_set	*set;
 		void			*ptr;
 	};
 	struct {
@@ -350,6 +352,7 @@ static int mnl_append_error(const struct nft_handle *h,
 		[NFT_COMPAT_RULE_REPLACE] = "RULE_REPLACE",
 		[NFT_COMPAT_RULE_DELETE] = "RULE_DELETE",
 		[NFT_COMPAT_RULE_FLUSH] = "RULE_FLUSH",
+		[NFT_COMPAT_SET_ADD] = "SET_ADD",
 	};
 	char errmsg[256];
 	char tcr[128];
@@ -390,6 +393,10 @@ static int mnl_append_error(const struct nft_handle *h,
 		}
 #endif
 		break;
+	case NFT_COMPAT_SET_ADD:
+		snprintf(tcr, sizeof(tcr), "set %s",
+			 nftnl_set_get_str(o->set, NFTNL_SET_NAME));
+		break;
 	}
 
 	return snprintf(buf, len, "%s: %s", errmsg, tcr);
@@ -419,6 +426,13 @@ batch_table_add(struct nft_handle *h, enum obj_update_type type,
 	return batch_add(h, type, t);
 }
 
+static struct obj_update *
+batch_set_add(struct nft_handle *h, enum obj_update_type type,
+	      struct nftnl_set *s)
+{
+	return batch_add(h, type, s);
+}
+
 static int batch_chain_add(struct nft_handle *h, enum obj_update_type type,
 			   struct nftnl_chain *c)
 {
@@ -2803,6 +2817,39 @@ static void nft_compat_table_batch_add(struct nft_handle *h, uint16_t type,
 	nftnl_table_nlmsg_build_payload(nlh, table);
 }
 
+static void nft_compat_set_batch_add(struct nft_handle *h, uint16_t type,
+				     uint16_t flags, uint32_t seq,
+				     struct nftnl_set *set)
+{
+	struct nlmsghdr *nlh;
+
+	nlh = nftnl_nlmsg_build_hdr(nftnl_batch_buffer(h->batch),
+					type, h->family, flags, seq);
+	nftnl_set_nlmsg_build_payload(nlh, set);
+}
+
+static void nft_compat_setelem_batch_add(struct nft_handle *h, uint16_t type,
+					 uint16_t flags, uint32_t *seq,
+					 struct nftnl_set *set)
+{
+	struct nftnl_set_elems_iter *iter;
+	struct nlmsghdr *nlh;
+
+	iter = nftnl_set_elems_iter_create(set);
+	if (!iter)
+		return;
+
+	while (nftnl_set_elems_iter_cur(iter)) {
+		(*seq)++;
+		mnl_nft_batch_continue(h->batch);
+		nlh = nftnl_nlmsg_build_hdr(nftnl_batch_buffer(h->batch),
+					    type, h->family, flags, *seq);
+		if (nftnl_set_elems_nlmsg_build_payload_iter(nlh, iter) <= 0)
+			break;
+	}
+	nftnl_set_elems_iter_destroy(iter);
+}
+
 static void nft_compat_chain_batch_add(struct nft_handle *h, uint16_t type,
 				       uint16_t flags, uint32_t seq,
 				       struct nftnl_chain *chain)
@@ -2852,6 +2899,9 @@ static void batch_obj_del(struct nft_handle *h, struct obj_update *o)
 	case NFT_COMPAT_RULE_FLUSH:
 		nftnl_rule_free(o->rule);
 		break;
+	case NFT_COMPAT_SET_ADD:
+		nftnl_set_free(o->set);
+		break;
 	}
 	h->obj_list_num--;
 	list_del(&o->head);
@@ -2918,6 +2968,7 @@ static void nft_refresh_transaction(struct nft_handle *h)
 		case NFT_COMPAT_RULE_REPLACE:
 		case NFT_COMPAT_RULE_DELETE:
 		case NFT_COMPAT_RULE_FLUSH:
+		case NFT_COMPAT_SET_ADD:
 			break;
 		}
 	}
@@ -3008,6 +3059,13 @@ retry:
 			nft_compat_rule_batch_add(h, NFT_MSG_DELRULE, 0,
 						  n->seq, n->rule);
 			break;
+		case NFT_COMPAT_SET_ADD:
+			nft_compat_set_batch_add(h, NFT_MSG_NEWSET,
+						 NLM_F_CREATE, n->seq, n->set);
+			nft_compat_setelem_batch_add(h, NFT_MSG_NEWSETELEM,
+						     NLM_F_CREATE, &n->seq, n->set);
+			seq = n->seq;
+			break;
 		}
 
 		mnl_nft_batch_continue(h->batch);
-- 
2.22.0

[iptables PATCH 07/14] nft: family_ops: Pass nft_handle to 'add' callback

[Phil Sutter]

In order for add_match() to create anonymous sets when converting
xtables matches it needs access to nft handle. So pass it along from
callers of family ops' add callback.

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

diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
index 9805bbe0de87b..d9a5f861eecb1 100644
--- a/iptables/nft-arp.c
+++ b/iptables/nft-arp.c
@@ -149,7 +149,7 @@ static bool need_devaddr(struct arpt_devaddr_info *info)
 	return false;
 }
 
-static int nft_arp_add(struct nftnl_rule *r, void *data)
+static int nft_arp_add(struct nft_handle *h, struct nftnl_rule *r, void *data)
 {
 	struct iptables_command_state *cs = data;
 	struct arpt_entry *fw = &cs->arp;
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 2e4b309b86135..0fc21b3a3c0d6 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -126,7 +126,8 @@ static int _add_action(struct nftnl_rule *r, struct iptables_command_state *cs)
 	return add_action(r, cs, false);
 }
 
-static int nft_bridge_add(struct nftnl_rule *r, void *data)
+static int nft_bridge_add(struct nft_handle *h,
+			  struct nftnl_rule *r, void *data)
 {
 	struct iptables_command_state *cs = data;
 	struct ebt_match *iter;
@@ -182,7 +183,7 @@ static int nft_bridge_add(struct nftnl_rule *r, void *data)
 
 	for (iter = cs->match_list; iter; iter = iter->next) {
 		if (iter->ismatch) {
-			if (add_match(r, iter->u.match->m))
+			if (add_match(h, r, iter->u.match->m))
 				break;
 		} else {
 			if (add_target(r, iter->u.watcher->t))
diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index 4497eb9b9347c..57d1b3c6d2d0c 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -26,7 +26,7 @@
 #include "nft.h"
 #include "nft-shared.h"
 
-static int nft_ipv4_add(struct nftnl_rule *r, void *data)
+static int nft_ipv4_add(struct nft_handle *h, struct nftnl_rule *r, void *data)
 {
 	struct iptables_command_state *cs = data;
 	struct xtables_rule_match *matchp;
@@ -77,7 +77,7 @@ static int nft_ipv4_add(struct nftnl_rule *r, void *data)
 	add_compat(r, cs->fw.ip.proto, cs->fw.ip.invflags & XT_INV_PROTO);
 
 	for (matchp = cs->matches; matchp; matchp = matchp->next) {
-		ret = add_match(r, matchp->match->m);
+		ret = add_match(h, r, matchp->match->m);
 		if (ret < 0)
 			return ret;
 	}
diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
index cacb1c9e141f2..0e2c4a2946e25 100644
--- a/iptables/nft-ipv6.c
+++ b/iptables/nft-ipv6.c
@@ -25,7 +25,7 @@
 #include "nft.h"
 #include "nft-shared.h"
 
-static int nft_ipv6_add(struct nftnl_rule *r, void *data)
+static int nft_ipv6_add(struct nft_handle *h, struct nftnl_rule *r, void *data)
 {
 	struct iptables_command_state *cs = data;
 	struct xtables_rule_match *matchp;
@@ -66,7 +66,7 @@ static int nft_ipv6_add(struct nftnl_rule *r, void *data)
 	add_compat(r, cs->fw6.ipv6.proto, cs->fw6.ipv6.invflags & XT_INV_PROTO);
 
 	for (matchp = cs->matches; matchp; matchp = matchp->next) {
-		ret = add_match(r, matchp->match->m);
+		ret = add_match(h, r, matchp->match->m);
 		if (ret < 0)
 			return ret;
 	}
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index ad5e8cde239f4..0972c228bc65f 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -35,6 +35,7 @@
 #define FMT(tab,notab) ((format) & FMT_NOTABLE ? (notab) : (tab))
 
 struct xtables_args;
+struct nft_handle;
 struct xt_xlate;
 
 enum {
@@ -69,7 +70,7 @@ struct nft_xt_ctx {
 };
 
 struct nft_family_ops {
-	int (*add)(struct nftnl_rule *r, void *data);
+	int (*add)(struct nft_handle *h, struct nftnl_rule *r, void *data);
 	bool (*is_same)(const void *data_a,
 			const void *data_b);
 	void (*print_payload)(struct nftnl_expr *e,
@@ -163,7 +164,6 @@ void save_matches_and_target(const struct iptables_command_state *cs,
 
 struct nft_family_ops *nft_family_ops_lookup(int family);
 
-struct nft_handle;
 void nft_ipv46_parse_target(struct xtables_target *t, void *data);
 bool nft_ipv46_rule_find(struct nft_family_ops *ops, struct nftnl_rule *r,
 			 void *data);
diff --git a/iptables/nft.c b/iptables/nft.c
index ef43d4e5445b5..30c9bd0ab8df0 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1057,7 +1057,8 @@ static int add_nft_limit(struct nftnl_rule *r, struct xt_entry_match *m)
 	return 0;
 }
 
-int add_match(struct nftnl_rule *r, struct xt_entry_match *m)
+int add_match(struct nft_handle *h,
+	      struct nftnl_rule *r, struct xt_entry_match *m)
 {
 	struct nftnl_expr *expr;
 	int ret;
@@ -1279,7 +1280,7 @@ nft_rule_new(struct nft_handle *h, const char *chain, const char *table,
 	nftnl_rule_set(r, NFTNL_RULE_TABLE, (char *)table);
 	nftnl_rule_set(r, NFTNL_RULE_CHAIN, (char *)chain);
 
-	if (h->ops->add(r, data) < 0)
+	if (h->ops->add(h, r, data) < 0)
 		goto err;
 
 	return r;
diff --git a/iptables/nft.h b/iptables/nft.h
index 2f929a15f98df..ff57976abbd64 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -134,7 +134,7 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain, const char *
  */
 int add_counters(struct nftnl_rule *r, uint64_t packets, uint64_t bytes);
 int add_verdict(struct nftnl_rule *r, int verdict);
-int add_match(struct nftnl_rule *r, struct xt_entry_match *m);
+int add_match(struct nft_handle *h, struct nftnl_rule *r, struct xt_entry_match *m);
 int add_target(struct nftnl_rule *r, struct xt_entry_target *t);
 int add_jumpto(struct nftnl_rule *r, const char *name, int verdict);
 int add_action(struct nftnl_rule *r, struct iptables_command_state *cs, bool goto_set);
-- 
2.22.0

[iptables PATCH 08/14] nft: family_ops: Pass nft_handle to 'rule_find' callback

[Phil Sutter]

In order to prepare for rules containing set references, nft handle has
to be passed to nft_rule_to_iptables_command_state() in order to let it
access the set in cache.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-arp.c    | 4 ++--
 iptables/nft-bridge.c | 4 ++--
 iptables/nft-shared.c | 7 +++----
 iptables/nft-shared.h | 4 ++--
 iptables/nft.c        | 2 +-
 5 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
index d9a5f861eecb1..78bcc3b4b6ffc 100644
--- a/iptables/nft-arp.c
+++ b/iptables/nft-arp.c
@@ -655,7 +655,7 @@ static bool nft_arp_is_same(const void *data_a,
 				  (unsigned char *)b->arp.outiface_mask);
 }
 
-static bool nft_arp_rule_find(struct nft_family_ops *ops, struct nftnl_rule *r,
+static bool nft_arp_rule_find(struct nft_handle *h, struct nftnl_rule *r,
 			      void *data)
 {
 	const struct iptables_command_state *cs = data;
@@ -676,7 +676,7 @@ static bool nft_arp_rule_find(struct nft_family_ops *ops, struct nftnl_rule *r,
 
 	ret = true;
 out:
-	ops->clear_cs(&this);
+	h->ops->clear_cs(&this);
 	return ret;
 }
 
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 0fc21b3a3c0d6..73bca2f38101e 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -537,7 +537,7 @@ 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_family_ops *ops, struct nftnl_rule *r,
+static bool nft_bridge_rule_find(struct nft_handle *h, struct nftnl_rule *r,
 				 void *data)
 {
 	struct iptables_command_state *cs = data;
@@ -568,7 +568,7 @@ static bool nft_bridge_rule_find(struct nft_family_ops *ops, struct nftnl_rule *
 
 	ret = true;
 out:
-	ops->clear_cs(&this);
+	h->ops->clear_cs(&this);
 	return ret;
 }
 
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 3ab21932f8fe6..0f5ca3b4eab23 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -970,8 +970,7 @@ void nft_ipv46_parse_target(struct xtables_target *t, void *data)
 	cs->target = t;
 }
 
-bool nft_ipv46_rule_find(struct nft_family_ops *ops,
-			 struct nftnl_rule *r, void *data)
+bool nft_ipv46_rule_find(struct nft_handle *h, struct nftnl_rule *r, void *data)
 {
 	struct iptables_command_state *cs = data, this = {};
 	bool ret = false;
@@ -982,7 +981,7 @@ bool nft_ipv46_rule_find(struct nft_family_ops *ops,
 #ifdef DEBUG_DEL
 	nft_rule_print_save(r, NFT_RULE_APPEND, 0);
 #endif
-	if (!ops->is_same(cs, &this))
+	if (!h->ops->is_same(cs, &this))
 		goto out;
 
 	if (!compare_matches(cs->matches, this.matches)) {
@@ -1002,7 +1001,7 @@ bool nft_ipv46_rule_find(struct nft_family_ops *ops,
 
 	ret = true;
 out:
-	ops->clear_cs(&this);
+	h->ops->clear_cs(&this);
 	return ret;
 }
 
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index 0972c228bc65f..6a4a92c9360a2 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -104,7 +104,7 @@ struct nft_family_ops {
 	void (*rule_to_cs)(const struct nftnl_rule *r,
 			   struct iptables_command_state *cs);
 	void (*clear_cs)(struct iptables_command_state *cs);
-	bool (*rule_find)(struct nft_family_ops *ops, struct nftnl_rule *r,
+	bool (*rule_find)(struct nft_handle *h, struct nftnl_rule *r,
 			  void *data);
 	int (*xlate)(const void *data, struct xt_xlate *xl);
 };
@@ -165,7 +165,7 @@ 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_family_ops *ops, struct nftnl_rule *r,
+bool nft_ipv46_rule_find(struct nft_handle *h, struct nftnl_rule *r,
 			 void *data);
 
 bool compare_matches(struct xtables_rule_match *mt1, struct xtables_rule_match *mt2);
diff --git a/iptables/nft.c b/iptables/nft.c
index 30c9bd0ab8df0..b51491348b116 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -2344,7 +2344,7 @@ nft_rule_find(struct nft_handle *h, struct nftnl_chain *c, void *data, int rulen
 
 	r = nftnl_rule_iter_next(iter);
 	while (r != NULL) {
-		found = h->ops->rule_find(h->ops, r, data);
+		found = h->ops->rule_find(h, r, data);
 		if (found)
 			break;
 		r = nftnl_rule_iter_next(iter);
-- 
2.22.0

[iptables PATCH 09/14] nft: family_ops: Pass nft_handle to 'print_rule' callback

[Phil Sutter]

Prepare for 'rule_to_cs' callback to receive nft_handle pointer so it is
able to access cache for set lookups.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-arp.c    |  4 ++--
 iptables/nft-bridge.c |  4 ++--
 iptables/nft-ipv4.c   |  4 ++--
 iptables/nft-ipv6.c   |  4 ++--
 iptables/nft-shared.h |  4 ++--
 iptables/nft.c        | 20 ++++++++++----------
 6 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
index 78bcc3b4b6ffc..2956469be6340 100644
--- a/iptables/nft-arp.c
+++ b/iptables/nft-arp.c
@@ -604,8 +604,8 @@ nft_arp_save_rule(const void *data, unsigned int format)
 	printf("\n");
 }
 
-static void
-nft_arp_print_rule(struct nftnl_rule *r, unsigned int num, unsigned int format)
+static void nft_arp_print_rule(struct nft_handle *h, struct nftnl_rule *r,
+			       unsigned int num, unsigned int format)
 {
 	struct iptables_command_state cs = {};
 
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 73bca2f38101e..b0c6c5a4db3cd 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -472,8 +472,8 @@ static void nft_bridge_save_rule(const void *data, unsigned int format)
 		fputc('\n', stdout);
 }
 
-static void nft_bridge_print_rule(struct nftnl_rule *r, unsigned int num,
-				  unsigned int format)
+static void nft_bridge_print_rule(struct nft_handle *h, struct nftnl_rule *r,
+				  unsigned int num, unsigned int format)
 {
 	struct iptables_command_state cs = {};
 
diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index 57d1b3c6d2d0c..98d7f966e3694 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -261,8 +261,8 @@ static void print_fragment(unsigned int flags, unsigned int invflags,
 	fputc(' ', stdout);
 }
 
-static void nft_ipv4_print_rule(struct nftnl_rule *r, unsigned int num,
-				unsigned int format)
+static void nft_ipv4_print_rule(struct nft_handle *h, struct nftnl_rule *r,
+				unsigned int num, unsigned int format)
 {
 	struct iptables_command_state cs = {};
 
diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
index 0e2c4a2946e25..56236bff03c2b 100644
--- a/iptables/nft-ipv6.c
+++ b/iptables/nft-ipv6.c
@@ -187,8 +187,8 @@ static void nft_ipv6_parse_immediate(const char *jumpto, bool nft_goto,
 		cs->fw6.ipv6.flags |= IP6T_F_GOTO;
 }
 
-static void nft_ipv6_print_rule(struct nftnl_rule *r, unsigned int num,
-				unsigned int format)
+static void nft_ipv6_print_rule(struct nft_handle *h, struct nftnl_rule *r,
+				unsigned int num, unsigned int format)
 {
 	struct iptables_command_state cs = {};
 
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index 6a4a92c9360a2..38e5fd5387c10 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -90,8 +90,8 @@ struct nft_family_ops {
 			     const char *pol,
 			     const struct xt_counters *counters, bool basechain,
 			     uint32_t refs, uint32_t entries);
-	void (*print_rule)(struct nftnl_rule *r, unsigned int num,
-			   unsigned int format);
+	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);
diff --git a/iptables/nft.c b/iptables/nft.c
index b51491348b116..1acc97702bcce 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1321,7 +1321,7 @@ nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
 	}
 
 	if (verbose)
-		h->ops->print_rule(r, 0, FMT_PRINT_RULE);
+		h->ops->print_rule(h, r, 0, FMT_PRINT_RULE);
 
 	if (ref) {
 		nftnl_chain_rule_insert_at(r, ref);
@@ -2372,7 +2372,7 @@ int nft_rule_check(struct nft_handle *h, const char *chain,
 		goto fail_enoent;
 
 	if (verbose)
-		h->ops->print_rule(r, 0, FMT_PRINT_RULE);
+		h->ops->print_rule(h, r, 0, FMT_PRINT_RULE);
 
 	return 1;
 fail_enoent:
@@ -2401,7 +2401,7 @@ int nft_rule_delete(struct nft_handle *h, const char *chain,
 		if (ret < 0)
 			errno = ENOMEM;
 		if (verbose)
-			h->ops->print_rule(r, 0, FMT_PRINT_RULE);
+			h->ops->print_rule(h, r, 0, FMT_PRINT_RULE);
 	} else
 		errno = ENOENT;
 
@@ -2442,7 +2442,7 @@ nft_rule_add(struct nft_handle *h, const char *chain,
 	}
 
 	if (verbose)
-		h->ops->print_rule(r, 0, FMT_PRINT_RULE);
+		h->ops->print_rule(h, r, 0, FMT_PRINT_RULE);
 
 	return r;
 }
@@ -2551,8 +2551,8 @@ int nft_rule_replace(struct nft_handle *h, const char *chain,
 static int
 __nft_rule_list(struct nft_handle *h, struct nftnl_chain *c,
 		int rulenum, unsigned int format,
-		void (*cb)(struct nftnl_rule *r, unsigned int num,
-			   unsigned int format))
+		void (*cb)(struct nft_handle *h, struct nftnl_rule *r,
+			   unsigned int num, unsigned int format))
 {
 	struct nftnl_rule_iter *iter;
 	struct nftnl_rule *r;
@@ -2565,7 +2565,7 @@ __nft_rule_list(struct nft_handle *h, struct nftnl_chain *c,
 			 * valid chain but invalid rule number
 			 */
 			return 1;
-		cb(r, rulenum, format);
+		cb(h, r, rulenum, format);
 		return 1;
 	}
 
@@ -2575,7 +2575,7 @@ __nft_rule_list(struct nft_handle *h, struct nftnl_chain *c,
 
 	r = nftnl_rule_iter_next(iter);
 	while (r != NULL) {
-		cb(r, ++rule_ctr, format);
+		cb(h, r, ++rule_ctr, format);
 		r = nftnl_rule_iter_next(iter);
 	}
 
@@ -2678,8 +2678,8 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
 	return 1;
 }
 
-static void
-list_save(struct nftnl_rule *r, unsigned int num, unsigned int format)
+static void list_save(struct nft_handle *h, struct nftnl_rule *r,
+		      unsigned int num, unsigned int format)
 {
 	nft_rule_print_save(r, NFT_RULE_APPEND, format);
 }
-- 
2.22.0

[iptables PATCH 10/14] nft: family_ops: Pass nft_handle to 'rule_to_cs' callback

[Phil Sutter]

This is the actual callback used to parse nftables rules. Pass
nft_handle to it so it can access the cache (and possible sets therein).

Having to pass nft_handle to nft_rule_print_save() allows to simplify it
a bit since no family ops lookup has to be done anymore.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-arp.c         |  4 ++--
 iptables/nft-bridge.c      |  9 +++++----
 iptables/nft-ipv4.c        |  2 +-
 iptables/nft-ipv6.c        |  2 +-
 iptables/nft-shared.c      |  5 +++--
 iptables/nft-shared.h      |  5 +++--
 iptables/nft.c             | 29 +++++++++++++----------------
 iptables/nft.h             |  4 ++--
 iptables/xtables-monitor.c | 17 +++++++++++++++--
 iptables/xtables-save.c    |  3 +++
 10 files changed, 48 insertions(+), 32 deletions(-)

diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
index 2956469be6340..c31d80eedfd60 100644
--- a/iptables/nft-arp.c
+++ b/iptables/nft-arp.c
@@ -612,7 +612,7 @@ static void nft_arp_print_rule(struct nft_handle *h, struct nftnl_rule *r,
 	if (format & FMT_LINENUMBERS)
 		printf("%u ", num);
 
-	nft_rule_to_iptables_command_state(r, &cs);
+	nft_rule_to_iptables_command_state(h, r, &cs);
 
 	nft_arp_print_rule_details(&cs, format);
 	print_matches_and_target(&cs, format);
@@ -663,7 +663,7 @@ static bool nft_arp_rule_find(struct nft_handle *h, struct nftnl_rule *r,
 	bool ret = false;
 
 	/* Delete by matching rule case */
-	nft_rule_to_iptables_command_state(r, &this);
+	nft_rule_to_iptables_command_state(h, r, &this);
 
 	if (!nft_arp_is_same(&cs->arp, &this.arp))
 		goto out;
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index b0c6c5a4db3cd..20ce92a6d5242 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -333,11 +333,12 @@ static void nft_bridge_parse_target(struct xtables_target *t, void *data)
 	cs->target = t;
 }
 
-static void nft_rule_to_ebtables_command_state(const struct nftnl_rule *r,
+static void nft_rule_to_ebtables_command_state(struct nft_handle *h,
+					       const struct nftnl_rule *r,
 					       struct iptables_command_state *cs)
 {
 	cs->eb.bitmask = EBT_NOPROTO;
-	nft_rule_to_iptables_command_state(r, cs);
+	nft_rule_to_iptables_command_state(h, r, cs);
 }
 
 static void print_iface(const char *option, const char *name, bool invert)
@@ -480,7 +481,7 @@ static void nft_bridge_print_rule(struct nft_handle *h, struct nftnl_rule *r,
 	if (format & FMT_LINENUMBERS)
 		printf("%d ", num);
 
-	nft_rule_to_ebtables_command_state(r, &cs);
+	nft_rule_to_ebtables_command_state(h, r, &cs);
 	nft_bridge_save_rule(&cs, format);
 	ebt_cs_clean(&cs);
 }
@@ -544,7 +545,7 @@ static bool nft_bridge_rule_find(struct nft_handle *h, struct nftnl_rule *r,
 	struct iptables_command_state this = {};
 	bool ret = false;
 
-	nft_rule_to_ebtables_command_state(r, &this);
+	nft_rule_to_ebtables_command_state(h, r, &this);
 
 	DEBUGP("comparing with... ");
 
diff --git a/iptables/nft-ipv4.c b/iptables/nft-ipv4.c
index 98d7f966e3694..70634f8fad84d 100644
--- a/iptables/nft-ipv4.c
+++ b/iptables/nft-ipv4.c
@@ -266,7 +266,7 @@ static void nft_ipv4_print_rule(struct nft_handle *h, struct nftnl_rule *r,
 {
 	struct iptables_command_state cs = {};
 
-	nft_rule_to_iptables_command_state(r, &cs);
+	nft_rule_to_iptables_command_state(h, r, &cs);
 
 	print_rule_details(&cs, cs.jumpto, cs.fw.ip.flags,
 			   cs.fw.ip.invflags, cs.fw.ip.proto, num, format);
diff --git a/iptables/nft-ipv6.c b/iptables/nft-ipv6.c
index 56236bff03c2b..d01491bfdb689 100644
--- a/iptables/nft-ipv6.c
+++ b/iptables/nft-ipv6.c
@@ -192,7 +192,7 @@ static void nft_ipv6_print_rule(struct nft_handle *h, struct nftnl_rule *r,
 {
 	struct iptables_command_state cs = {};
 
-	nft_rule_to_iptables_command_state(r, &cs);
+	nft_rule_to_iptables_command_state(h, r, &cs);
 
 	print_rule_details(&cs, cs.jumpto, cs.fw6.ipv6.flags,
 			   cs.fw6.ipv6.invflags, cs.fw6.ipv6.proto,
diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 0f5ca3b4eab23..d5984d0577ed1 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -580,7 +580,8 @@ static void nft_parse_limit(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 		ctx->h->ops->parse_match(match, ctx->cs);
 }
 
-void nft_rule_to_iptables_command_state(const struct nftnl_rule *r,
+void nft_rule_to_iptables_command_state(struct nft_handle *h,
+					const struct nftnl_rule *r,
 					struct iptables_command_state *cs)
 {
 	struct nftnl_expr_iter *iter;
@@ -975,7 +976,7 @@ bool nft_ipv46_rule_find(struct nft_handle *h, struct nftnl_rule *r, void *data)
 	struct iptables_command_state *cs = data, this = {};
 	bool ret = false;
 
-	nft_rule_to_iptables_command_state(r, &this);
+	nft_rule_to_iptables_command_state(h, r, &this);
 
 	DEBUGP("comparing with... ");
 #ifdef DEBUG_DEL
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index 38e5fd5387c10..ff3beef1af0de 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -101,7 +101,7 @@ struct nft_family_ops {
 			   struct xtables_args *args);
 	void (*parse_match)(struct xtables_match *m, void *data);
 	void (*parse_target)(struct xtables_target *t, void *data);
-	void (*rule_to_cs)(const struct nftnl_rule *r,
+	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,
@@ -138,7 +138,8 @@ int parse_meta(struct nftnl_expr *e, uint8_t key, char *iniface,
 		unsigned char *outiface_mask, uint8_t *invflags);
 void print_proto(uint16_t proto, int invert);
 void get_cmp_data(struct nftnl_expr *e, void *data, size_t dlen, bool *inv);
-void nft_rule_to_iptables_command_state(const struct nftnl_rule *r,
+void nft_rule_to_iptables_command_state(struct nft_handle *h,
+					const struct nftnl_rule *r,
 					struct iptables_command_state *cs);
 void nft_clear_iptables_command_state(struct iptables_command_state *cs);
 void print_header(unsigned int format, const char *chain, const char *pol,
diff --git a/iptables/nft.c b/iptables/nft.c
index 1acc97702bcce..821afa8f5e78c 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -389,7 +389,7 @@ static int mnl_append_error(const struct nft_handle *h,
 			 nftnl_rule_get_str(o->rule, NFTNL_RULE_CHAIN));
 #if 0
 		{
-			nft_rule_print_save(o->rule, NFT_RULE_APPEND, FMT_NOCOUNTS);
+			nft_rule_print_save(h, o->rule, NFT_RULE_APPEND, FMT_NOCOUNTS);
 		}
 #endif
 		break;
@@ -1339,19 +1339,16 @@ nft_rule_append(struct nft_handle *h, const char *chain, const char *table,
 }
 
 void
-nft_rule_print_save(const struct nftnl_rule *r, enum nft_rule_print type,
-		    unsigned int format)
+nft_rule_print_save(struct nft_handle *h, const struct nftnl_rule *r,
+		    enum nft_rule_print type, unsigned int format)
 {
 	const char *chain = nftnl_rule_get_str(r, NFTNL_RULE_CHAIN);
-	int family = nftnl_rule_get_u32(r, NFTNL_RULE_FAMILY);
 	struct iptables_command_state cs = {};
-	struct nft_family_ops *ops;
 
-	ops = nft_family_ops_lookup(family);
-	ops->rule_to_cs(r, &cs);
+	h->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)) && h->ops->save_counters)
+		h->ops->save_counters(&cs);
 
 	/* print chain name */
 	switch(type) {
@@ -1363,11 +1360,11 @@ nft_rule_print_save(const struct nftnl_rule *r, enum nft_rule_print type,
 		break;
 	}
 
-	if (ops->save_rule)
-		ops->save_rule(&cs, format);
+	if (h->ops->save_rule)
+		h->ops->save_rule(&cs, format);
 
-	if (ops->clear_cs)
-		ops->clear_cs(&cs);
+	if (h->ops->clear_cs)
+		h->ops->clear_cs(&cs);
 }
 
 static int nftnl_chain_list_cb(const struct nlmsghdr *nlh, void *data)
@@ -1836,7 +1833,7 @@ static int nft_chain_save_rules(struct nft_handle *h,
 
 	r = nftnl_rule_iter_next(iter);
 	while (r != NULL) {
-		nft_rule_print_save(r, NFT_RULE_APPEND, format);
+		nft_rule_print_save(h, r, NFT_RULE_APPEND, format);
 		r = nftnl_rule_iter_next(iter);
 	}
 
@@ -2681,7 +2678,7 @@ int nft_rule_list(struct nft_handle *h, const char *chain, const char *table,
 static void list_save(struct nft_handle *h, struct nftnl_rule *r,
 		      unsigned int num, unsigned int format)
 {
-	nft_rule_print_save(r, NFT_RULE_APPEND, format);
+	nft_rule_print_save(h, r, NFT_RULE_APPEND, format);
 }
 
 static int __nftnl_rule_list_chain_save(struct nftnl_chain *c, void *data)
@@ -2797,7 +2794,7 @@ int nft_rule_zero_counters(struct nft_handle *h, const char *chain,
 		goto error;
 	}
 
-	nft_rule_to_iptables_command_state(r, &cs);
+	nft_rule_to_iptables_command_state(h, r, &cs);
 
 	cs.counters.pcnt = cs.counters.bcnt = 0;
 
diff --git a/iptables/nft.h b/iptables/nft.h
index ff57976abbd64..09604692d66d0 100644
--- a/iptables/nft.h
+++ b/iptables/nft.h
@@ -145,8 +145,8 @@ enum nft_rule_print {
 	NFT_RULE_DEL,
 };
 
-void nft_rule_print_save(const struct nftnl_rule *r, enum nft_rule_print type,
-			 unsigned int format);
+void nft_rule_print_save(struct nft_handle *h, const struct nftnl_rule *r,
+			 enum nft_rule_print type, unsigned int format);
 
 uint32_t nft_invflags2cmp(uint32_t invflags, uint32_t flag);
 
diff --git a/iptables/xtables-monitor.c b/iptables/xtables-monitor.c
index eb80bac81c645..a5245d1422af9 100644
--- a/iptables/xtables-monitor.c
+++ b/iptables/xtables-monitor.c
@@ -11,6 +11,7 @@
 
 #define _GNU_SOURCE
 #include "config.h"
+#include <errno.h>
 #include <stdlib.h>
 #include <time.h>
 #include <string.h>
@@ -41,6 +42,7 @@
 struct cb_arg {
 	uint32_t nfproto;
 	bool is_event;
+	struct nft_handle *h;
 };
 
 static int table_cb(const struct nlmsghdr *nlh, void *data)
@@ -106,7 +108,7 @@ static int rule_cb(const struct nlmsghdr *nlh, void *data)
 	}
 
 	printf("-t %s ", nftnl_rule_get_str(r, NFTNL_RULE_TABLE));
-	nft_rule_print_save(r, type == NFT_MSG_NEWRULE ? NFT_RULE_APPEND :
+	nft_rule_print_save(arg->h, r, type == NFT_MSG_NEWRULE ? NFT_RULE_APPEND :
 							   NFT_RULE_DEL,
 			    counters ? 0 : FMT_NOCOUNTS);
 err_free:
@@ -593,7 +595,10 @@ int xtables_monitor_main(int argc, char *argv[])
 	struct mnl_socket *nl;
 	char buf[MNL_SOCKET_BUFFER_SIZE];
 	uint32_t nfgroup = 0;
-	struct cb_arg cb_arg = {};
+	struct nft_handle h = {};
+	struct cb_arg cb_arg = {
+		.h = &h,
+	};
 	int ret, c;
 
 	xtables_globals.program_name = "xtables-monitor";
@@ -610,6 +615,14 @@ int xtables_monitor_main(int argc, char *argv[])
 	init_extensions4();
 #endif
 
+	if (nft_init(&h, xtables_ipv4)) {
+		fprintf(stderr, "%s/%s Failed to initialize nft: %s\n",
+			xtables_globals.program_name,
+			xtables_globals.program_version,
+			strerror(errno));
+		exit(EXIT_FAILURE);
+	}
+
 	opterr = 0;
 	while ((c = getopt_long(argc, argv, "ceht46V", options, NULL)) != -1) {
 		switch (c) {
diff --git a/iptables/xtables-save.c b/iptables/xtables-save.c
index 77b13f7ffbcdd..88f974a55f15b 100644
--- a/iptables/xtables-save.c
+++ b/iptables/xtables-save.c
@@ -248,6 +248,9 @@ xtables_save_main(int family, int argc, char *argv[],
 				strerror(errno));
 		exit(EXIT_FAILURE);
 	}
+	h.ops = nft_family_ops_lookup(h.family);
+	if (!h.ops)
+		xtables_error(PARAMETER_PROBLEM, "Unknown family");
 
 	ret = do_output(&h, tablename, &d);
 	nft_fini(&h);
-- 
2.22.0

[iptables PATCH 11/14] nft: Bore up nft_parse_payload()

[Phil Sutter]

Allow for closer inspection by storing payload expression's base and
length values. Also facilitate for two consecutive payload expressions
as LHS of a (cmp/lookup) statement as used with concatenations.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-shared.c | 8 ++++++++
 iptables/nft-shared.h | 4 +++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index d5984d0577ed1..0f8cabf9abcc7 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -445,8 +445,16 @@ static void nft_parse_meta(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 
 static void nft_parse_payload(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 {
+	if (ctx->flags & NFT_XT_CTX_PAYLOAD) {
+		memcpy(&ctx->prev_payload, &ctx->payload,
+		       sizeof(ctx->prev_payload));
+		ctx->flags |= NFT_XT_CTX_PREV_PAYLOAD;
+	}
+
 	ctx->reg = nftnl_expr_get_u32(e, NFTNL_EXPR_META_DREG);
+	ctx->payload.base = nftnl_expr_get_u32(e, NFTNL_EXPR_PAYLOAD_BASE);
 	ctx->payload.offset = nftnl_expr_get_u32(e, NFTNL_EXPR_PAYLOAD_OFFSET);
+	ctx->payload.len = nftnl_expr_get_u32(e, NFTNL_EXPR_PAYLOAD_LEN);
 	ctx->flags |= NFT_XT_CTX_PAYLOAD;
 }
 
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index ff3beef1af0de..cb7eea6208cd2 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -43,6 +43,7 @@ enum {
 	NFT_XT_CTX_META		= (1 << 1),
 	NFT_XT_CTX_BITWISE	= (1 << 2),
 	NFT_XT_CTX_IMMEDIATE	= (1 << 3),
+	NFT_XT_CTX_PREV_PAYLOAD	= (1 << 4),
 };
 
 struct nft_xt_ctx {
@@ -53,9 +54,10 @@ struct nft_xt_ctx {
 
 	uint32_t reg;
 	struct {
+		uint32_t base;
 		uint32_t offset;
 		uint32_t len;
-	} payload;
+	} payload, prev_payload;
 	struct {
 		uint32_t key;
 	} meta;
-- 
2.22.0

Re: [iptables PATCH 11/14] nft: Bore up nft_parse_payload()

[Pablo Neira Ayuso]

On Wed, Aug 21, 2019 at 11:25:59AM +0200, Phil Sutter wrote:
> Allow for closer inspection by storing payload expression's base and
> length values. Also facilitate for two consecutive payload expressions
> as LHS of a (cmp/lookup) statement as used with concatenations.
> 
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  iptables/nft-shared.c | 8 ++++++++
>  iptables/nft-shared.h | 4 +++-
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
> index d5984d0577ed1..0f8cabf9abcc7 100644
> --- a/iptables/nft-shared.c
> +++ b/iptables/nft-shared.c
> @@ -445,8 +445,16 @@ static void nft_parse_meta(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
>  
>  static void nft_parse_payload(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
>  {
> +	if (ctx->flags & NFT_XT_CTX_PAYLOAD) {
> +		memcpy(&ctx->prev_payload, &ctx->payload,
> +		       sizeof(ctx->prev_payload));
> +		ctx->flags |= NFT_XT_CTX_PREV_PAYLOAD;
> +	}
>
>  	ctx->reg = nftnl_expr_get_u32(e, NFTNL_EXPR_META_DREG);
> +	ctx->payload.base = nftnl_expr_get_u32(e, NFTNL_EXPR_PAYLOAD_BASE);
>  	ctx->payload.offset = nftnl_expr_get_u32(e, NFTNL_EXPR_PAYLOAD_OFFSET);
> +	ctx->payload.len = nftnl_expr_get_u32(e, NFTNL_EXPR_PAYLOAD_LEN);
>  	ctx->flags |= NFT_XT_CTX_PAYLOAD;
>  }
>  
> diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
> index ff3beef1af0de..cb7eea6208cd2 100644
> --- a/iptables/nft-shared.h
> +++ b/iptables/nft-shared.h
> @@ -43,6 +43,7 @@ enum {
>  	NFT_XT_CTX_META		= (1 << 1),
>  	NFT_XT_CTX_BITWISE	= (1 << 2),
>  	NFT_XT_CTX_IMMEDIATE	= (1 << 3),
> +	NFT_XT_CTX_PREV_PAYLOAD	= (1 << 4),

Why does ebt among needs this?

Re: [iptables PATCH 11/14] nft: Bore up nft_parse_payload()

[Pablo Neira Ayuso]

On Tue, Aug 27, 2019 at 12:38:52PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Aug 21, 2019 at 11:25:59AM +0200, Phil Sutter wrote:
> > Allow for closer inspection by storing payload expression's base and
> > length values. Also facilitate for two consecutive payload expressions
> > as LHS of a (cmp/lookup) statement as used with concatenations.
> > 
> > Signed-off-by: Phil Sutter <phil@nwl.cc>
> > ---
> >  iptables/nft-shared.c | 8 ++++++++
> >  iptables/nft-shared.h | 4 +++-
> >  2 files changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
> > index d5984d0577ed1..0f8cabf9abcc7 100644
> > --- a/iptables/nft-shared.c
> > +++ b/iptables/nft-shared.c
> > @@ -445,8 +445,16 @@ static void nft_parse_meta(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
> >  
> >  static void nft_parse_payload(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
> >  {
> > +	if (ctx->flags & NFT_XT_CTX_PAYLOAD) {
> > +		memcpy(&ctx->prev_payload, &ctx->payload,
> > +		       sizeof(ctx->prev_payload));
> > +		ctx->flags |= NFT_XT_CTX_PREV_PAYLOAD;
> > +	}
> >
> >  	ctx->reg = nftnl_expr_get_u32(e, NFTNL_EXPR_META_DREG);
> > +	ctx->payload.base = nftnl_expr_get_u32(e, NFTNL_EXPR_PAYLOAD_BASE);
> >  	ctx->payload.offset = nftnl_expr_get_u32(e, NFTNL_EXPR_PAYLOAD_OFFSET);
> > +	ctx->payload.len = nftnl_expr_get_u32(e, NFTNL_EXPR_PAYLOAD_LEN);
> >  	ctx->flags |= NFT_XT_CTX_PAYLOAD;
> >  }
> >  
> > diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
> > index ff3beef1af0de..cb7eea6208cd2 100644
> > --- a/iptables/nft-shared.h
> > +++ b/iptables/nft-shared.h
> > @@ -43,6 +43,7 @@ enum {
> >  	NFT_XT_CTX_META		= (1 << 1),
> >  	NFT_XT_CTX_BITWISE	= (1 << 2),
> >  	NFT_XT_CTX_IMMEDIATE	= (1 << 3),
> > +	NFT_XT_CTX_PREV_PAYLOAD	= (1 << 4),
> 
> Why does ebt among needs this?

We can move this discussion to patch 14/14, where I'm suggesting you
store context for this.

[iptables PATCH 12/14] nft: Embed rule's table name in nft_xt_ctx

[Phil Sutter]

Down to the point where expression parsing happens, the rule's table is
not known anymore but relevant if set lookups are required.

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

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 0f8cabf9abcc7..5615dfae00569 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -597,6 +597,7 @@ void nft_rule_to_iptables_command_state(struct nft_handle *h,
 	struct nft_xt_ctx ctx = {
 		.cs = cs,
 		.h = h,
+		.table = nftnl_rule_get_str(r, NFTNL_RULE_TABLE),
 	};
 
 	iter = nftnl_expr_iter_create(r);
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index cb7eea6208cd2..6d2b237d90bbc 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -51,6 +51,7 @@ struct nft_xt_ctx {
 	struct nftnl_expr_iter *iter;
 	struct nft_handle *h;
 	uint32_t flags;
+	const char *table;
 
 	uint32_t reg;
 	struct {
-- 
2.22.0

[iptables PATCH 13/14] nft: Support parsing lookup expression

[Phil Sutter]

Add required glue code to support family specific lookup expression
parsers implemented as family_ops callback.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-shared.c | 9 +++++++++
 iptables/nft-shared.h | 2 ++
 iptables/nft.c        | 5 ++++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/iptables/nft-shared.c b/iptables/nft-shared.c
index 5615dfae00569..1c540ccfb70e1 100644
--- a/iptables/nft-shared.c
+++ b/iptables/nft-shared.c
@@ -588,6 +588,13 @@ static void nft_parse_limit(struct nft_xt_ctx *ctx, struct nftnl_expr *e)
 		ctx->h->ops->parse_match(match, ctx->cs);
 }
 
+static void nft_parse_lookup(struct nft_xt_ctx *ctx, struct nft_handle *h,
+			     struct nftnl_expr *e)
+{
+	if (ctx->h->ops->parse_lookup)
+		ctx->h->ops->parse_lookup(ctx, e, NULL);
+}
+
 void nft_rule_to_iptables_command_state(struct nft_handle *h,
 					const struct nftnl_rule *r,
 					struct iptables_command_state *cs)
@@ -628,6 +635,8 @@ void nft_rule_to_iptables_command_state(struct nft_handle *h,
 			nft_parse_target(&ctx, expr);
 		else if (strcmp(name, "limit") == 0)
 			nft_parse_limit(&ctx, expr);
+		else if (strcmp(name, "lookup") == 0)
+			nft_parse_lookup(&ctx, h, expr);
 
 		expr = nftnl_expr_iter_next(iter);
 	}
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index 6d2b237d90bbc..e5acb447045d3 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -86,6 +86,8 @@ struct nft_family_ops {
 			      void *data);
 	void (*parse_cmp)(struct nft_xt_ctx *ctx, struct nftnl_expr *e,
 			  void *data);
+	void (*parse_lookup)(struct nft_xt_ctx *ctx, struct nftnl_expr *e,
+			     void *data);
 	void (*parse_immediate)(const char *jumpto, bool nft_goto, void *data);
 
 	void (*print_table_header)(const char *tablename);
diff --git a/iptables/nft.c b/iptables/nft.c
index 821afa8f5e78c..4cc4b1798d979 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1065,6 +1065,8 @@ int add_match(struct nft_handle *h,
 
 	if (!strcmp(m->u.user.name, "limit"))
 		return add_nft_limit(r, m);
+	else if (!strcmp(m->u.user.name, "among"))
+		return add_nft_among(h, r, m);
 
 	expr = nftnl_expr_alloc("match");
 	if (expr == NULL)
@@ -3497,7 +3499,8 @@ static const char *supported_exprs[] = {
 	"cmp",
 	"bitwise",
 	"counter",
-	"immediate"
+	"immediate",
+	"lookup"
 };
 
 
-- 
2.22.0

[iptables PATCH 14/14] nft: bridge: Rudimental among extension support

[Phil Sutter]

Support among match as far as possible given the limitations of nftables
sets, namely limited to homogeneous MAC address only or MAC and IP
address only matches.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 extensions/libebt_among.c | 278 ++++++++++++++++++++++++++++++++++++++
 extensions/libebt_among.t |  16 +++
 iptables/ebtables-nft.8   |  66 ++++-----
 iptables/nft-bridge.c     | 238 ++++++++++++++++++++++++++++++++
 iptables/nft-bridge.h     |  21 +++
 iptables/nft.c            | 128 ++++++++++++++++++
 iptables/xtables-eb.c     |   1 +
 7 files changed, 717 insertions(+), 31 deletions(-)
 create mode 100644 extensions/libebt_among.c
 create mode 100644 extensions/libebt_among.t

diff --git a/extensions/libebt_among.c b/extensions/libebt_among.c
new file mode 100644
index 0000000000000..81875fa02f3c4
--- /dev/null
+++ b/extensions/libebt_among.c
@@ -0,0 +1,278 @@
+/* ebt_among
+ *
+ * Authors:
+ * Grzegorz Borowiak <grzes@gnu.univ.gda.pl>
+ *
+ * August, 2003
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <getopt.h>
+#include <ctype.h>
+#include <unistd.h>
+#include <netinet/ether.h>
+#include <linux/if_ether.h>
+#include <linux/netfilter_bridge/ebt_among.h>
+#include <sys/mman.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <xtables.h>
+#include "iptables/nft.h"
+#include "iptables/nft-bridge.h"
+
+#define AMONG_DST '1'
+#define AMONG_SRC '2'
+#define AMONG_DST_F '3'
+#define AMONG_SRC_F '4'
+
+static const struct option bramong_opts[] = {
+	{"among-dst", required_argument, 0, AMONG_DST},
+	{"among-src", required_argument, 0, AMONG_SRC},
+	{"among-dst-file", required_argument, 0, AMONG_DST_F},
+	{"among-src-file", required_argument, 0, AMONG_SRC_F},
+	{0}
+};
+
+static void bramong_print_help(void)
+{
+	printf(
+"`among' options:\n"
+"--among-dst      [!] list      : matches if ether dst is in list\n"
+"--among-src      [!] list      : matches if ether src is in list\n"
+"--among-dst-file [!] file      : obtain dst list from file\n"
+"--among-src-file [!] file      : obtain src list from file\n"
+"list has form:\n"
+" xx:xx:xx:xx:xx:xx[=ip.ip.ip.ip],yy:yy:yy:yy:yy:yy[=ip.ip.ip.ip]"
+",...,zz:zz:zz:zz:zz:zz[=ip.ip.ip.ip][,]\n"
+"Things in brackets are optional.\n"
+"If you want to allow two (or more) IP addresses to one MAC address, you\n"
+"can specify two (or more) pairs with the same MAC, e.g.\n"
+" 00:00:00:fa:eb:fe=153.19.120.250,00:00:00:fa:eb:fe=192.168.0.1\n"
+	);
+}
+
+static int
+parse_nft_among_pairs(char *buf, struct nft_among_pair **pairsp,
+		      int *cntp, bool *ipp)
+{
+	struct nft_among_pair *pairs;
+	int cnt = 0, i, idx = 0;
+	bool ip = false;
+	char *p;
+
+	if (*buf)
+		cnt++;
+	for (p = buf; *p; p++) {
+		if (*p == ',')
+			cnt++;
+	}
+	if (!cnt)
+		return -1;
+
+	pairs = xtables_calloc(cnt, sizeof(*pairs));
+	p = strtok(buf, ",");
+	while (p) {
+		struct nft_among_pair tmpair = {};
+		unsigned int tmp[ETH_ALEN] = {};
+		char *sep = index(p, '=');
+
+		if (sep) {
+			if (idx > 0 && !ip)
+				xtables_error(PARAMETER_PROBLEM,
+					      "among: Mixed MAC and MAC=IP not allowed.");
+			ip = true;
+			*sep = '\0';
+			if (sscanf(sep + 1, "%d.%d.%d.%d",
+				   &tmp[0], &tmp[1], &tmp[2], &tmp[3]) != 4)
+					xtables_error(PARAMETER_PROBLEM,
+						      "Invalid IP address '%s'\n",
+						      sep + 1);
+			for (i = 0; i < 4; i++) {
+				if (tmp[i] > 255)
+					xtables_error(PARAMETER_PROBLEM,
+						      "Invalid IP address '%s'\n",
+						      sep + 1);
+				tmpair.ip[i] = tmp[i];
+			}
+		} else if (idx > 0 && ip) {
+				xtables_error(PARAMETER_PROBLEM,
+					      "among: Mixed MAC and MAC=IP not allowed.");
+		}
+		if (sscanf(p, "%x:%x:%x:%x:%x:%x",
+			   &tmp[0], &tmp[1], &tmp[2],
+			   &tmp[3], &tmp[4], &tmp[5]) != 6)
+				xtables_error(PARAMETER_PROBLEM,
+					      "Invalid MAC address '%s'\n", p);
+		for (i = 0; i < ETH_ALEN; i++) {
+			if (tmp[i] > 255)
+				xtables_error(PARAMETER_PROBLEM,
+					      "Invalid MAC address '%s'\n", p);
+			tmpair.mac[i] = tmp[i];
+		}
+		for (i = 0; i < idx; i++) {
+			if (memcmp(tmpair.buf, pairs[i].buf,
+				   sizeof(tmpair.buf)) < 0)
+				break;
+		}
+		memmove(pairs + i + 1, pairs + i, sizeof(*pairs) * (idx - i));
+		memcpy(pairs + i, &tmpair, sizeof(tmpair));
+		idx++;
+		p = strtok(NULL, ",");
+	}
+
+	if (pairsp)
+		*pairsp = pairs;
+	if (cntp)
+		*cntp = cnt;
+	if (ipp)
+		*ipp = ip;
+	return 0;
+}
+
+static int bramong_parse(int c, char **argv, int invert,
+		 unsigned int *flags, const void *entry,
+		 struct xt_entry_match **match)
+{
+	struct nft_among_data *data = (struct nft_among_data *)(*match)->data;
+	struct xt_entry_match *new_match;
+	struct nft_among_pair *pairs;
+	struct stat stats;
+	bool dst = false;
+	int fd = -1, cnt;
+	size_t new_size;
+	long flen = 0;
+	int poff;
+	bool ip;
+	int ret;
+
+	switch (c) {
+	case AMONG_DST_F:
+		dst = true;
+		/* fall through */
+	case AMONG_SRC_F:
+		if ((fd = open(optarg, O_RDONLY)) == -1)
+			xtables_error(PARAMETER_PROBLEM,
+				      "Couldn't open file '%s'", optarg);
+		fstat(fd, &stats);
+		flen = stats.st_size;
+		/* use mmap because the file will probably be big */
+		optarg = mmap(0, flen, PROT_READ | PROT_WRITE,
+			      MAP_PRIVATE, fd, 0);
+		if (optarg == MAP_FAILED)
+			xtables_error(PARAMETER_PROBLEM,
+				      "Couldn't map file to memory");
+		if (optarg[flen-1] != '\n')
+			xtables_error(PARAMETER_PROBLEM,
+				      "File should end with a newline");
+		if (strchr(optarg, '\n') != optarg+flen-1)
+			xtables_error(PARAMETER_PROBLEM,
+				      "File should only contain one line");
+		optarg[flen-1] = '\0';
+		/* fall through */
+	case AMONG_DST:
+		if (c == AMONG_DST)
+			dst = true;
+		/* fall through */
+	case AMONG_SRC:
+		break;
+	default:
+		return 0;
+	}
+
+	ret = parse_nft_among_pairs(optarg, &pairs, &cnt, &ip);
+	if (ret)
+		return 0;
+	new_size = data->src.cnt + data->dst.cnt + cnt;
+	new_size *= sizeof(struct nft_among_pair);
+	new_size += XT_ALIGN(sizeof(struct xt_entry_match)) +
+			sizeof(struct nft_among_data);
+	new_match = xtables_calloc(1, new_size);
+	memcpy(new_match, *match, (*match)->u.match_size);
+	new_match->u.match_size = new_size;
+
+	data = (struct nft_among_data *)new_match->data;
+	if (dst) {
+		data->dst.cnt = cnt;
+		data->dst.inv = invert;
+		data->dst.ip = ip;
+		poff = data->src.cnt;
+	} else {
+		data->src.cnt = cnt;
+		data->src.inv = invert;
+		data->src.ip = ip;
+		poff = 0;
+		memmove(data->pairs + cnt, data->pairs,
+			data->dst.cnt * sizeof(struct nft_among_pair));
+	}
+	memcpy(data->pairs + poff, pairs, cnt * sizeof(struct nft_among_pair));
+	free(pairs);
+	free(*match);
+	*match = new_match;
+
+	if (c == AMONG_DST_F || c == AMONG_SRC_F) {
+		munmap(argv, flen);
+		close(fd);
+	}
+	return 1;
+}
+
+static void __bramong_print(struct nft_among_pair *pairs,
+			    int cnt, bool inv, bool ip)
+{
+	const char *isep = "", *sep;
+	int i, j;
+
+	if (inv)
+		printf("! ");
+
+	for (i = 0; i < cnt; i++) {
+		printf("%s", isep);
+		isep = ",";
+
+		for (sep = "", j = 0; j < ETH_ALEN; sep = ":", j++)
+			printf("%s%02x", sep, pairs[i].mac[j]);
+
+		if (!ip)
+			continue;
+		for (sep = "=", j = 0; j < 4; sep = ".", j++)
+			printf("%s%u", sep, pairs[i].ip[j]);
+	}
+	printf(" ");
+}
+
+static void bramong_print(const void *ip, const struct xt_entry_match *match,
+			  int numeric)
+{
+	struct nft_among_data *data = (struct nft_among_data *)match->data;
+
+	if (data->src.cnt) {
+		printf("--among-src ");
+		__bramong_print(data->pairs,
+				data->src.cnt, data->src.inv, data->src.ip);
+	}
+	if (data->dst.cnt) {
+		printf("--among-dst ");
+		__bramong_print(data->pairs + data->src.cnt,
+				data->dst.cnt, data->dst.inv, data->dst.ip);
+	}
+}
+
+static struct xtables_match bramong_match = {
+	.name		= "among",
+	.revision	= 0,
+	.version	= XTABLES_VERSION,
+	.family		= NFPROTO_BRIDGE,
+	.size		= XT_ALIGN(sizeof(struct nft_among_data)),
+	.userspacesize	= XT_ALIGN(sizeof(struct nft_among_data)),
+	.help		= bramong_print_help,
+	.parse		= bramong_parse,
+	.print		= bramong_print,
+	.extra_opts	= bramong_opts,
+};
+
+void _init(void)
+{
+	xtables_register_match(&bramong_match);
+}
diff --git a/extensions/libebt_among.t b/extensions/libebt_among.t
new file mode 100644
index 0000000000000..cfdbbcaf3555d
--- /dev/null
+++ b/extensions/libebt_among.t
@@ -0,0 +1,16 @@
+:INPUT,FORWARD,OUTPUT
+--among-dst de:ad:00:be:ee:ff,c0:ff:ee:00:ba:be;--among-dst c0:ff:ee:00:ba:be,de:ad:00:be:ee:ff;OK
+--among-dst ! c0:ff:ee:00:ba:be,de:ad:00:be:ee:ff;=;OK
+--among-src be:ef:00:c0:ff:ee,c0:ff:ee:00:ba:be,de:ad:00:be:ee:ff;=;OK
+--among-src de:ad:00:be:ee:ff=10.0.0.1,c0:ff:ee:00:ba:be=192.168.1.1;--among-src c0:ff:ee:00:ba:be=192.168.1.1,de:ad:00:be:ee:ff=10.0.0.1;OK
+--among-src ! c0:ff:ee:00:ba:be=192.168.1.1,de:ad:00:be:ee:ff=10.0.0.1;=;OK
+--among-src de:ad:00:be:ee:ff --among-dst c0:ff:ee:00:ba:be;=;OK
+--among-src de:ad:00:be:ee:ff=10.0.0.1 --among-dst c0:ff:ee:00:ba:be=192.168.1.1;=;OK
+--among-src ! de:ad:00:be:ee:ff --among-dst c0:ff:ee:00:ba:be;=;OK
+--among-src de:ad:00:be:ee:ff=10.0.0.1 --among-dst ! c0:ff:ee:00:ba:be=192.168.1.1;=;OK
+--among-src ! de:ad:00:be:ee:ff --among-dst c0:ff:ee:00:ba:be=192.168.1.1;=;OK
+--among-src de:ad:00:be:ee:ff=10.0.0.1 --among-dst ! c0:ff:ee:00:ba:be=192.168.1.1;=;OK
+--among-src;=;FAIL
+--among-src 00:11=10.0.0.1;=;FAIL
+--among-src de:ad:00:be:ee:ff=10.256.0.1;=;FAIL
+--among-src de:ad:00:be:ee:ff,c0:ff:ee:00:ba:be=192.168.1.1;=;FAIL
diff --git a/iptables/ebtables-nft.8 b/iptables/ebtables-nft.8
index db8b2ab28cca5..a91f0c1aacb0f 100644
--- a/iptables/ebtables-nft.8
+++ b/iptables/ebtables-nft.8
@@ -522,35 +522,39 @@ If the 802.3 DSAP and SSAP values are 0xaa then the SNAP type field must
 be consulted to determine the payload protocol.  This is a two byte
 (hexadecimal) argument.  Only 802.3 frames with DSAP/SSAP 0xaa are
 checked for type.
-.\" .SS among
-.\" Match a MAC address or MAC/IP address pair versus a list of MAC addresses
-.\" and MAC/IP address pairs.
-.\" A list entry has the following format:
-.\" .IR xx:xx:xx:xx:xx:xx[=ip.ip.ip.ip][,] ". Multiple"
-.\" list entries are separated by a comma, specifying an IP address corresponding to
-.\" the MAC address is optional. Multiple MAC/IP address pairs with the same MAC address
-.\" but different IP address (and vice versa) can be specified. If the MAC address doesn't
-.\" match any entry from the list, the frame doesn't match the rule (unless "!" was used).
-.\" .TP
-.\" .BR "--among-dst " "[!] \fIlist\fP"
-.\" Compare the MAC destination to the given list. If the Ethernet frame has type
-.\" .IR IPv4 " or " ARP ,
-.\" then comparison with MAC/IP destination address pairs from the
-.\" list is possible.
-.\" .TP
-.\" .BR "--among-src " "[!] \fIlist\fP"
-.\" Compare the MAC source to the given list. If the Ethernet frame has type
-.\" .IR IPv4 " or " ARP ,
-.\" then comparison with MAC/IP source address pairs from the list
-.\" is possible.
-.\" .TP
-.\" .BR "--among-dst-file " "[!] \fIfile\fP"
-.\" Same as
-.\" .BR --among-dst " but the list is read in from the specified file."
-.\" .TP
-.\" .BR "--among-src-file " "[!] \fIfile\fP"
-.\" Same as
-.\" .BR --among-src " but the list is read in from the specified file."
+.SS among
+Match a MAC address or MAC/IP address pair versus a list of MAC addresses
+and MAC/IP address pairs.
+A list entry has the following format:
+.IR xx:xx:xx:xx:xx:xx[=ip.ip.ip.ip][,] ". Multiple"
+list entries are separated by a comma, specifying an IP address corresponding to
+the MAC address is optional. Multiple MAC/IP address pairs with the same MAC address
+but different IP address (and vice versa) can be specified. If the MAC address doesn't
+match any entry from the list, the frame doesn't match the rule (unless "!" was used).
+.TP
+.BR "--among-dst " "[!] \fIlist\fP"
+Compare the MAC destination to the given list. If the Ethernet frame has type
+.IR IPv4 " or " ARP ,
+then comparison with MAC/IP destination address pairs from the
+list is possible.
+.TP
+.BR "--among-src " "[!] \fIlist\fP"
+Compare the MAC source to the given list. If the Ethernet frame has type
+.IR IPv4 " or " ARP ,
+then comparison with MAC/IP source address pairs from the list
+is possible.
+.TP
+.BR "--among-dst-file " "[!] \fIfile\fP"
+Same as
+.BR --among-dst " but the list is read in from the specified file."
+.TP
+.BR "--among-src-file " "[!] \fIfile\fP"
+Same as
+.BR --among-src " but the list is read in from the specified file."
+.PP
+Note that in this implementation of ebtables, among lists uses must be
+internally homogeneous regarding whether IP addresses are present or not. Mixed
+use of MAC addresses and MAC/IP address pairs is not supported yet.
 .SS arp
 Specify (R)ARP fields. The protocol must be specified as
 .IR ARP " or " RARP .
@@ -1108,8 +1112,8 @@ arp message and the hardware address length in the arp header is 6 bytes.
 The version of ebtables this man page ships with does not support the
 .B broute
 table. Also there is no support for
-.BR among " and " string
-matches. And finally, this list is probably not complete.
+.B string
+match. And finally, this list is probably not complete.
 .SH SEE ALSO
 .BR xtables-nft "(8), " iptables "(8), " ip (8)
 .PP
diff --git a/iptables/nft-bridge.c b/iptables/nft-bridge.c
index 20ce92a6d5242..7e5425c43c3d5 100644
--- a/iptables/nft-bridge.c
+++ b/iptables/nft-bridge.c
@@ -17,6 +17,8 @@
 #include <libiptc/libxtc.h>
 #include <linux/netfilter/nf_tables.h>
 
+#include <libnftnl/set.h>
+
 #include "nft-shared.h"
 #include "nft-bridge.h"
 #include "nft.h"
@@ -291,6 +293,241 @@ static void nft_bridge_parse_immediate(const char *jumpto, bool nft_goto,
 	cs->jumpto = jumpto;
 }
 
+/* return 0 if saddr, 1 if daddr, -1 on error */
+static int lookup_check_ether_payload(uint32_t base, uint32_t offset, uint32_t len)
+{
+	if (base != 0 || len != ETH_ALEN)
+		return -1;
+
+	switch (offset) {
+	case offsetof(struct ether_header, ether_dhost):
+		return 1;
+	case offsetof(struct ether_header, ether_shost):
+		return 0;
+	default:
+		return -1;
+	}
+}
+
+/* return 0 if saddr, 1 if daddr, -1 on error */
+static int lookup_check_iphdr_payload(uint32_t base, uint32_t offset, uint32_t len)
+{
+	if (base != 1 || len != 4)
+		return -1;
+
+	switch (offset) {
+	case offsetof(struct iphdr, daddr):
+		return 1;
+	case offsetof(struct iphdr, saddr):
+		return 0;
+	default:
+		return -1;
+	}
+}
+
+/* XXX: move this into libnftnl, replacing nftnl_set_lookup() */
+static struct nftnl_set *nft_set_byname(struct nft_handle *h,
+					const char *table, const char *set)
+{
+	struct nftnl_set_list_iter *iter;
+	struct nftnl_set_list *slist;
+	struct nftnl_set *s;
+
+	slist = nft_set_list_get(h, table);
+	if (!slist)
+		return NULL;
+
+	iter = nftnl_set_list_iter_create(slist);
+	if (!iter)
+		return NULL;
+
+	s = nftnl_set_list_iter_cur(iter);
+	while (s && strcmp(nftnl_set_get_str(s, NFTNL_SET_NAME), set))
+		s = nftnl_set_list_iter_next(iter);
+	nftnl_set_list_iter_destroy(iter);
+
+	return s;
+}
+
+/* Make sure previous payload expression(s) is/are consistent and extract if
+ * matching on source or destination address and if matching on MAC and IP or
+ * only MAC address. */
+static int lookup_analyze_payloads(const struct nft_xt_ctx *ctx,
+				   bool *dst, bool *ip)
+{
+	int val, val2 = -1;
+
+	if (ctx->flags & NFT_XT_CTX_PREV_PAYLOAD) {
+		val = lookup_check_ether_payload(ctx->prev_payload.base,
+						 ctx->prev_payload.offset,
+						 ctx->prev_payload.len);
+		if (val < 0) {
+			DEBUGP("unknown payload base/offset/len %d/%d/%d\n",
+			       ctx->prev_payload.base, ctx->prev_payload.offset,
+			       ctx->prev_payload.len);
+			return -1;
+		}
+		if (!(ctx->flags & NFT_XT_CTX_PAYLOAD)) {
+			DEBUGP("Previous but no current payload?\n");
+			return -1;
+		}
+		val2 = lookup_check_iphdr_payload(ctx->payload.base,
+						  ctx->payload.offset,
+						  ctx->payload.len);
+		if (val2 < 0) {
+			DEBUGP("unknown payload base/offset/len %d/%d/%d\n",
+			       ctx->payload.base, ctx->payload.offset,
+			       ctx->payload.len);
+			return -1;
+		} else if (val != val2) {
+			DEBUGP("mismatching payload match offsets\n");
+			return -1;
+		}
+	} else if (ctx->flags & NFT_XT_CTX_PAYLOAD) {
+		val = lookup_check_ether_payload(ctx->payload.base,
+						 ctx->payload.offset,
+						 ctx->payload.len);
+		if (val < 0) {
+			DEBUGP("unknown payload base/offset/len %d/%d/%d\n",
+			       ctx->payload.base, ctx->payload.offset,
+			       ctx->payload.len);
+			return -1;
+		}
+	} else {
+		DEBUGP("unknown LHS of lookup expression\n");
+		return -1;
+	}
+
+	if (dst)
+		*dst = (val == 1);
+	if (ip)
+		*ip = (val2 != -1);
+	return 0;
+}
+
+static struct nft_among_pair *
+set_elems_to_among_pairs(const struct nftnl_set *s, int cnt)
+{
+	struct nftnl_set_elems_iter *iter = nftnl_set_elems_iter_create(s);
+	struct nft_among_pair *pairs;
+	struct nftnl_set_elem *elem;
+	int idx, tmpcnt = 0;
+	const char *buf;
+	uint32_t buflen;
+
+	if (!iter) {
+		fprintf(stderr, "BUG: set elems iter allocation failed\n");
+		exit(EXIT_FAILURE);
+	}
+
+	pairs = xtables_calloc(cnt, sizeof(*pairs));
+
+	while ((elem = nftnl_set_elems_iter_next(iter))) {
+		buf = nftnl_set_elem_get(elem, NFTNL_SET_ELEM_KEY, &buflen);
+		if (!buf) {
+			fprintf(stderr, "BUG: set elem without key\n");
+			exit(EXIT_FAILURE);
+		}
+		for (idx = 0; idx < tmpcnt; idx++) {
+			if (memcmp(buf, pairs[idx].buf, buflen) < 0)
+				break;
+		}
+		memmove(pairs + idx + 1, pairs + idx,
+			sizeof(*pairs) * (tmpcnt - idx));
+		memcpy(pairs[idx].buf, buf, buflen);
+		tmpcnt++;
+	}
+	nftnl_set_elems_iter_destroy(iter);
+
+	return pairs;
+}
+
+static void nft_bridge_parse_lookup(struct nft_xt_ctx *ctx, struct nftnl_expr *e,
+				    void *data)
+{
+	struct xtables_match *match = NULL;
+	struct nft_among_data *among_data;
+	struct nft_among_pair *pairs;
+	bool is_dst, have_ip, inv;
+	struct ebt_match *ematch;
+	const char *set_name;
+	struct nftnl_set *s;
+	int poff, cnt;
+	size_t size;
+
+	if (lookup_analyze_payloads(ctx, &is_dst, &have_ip))
+		return;
+
+	set_name = nftnl_expr_get_str(e, NFTNL_EXPR_LOOKUP_SET);
+	s = nft_set_byname(ctx->h, ctx->table, set_name);
+	if (!s) {
+		fprintf(stderr,
+			"BUG: set '%s' in lookup expression not found\n",
+			set_name);
+		exit(EXIT_FAILURE);
+	}
+
+	cnt = nftnl_set_get_u32(s, NFTNL_SET_DESC_SIZE);
+
+	for (ematch = ctx->cs->match_list; ematch; ematch = ematch->next) {
+		if (!ematch->ismatch || strcmp(ematch->u.match->name, "among"))
+			continue;
+
+		match = ematch->u.match;
+		among_data = (struct nft_among_data *)match->m->data;
+
+		size = cnt + among_data->src.cnt + among_data->dst.cnt;
+		size *= sizeof(struct nft_among_pair);
+		size += XT_ALIGN(sizeof(struct xt_entry_match)) +
+			sizeof(struct nft_among_data);
+
+		match->m = xtables_realloc(match->m, size);
+		break;
+	}
+	if (!match) {
+		match = xtables_find_match("among", XTF_TRY_LOAD,
+					   &ctx->cs->matches);
+		size = XT_ALIGN(sizeof(struct xt_entry_match)) +
+			sizeof(struct nft_among_data) +
+			cnt * sizeof(struct nft_among_pair);
+		match->m = xtables_calloc(1, size);
+		strcpy(match->m->u.user.name, match->name);
+		match->m->u.user.revision = match->revision;
+		xs_init_match(match);
+
+		if (ctx->h->ops->parse_match != NULL)
+			ctx->h->ops->parse_match(match, ctx->cs);
+	}
+	if (match == NULL)
+		return;
+
+	match->m->u.match_size = size;
+
+	inv = !!(nftnl_expr_get_u32(e, NFTNL_EXPR_LOOKUP_FLAGS) &
+				    NFT_LOOKUP_F_INV);
+
+	among_data = (struct nft_among_data *)match->m->data;
+	if (is_dst) {
+		among_data->dst.cnt = cnt;
+		among_data->dst.inv = inv;
+		among_data->dst.ip = have_ip;
+		poff = among_data->src.cnt;
+	} else {
+		among_data->src.cnt = cnt;
+		among_data->src.inv = inv;
+		among_data->src.ip = have_ip;
+		poff = 0;
+		memmove(among_data->pairs + cnt, among_data->pairs,
+			among_data->dst.cnt * sizeof(struct nft_among_pair));
+	}
+
+	pairs = set_elems_to_among_pairs(s, cnt);
+	memcpy(among_data->pairs + poff, pairs, cnt * sizeof(*pairs));
+	free(pairs);
+
+	ctx->flags &= ~(NFT_XT_CTX_PAYLOAD | NFT_XT_CTX_PREV_PAYLOAD);
+}
+
 static void parse_watcher(void *object, struct ebt_match **match_list,
 			  bool ismatch)
 {
@@ -742,6 +979,7 @@ struct nft_family_ops nft_family_ops_bridge = {
 	.parse_meta		= nft_bridge_parse_meta,
 	.parse_payload		= nft_bridge_parse_payload,
 	.parse_immediate	= nft_bridge_parse_immediate,
+	.parse_lookup		= nft_bridge_parse_lookup,
 	.parse_match		= nft_bridge_parse_match,
 	.parse_target		= nft_bridge_parse_target,
 	.print_table_header	= nft_bridge_print_table_header,
diff --git a/iptables/nft-bridge.h b/iptables/nft-bridge.h
index d90066f1030a2..15a437574988a 100644
--- a/iptables/nft-bridge.h
+++ b/iptables/nft-bridge.h
@@ -122,4 +122,25 @@ void ebt_add_watcher(struct xtables_target *watcher,
                      struct iptables_command_state *cs);
 int ebt_command_default(struct iptables_command_state *cs);
 
+struct nft_among_pair {
+	union {
+		struct {
+			unsigned char mac[ETH_ALEN];
+			unsigned char pad[2];
+			unsigned char ip[4];
+		};
+		unsigned char buf[ETH_ALEN + 2 + 4];
+	} __attribute__((packed));
+};
+
+struct nft_among_data {
+	struct {
+		int cnt;
+		bool inv;
+		bool ip;
+	} src, dst;
+	/* first source, then dest pairs */
+	struct nft_among_pair pairs[0];
+};
+
 #endif
diff --git a/iptables/nft.c b/iptables/nft.c
index 4cc4b1798d979..540e057c47b75 100644
--- a/iptables/nft.c
+++ b/iptables/nft.c
@@ -1057,6 +1057,134 @@ static int add_nft_limit(struct nftnl_rule *r, struct xt_entry_match *m)
 	return 0;
 }
 
+static struct nftnl_set *add_anon_set(struct nft_handle *h, const char *table,
+				      uint32_t flags, uint32_t key_type,
+				      uint32_t key_len, uint32_t size)
+{
+	static uint32_t set_id = 0;
+	struct nftnl_set *s;
+
+	s = nftnl_set_alloc();
+	if (!s)
+		return NULL;
+
+	nftnl_set_set_u32(s, NFTNL_SET_FAMILY, h->family);
+	nftnl_set_set_str(s, NFTNL_SET_TABLE, table);
+	nftnl_set_set_str(s, NFTNL_SET_NAME, "__set%d");
+	nftnl_set_set_u32(s, NFTNL_SET_ID, ++set_id);
+	nftnl_set_set_u32(s, NFTNL_SET_FLAGS,
+			  NFT_SET_ANONYMOUS | NFT_SET_CONSTANT | flags);
+	nftnl_set_set_u32(s, NFTNL_SET_KEY_TYPE, key_type);
+	nftnl_set_set_u32(s, NFTNL_SET_KEY_LEN, key_len);
+	nftnl_set_set_u32(s, NFTNL_SET_DESC_SIZE, size);
+
+	return batch_set_add(h, NFT_COMPAT_SET_ADD, s) ? s : NULL;
+}
+
+static struct nftnl_expr *
+gen_payload(uint32_t base, uint32_t offset, uint32_t len, uint32_t dreg)
+{
+	struct nftnl_expr *e = nftnl_expr_alloc("payload");
+
+	if (!e)
+		return NULL;
+	nftnl_expr_set_u32(e, NFTNL_EXPR_PAYLOAD_BASE, base);
+	nftnl_expr_set_u32(e, NFTNL_EXPR_PAYLOAD_OFFSET, offset);
+	nftnl_expr_set_u32(e, NFTNL_EXPR_PAYLOAD_LEN, len);
+	nftnl_expr_set_u32(e, NFTNL_EXPR_PAYLOAD_DREG, dreg);
+	return e;
+}
+
+static struct nftnl_expr *
+gen_lookup(uint32_t sreg, const char *set_name, uint32_t set_id, uint32_t flags)
+{
+	struct nftnl_expr *e = nftnl_expr_alloc("lookup");
+
+	if (!e)
+		return NULL;
+	nftnl_expr_set_u32(e, NFTNL_EXPR_LOOKUP_SREG, sreg);
+	nftnl_expr_set_str(e, NFTNL_EXPR_LOOKUP_SET, set_name);
+	nftnl_expr_set_u32(e, NFTNL_EXPR_LOOKUP_SET_ID, set_id);
+	nftnl_expr_set_u32(e, NFTNL_EXPR_LOOKUP_FLAGS, flags);
+	return e;
+}
+
+static int __add_nft_among(struct nft_handle *h, const char *table,
+			   struct nftnl_rule *r, struct nft_among_pair *pairs,
+			   int cnt, bool dst, bool inv, bool ip)
+{
+	uint32_t set_id, type = 9, len = 6;
+	/*			!dst, dst */
+	int eth_addr_off[] = { 6, 0 };
+	int ip_addr_off[] = { 12, 16 };
+	struct nftnl_expr *e;
+	struct nftnl_set *s;
+	int idx = 0;
+
+	if (ip) {
+		type = type << 6 | 7;
+		len += 4 + 2;
+	}
+
+	s = add_anon_set(h, table, 0, type, len, cnt);
+	if (!s)
+		return -ENOMEM;
+	set_id = nftnl_set_get_u32(s, NFTNL_SET_ID);
+
+	for (idx = 0; idx < cnt; idx++) {
+		struct nftnl_set_elem *elem = nftnl_set_elem_alloc();
+
+		if (!elem)
+			return -ENOMEM;
+		nftnl_set_elem_set(elem, NFTNL_SET_ELEM_KEY,
+				   pairs[idx].buf, len);
+		nftnl_set_elem_add(s, elem);
+	}
+
+	e = gen_payload(0, eth_addr_off[dst], ETH_ALEN, NFT_REG_1);
+	if (!e)
+		return -ENOMEM;
+	nftnl_rule_add_expr(r, e);
+
+	if (ip) {
+		e = gen_payload(1, ip_addr_off[dst], 4, NFT_REG32_02);
+		if (!e)
+			return -ENOMEM;
+		nftnl_rule_add_expr(r, e);
+	}
+
+	e = gen_lookup(NFT_REG_1, "__set%d", set_id, inv);
+	if (!e)
+		return -ENOMEM;
+	nftnl_rule_add_expr(r, e);
+
+	return 0;
+}
+
+static int add_nft_among(struct nft_handle *h,
+			 struct nftnl_rule *r, struct xt_entry_match *m)
+{
+	struct nft_among_data *data = (struct nft_among_data *)m->data;
+	const char *table = nftnl_rule_get(r, NFTNL_RULE_TABLE);
+
+	if ((data->src.cnt && data->src.ip) ||
+	    (data->dst.cnt && data->dst.ip)) {
+		uint16_t eth_p_ip = htons(ETH_P_IP);
+
+		add_meta(r, NFT_META_PROTOCOL);
+		add_cmp_ptr(r, NFT_CMP_EQ, &eth_p_ip, 2);
+	}
+
+	if (data->src.cnt)
+		__add_nft_among(h, table, r, data->pairs, data->src.cnt,
+				false, data->src.inv, data->src.ip);
+	if (data->dst.cnt)
+		__add_nft_among(h, table, r, data->pairs + data->src.cnt,
+				data->dst.cnt, true, data->dst.inv,
+				data->dst.ip);
+	return 0;
+}
+
 int add_match(struct nft_handle *h,
 	      struct nftnl_rule *r, struct xt_entry_match *m)
 {
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c
index 121ecbecd0b64..7c572b80aadf9 100644
--- a/iptables/xtables-eb.c
+++ b/iptables/xtables-eb.c
@@ -593,6 +593,7 @@ void ebt_load_match_extensions(void)
 	ebt_load_match("pkttype");
 	ebt_load_match("vlan");
 	ebt_load_match("stp");
+	ebt_load_match("among");
 
 	ebt_load_watcher("log");
 	ebt_load_watcher("nflog");
-- 
2.22.0

Re: [iptables PATCH 14/14] nft: bridge: Rudimental among extension support

[Pablo Neira Ayuso]

On Wed, Aug 21, 2019 at 11:26:02AM +0200, Phil Sutter wrote:
[...]
> +/* XXX: move this into libnftnl, replacing nftnl_set_lookup() */
> +static struct nftnl_set *nft_set_byname(struct nft_handle *h,
> +					const char *table, const char *set)

Probably extend libnftnl to allow to take a pointer to a nftnl_set
object, as an alternative to the set name? The idea is that this
set object now belongs to the lookup extension, so this extension will
take care of releasing it from the destroy path.

Then, the lookup extension will have a pointer to the anonymous set so
you could then skip the cache code (and all the updates to have access
to it).

Anonymous sets can only be attached to one rule and they go away when
the rule is released. Then, flushing the rule would also release this
object.

Re: [iptables PATCH 14/14] nft: bridge: Rudimental among extension support

[Phil Sutter]

On Sat, Aug 24, 2019 at 06:53:34PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Aug 21, 2019 at 11:26:02AM +0200, Phil Sutter wrote:
> [...]
> > +/* XXX: move this into libnftnl, replacing nftnl_set_lookup() */
> > +static struct nftnl_set *nft_set_byname(struct nft_handle *h,
> > +					const char *table, const char *set)
> 
> Probably extend libnftnl to allow to take a pointer to a nftnl_set
> object, as an alternative to the set name? The idea is that this
> set object now belongs to the lookup extension, so this extension will
> take care of releasing it from the destroy path.
> 
> Then, the lookup extension will have a pointer to the anonymous set so
> you could then skip the cache code (and all the updates to have access
> to it).

Sounds like a nice approach! So I would add a new
NFTNL_EXPR_LOOKUP_SET_PTR to link the set and introduce
NFTA_LOOKUP_ANON_SET (or so) which starts a nested attribute filled
simply by nftnl_set_nlmsg_build_payload()? Kernel code would have to be
extended accordingly, of course.

Seems like I can't reuse nftnl_set_nlmsg_parse() since
mnl_attr_parse_nested() would have to be called. But I guess outsourcing
the attribute handling from the further and introducing a second wrapper
would do.

Also, this would limit ebtables-nft among match to kernels supporting
this new way of anon set creating.

> Anonymous sets can only be attached to one rule and they go away when
> the rule is released. Then, flushing the rule would also release this
> object.

Luckily, in kernel space it seems like anonymous sets are released
automatically if the referencing rule is removed.

Cheers, Phil

Re: [iptables PATCH 14/14] nft: bridge: Rudimental among extension support

[Pablo Neira Ayuso]

On Mon, Aug 26, 2019 at 05:40:06PM +0200, Phil Sutter wrote:
> On Sat, Aug 24, 2019 at 06:53:34PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Aug 21, 2019 at 11:26:02AM +0200, Phil Sutter wrote:
> > [...]
> > > +/* XXX: move this into libnftnl, replacing nftnl_set_lookup() */
> > > +static struct nftnl_set *nft_set_byname(struct nft_handle *h,
> > > +					const char *table, const char *set)
> > 
> > Probably extend libnftnl to allow to take a pointer to a nftnl_set
> > object, as an alternative to the set name? The idea is that this
> > set object now belongs to the lookup extension, so this extension will
> > take care of releasing it from the destroy path.
> > 
> > Then, the lookup extension will have a pointer to the anonymous set so
> > you could then skip the cache code (and all the updates to have access
> > to it).
> 
> Sounds like a nice approach! So I would add a new
> NFTNL_EXPR_LOOKUP_SET_PTR to link the set and introduce
> NFTA_LOOKUP_ANON_SET (or so) which starts a nested attribute filled
> simply by nftnl_set_nlmsg_build_payload()? Kernel code would have to be
> extended accordingly, of course.

No need for kernel code update.

> Seems like I can't reuse nftnl_set_nlmsg_parse() since
> mnl_attr_parse_nested() would have to be called. But I guess outsourcing
> the attribute handling from the further and introducing a second wrapper
> would do.

My proposal is to add NFTNL_EXPR_LOOKUP_SET_PTR, that allows you to
pass a pointer to the set, if that makes this simpler for you.

This would be an alias of the SET_ID, from the build path it would
use. You would still need to add the set command.

But I think you will end up needing the set cache anyway from the
netlink dump path anyway.

I'm re-evaluating, and I think your patchset is a good approach.

Re: [iptables PATCH 14/14] nft: bridge: Rudimental among extension support

[Pablo Neira Ayuso]

On Wed, Aug 21, 2019 at 11:26:02AM +0200, Phil Sutter wrote:
[...]
> +/* Make sure previous payload expression(s) is/are consistent and extract if
> + * matching on source or destination address and if matching on MAC and IP or
> + * only MAC address. */
> +static int lookup_analyze_payloads(const struct nft_xt_ctx *ctx,
> +				   bool *dst, bool *ip)
> +{
> +	int val, val2 = -1;
> +
> +	if (ctx->flags & NFT_XT_CTX_PREV_PAYLOAD) {

Can you probably achieve this by storing protocol context?

Something like storing the current network base in the nft_xt_ctx
structure, rather than the last payload that you have seen.

From the context you annotate, then among will find the information
that it needs in the context.

We can reuse this context later on to generate native tcp/udp/etc.
matching.

[...]
> +static int __add_nft_among(struct nft_handle *h, const char *table,
> +			   struct nftnl_rule *r, struct nft_among_pair *pairs,
> +			   int cnt, bool dst, bool inv, bool ip)
> +{
> +	uint32_t set_id, type = 9, len = 6;
> +	/*			!dst, dst */
> +	int eth_addr_off[] = { 6, 0 };
> +	int ip_addr_off[] = { 12, 16 };
> +	struct nftnl_expr *e;
> +	struct nftnl_set *s;
> +	int idx = 0;
> +
> +	if (ip) {
> +		type = type << 6 | 7;
> +		len += 4 + 2;
> +	}

Magic numbers, please help me understand this.

I think this is the way to go, let's just sort out these few glitches.

Re: [iptables PATCH 14/14] nft: bridge: Rudimental among extension support

[Phil Sutter]

Hi Pablo,

On Tue, Aug 27, 2019 at 12:49:19PM +0200, Pablo Neira Ayuso wrote:
> On Wed, Aug 21, 2019 at 11:26:02AM +0200, Phil Sutter wrote:
> [...]
> > +/* Make sure previous payload expression(s) is/are consistent and extract if
> > + * matching on source or destination address and if matching on MAC and IP or
> > + * only MAC address. */
> > +static int lookup_analyze_payloads(const struct nft_xt_ctx *ctx,
> > +				   bool *dst, bool *ip)
> > +{
> > +	int val, val2 = -1;
> > +
> > +	if (ctx->flags & NFT_XT_CTX_PREV_PAYLOAD) {
> 
> Can you probably achieve this by storing protocol context?
> 
> Something like storing the current network base in the nft_xt_ctx
> structure, rather than the last payload that you have seen.
> 
> From the context you annotate, then among will find the information
> that it needs in the context.
> 
> We can reuse this context later on to generate native tcp/udp/etc.
> matching.

Sorry, I don't understand your approach. With protocol context as it is
used in nftables in mind, I don't see how that applies here. For among
match, we simply have a payload match for MAC address and optionally a
second one for IP address. These are not related apart from the fact
that among allows to match only source or only destination addresses.
The problem lookup_analyze_payloads() solves is:

1) Are we matching MAC only or MAC and IP?
2) Are we matching source or destination?
3) Is everything consistent, i.e., no IP match without MAC one and no
   mixed source/destination matches?

If (3) evaluates false, there may be a different extension this lookups
suits for, but currently such a lookup is simply ignored.

> [...]
> > +static int __add_nft_among(struct nft_handle *h, const char *table,
> > +			   struct nftnl_rule *r, struct nft_among_pair *pairs,
> > +			   int cnt, bool dst, bool inv, bool ip)
> > +{
> > +	uint32_t set_id, type = 9, len = 6;
> > +	/*			!dst, dst */
> > +	int eth_addr_off[] = { 6, 0 };
> > +	int ip_addr_off[] = { 12, 16 };
> > +	struct nftnl_expr *e;
> > +	struct nftnl_set *s;
> > +	int idx = 0;
> > +
> > +	if (ip) {
> > +		type = type << 6 | 7;
> > +		len += 4 + 2;
> > +	}
> 
> Magic numbers, please help me understand this.

Ah, sorry. The 'type' values are TYPE_LLADDR and TYPE_IPADDR from
nftables' enum datatypes. Seems like neither kernel nor libnftnl care
about it, so this is useful only to make nft list things correctly.

Values added to 'len' are four bytes IPv4 address length and two bytes
padding. I'll try to find more illustrative ways to write them.

> I think this is the way to go, let's just sort out these few glitches.

OK, cool. I started implementing the inline anonymous set idea already,
but kernel code becomes pretty ugly when trying to create a new set from
within expr_ops->init. :(

Thanks, Phil

Re: [iptables PATCH 14/14] nft: bridge: Rudimental among extension support

[Pablo Neira Ayuso]

On Tue, Aug 27, 2019 at 01:35:26PM +0200, Phil Sutter wrote:
> Hi Pablo,
> 
> On Tue, Aug 27, 2019 at 12:49:19PM +0200, Pablo Neira Ayuso wrote:
> > On Wed, Aug 21, 2019 at 11:26:02AM +0200, Phil Sutter wrote:
> > [...]
> > > +/* Make sure previous payload expression(s) is/are consistent and extract if
> > > + * matching on source or destination address and if matching on MAC and IP or
> > > + * only MAC address. */
> > > +static int lookup_analyze_payloads(const struct nft_xt_ctx *ctx,
> > > +				   bool *dst, bool *ip)
> > > +{
> > > +	int val, val2 = -1;
> > > +
> > > +	if (ctx->flags & NFT_XT_CTX_PREV_PAYLOAD) {
> > 
> > Can you probably achieve this by storing protocol context?
> > 
> > Something like storing the current network base in the nft_xt_ctx
> > structure, rather than the last payload that you have seen.
> > 
> > From the context you annotate, then among will find the information
> > that it needs in the context.
> > 
> > We can reuse this context later on to generate native tcp/udp/etc.
> > matching.
> 
> Sorry, I don't understand your approach. With protocol context as it is
> used in nftables in mind, I don't see how that applies here. For among
> match, we simply have a payload match for MAC address and optionally a
> second one for IP address. These are not related apart from the fact
> that among allows to match only source or only destination addresses.
> The problem lookup_analyze_payloads() solves is:
> 
> 1) Are we matching MAC only or MAC and IP?
> 2) Are we matching source or destination?
> 3) Is everything consistent, i.e., no IP match without MAC one and no
>    mixed source/destination matches?

Could you store something like a ebt_among_flags field in nft_xt_ct
object that the among match can use to check for the dependencies via
flags?

> If (3) evaluates false, there may be a different extension this lookups
> suits for, but currently such a lookup is simply ignored.
> 
> > [...]
> > > +static int __add_nft_among(struct nft_handle *h, const char *table,
> > > +			   struct nftnl_rule *r, struct nft_among_pair *pairs,
> > > +			   int cnt, bool dst, bool inv, bool ip)
> > > +{
> > > +	uint32_t set_id, type = 9, len = 6;
> > > +	/*			!dst, dst */
> > > +	int eth_addr_off[] = { 6, 0 };
> > > +	int ip_addr_off[] = { 12, 16 };
> > > +	struct nftnl_expr *e;
> > > +	struct nftnl_set *s;
> > > +	int idx = 0;
> > > +
> > > +	if (ip) {
> > > +		type = type << 6 | 7;
> > > +		len += 4 + 2;
> > > +	}
> > 
> > Magic numbers, please help me understand this.
> 
> Ah, sorry. The 'type' values are TYPE_LLADDR and TYPE_IPADDR from
> nftables' enum datatypes. Seems like neither kernel nor libnftnl care
> about it, so this is useful only to make nft list things correctly.

Probably good if we make these public through libnftnl. Just like we
made for udata definitions.

> Values added to 'len' are four bytes IPv4 address length and two bytes
> padding. I'll try to find more illustrative ways to write them.
> 
> > I think this is the way to go, let's just sort out these few glitches.
> 
> OK, cool. I started implementing the inline anonymous set idea already,
> but kernel code becomes pretty ugly when trying to create a new set from
> within expr_ops->init. :(

Not my intention that you update kernel. I was just wondering if a
NFT_LOOKUP_SET_PTR that becomes an alias of NFT_LOOKUP_SET_ID (but
that takes the set pointer as input) would make things easier for you.
Also, this could use it to fetch the set via nftnl_expr_get() once
attached, so you don't need to make cache lookups. Still the cache
lookup would be needed anyway for the netlink dump path though, so not
sure if this would really simplify things there.

Re: [iptables PATCH 14/14] nft: bridge: Rudimental among extension support

[Pablo Neira Ayuso]

On Tue, Aug 27, 2019 at 02:21:11PM +0200, Pablo Neira Ayuso wrote:
> On Tue, Aug 27, 2019 at 01:35:26PM +0200, Phil Sutter wrote:
> > Hi Pablo,
> > 
> > On Tue, Aug 27, 2019 at 12:49:19PM +0200, Pablo Neira Ayuso wrote:
> > > On Wed, Aug 21, 2019 at 11:26:02AM +0200, Phil Sutter wrote:
> > > [...]
> > > > +/* Make sure previous payload expression(s) is/are consistent and extract if
> > > > + * matching on source or destination address and if matching on MAC and IP or
> > > > + * only MAC address. */
> > > > +static int lookup_analyze_payloads(const struct nft_xt_ctx *ctx,
> > > > +				   bool *dst, bool *ip)
> > > > +{
> > > > +	int val, val2 = -1;
> > > > +
> > > > +	if (ctx->flags & NFT_XT_CTX_PREV_PAYLOAD) {
> > > 
> > > Can you probably achieve this by storing protocol context?
> > > 
> > > Something like storing the current network base in the nft_xt_ctx
> > > structure, rather than the last payload that you have seen.
> > > 
> > > From the context you annotate, then among will find the information
> > > that it needs in the context.
> > > 
> > > We can reuse this context later on to generate native tcp/udp/etc.
> > > matching.
> > 
> > Sorry, I don't understand your approach. With protocol context as it is
> > used in nftables in mind, I don't see how that applies here. For among
> > match, we simply have a payload match for MAC address and optionally a
> > second one for IP address. These are not related apart from the fact
> > that among allows to match only source or only destination addresses.
> > The problem lookup_analyze_payloads() solves is:
> > 
> > 1) Are we matching MAC only or MAC and IP?
> > 2) Are we matching source or destination?
> > 3) Is everything consistent, i.e., no IP match without MAC one and no
> >    mixed source/destination matches?

Ok, so you are storing the last two payload expressions in the
nft_xt_ctx object. Looks fine to me.

We might need to revisit this when supporting for native payload
matching. The existing context infrastructure might not be enough if
we need to express more complex things. But that can be done later on.