[PATCH nftables 0/9] nftables: add support for wildcard string as set keys

[PATCH nftables 0/9] nftables: add support for wildcard string as set keys

[Florian Westphal]

Allow to match something like

meta iifname { eth0, ppp* }.

Set ranges or concatenations are not yet supported.
Test passes on x86_64 and s390 (bigendian), however, the test fails dump
validation:

-  iifname { "eth0", "abcdef0" } counter packets 0 bytes 0
+  iifname { "abcdef0", "eth0" } counter packets 0 bytes 0

This happens with other tests as well.  Other tests fail
on s390 too but there are no new failures.

I wil try to get string range support working and will
then ook into concat set support.

Florian Westphal (9):
  evaluate: make byteorder conversion on string base type a no-op
  evaluate: keep prefix expression length
  segtree: split prefix and range creation to a helper function
  evaluate: string prefix expression must retain original length
  src: make interval sets work with string datatypes
  segtree: add string "range" reversal support
  tests: add testcases for interface names in sets
  segtree: use correct byte order for 'element get'
  segtree: add support for get element with sets that contain ifnames

 src/evaluate.c                                |  18 +-
 src/expression.c                              |   9 +-
 src/segtree.c                                 | 228 +++++++++++++-----
 .../sets/dumps/sets_with_ifnames.nft          |  28 +++
 tests/shell/testcases/sets/sets_with_ifnames  | 102 ++++++++
 5 files changed, 315 insertions(+), 70 deletions(-)
 create mode 100644 tests/shell/testcases/sets/dumps/sets_with_ifnames.nft
 create mode 100755 tests/shell/testcases/sets/sets_with_ifnames

-- 
2.35.1

[PATCH nftables 1/9] evaluate: make byteorder conversion on string base type a no-op

[Florian Westphal]

Prerequisite for support of interface names in interval sets:
 table inet filter {
	set s {
		type ifname
		flags interval
		elements = { "foo" }
	}
	chain input {
		type filter hook input priority filter; policy accept;
		iifname @s counter
	}
 }

Will yield: "Byteorder mismatch: meta expected big endian, got host endian".
This is because of:

 /* Data for range lookups needs to be in big endian order */
 if (right->set->flags & NFT_SET_INTERVAL &&
   byteorder_conversion(ctx, &rel->left, BYTEORDER_BIG_ENDIAN) < 0)

It doesn't make sense to me to add checks to all callers of
byteorder_conversion(), so treat this similar to EXPR_CONCAT and turn
TYPE_STRING byteorder change into a no-op.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index 6b3b63662411..d5ae071add1f 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -138,6 +138,7 @@ static enum ops byteorder_conversion_op(struct expr *expr,
 static int byteorder_conversion(struct eval_ctx *ctx, struct expr **expr,
 				enum byteorder byteorder)
 {
+	enum datatypes basetype;
 	enum ops op;
 
 	assert(!expr_is_constant(*expr) || expr_is_singleton(*expr));
@@ -149,11 +150,19 @@ static int byteorder_conversion(struct eval_ctx *ctx, struct expr **expr,
 	if ((*expr)->etype == EXPR_CONCAT)
 		return 0;
 
-	if (expr_basetype(*expr)->type != TYPE_INTEGER)
+	basetype = expr_basetype(*expr)->type;
+	switch (basetype) {
+	case TYPE_INTEGER:
+		break;
+	case TYPE_STRING:
+		return 0;
+	default:
 		return expr_error(ctx->msgs, *expr,
-			 	  "Byteorder mismatch: expected %s, got %s",
+				  "Byteorder mismatch: %s expected %s, %s got %s",
 				  byteorder_names[byteorder],
+				  expr_name(*expr),
 				  byteorder_names[(*expr)->byteorder]);
+	}
 
 	if (expr_is_constant(*expr) || (*expr)->len / BITS_PER_BYTE < 2)
 		(*expr)->byteorder = byteorder;
-- 
2.35.1

[PATCH nftables 2/9] evaluate: keep prefix expression length

[Florian Westphal]

Else, range_expr_value_high() will see a 0 length when doing:

mpz_init_bitmask(tmp, expr->len - expr->prefix_len);

This wasn't a problem so far because prefix expressions generated
from "string*" were never passed down to the prefix->range conversion
functions.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c   | 1 +
 src/expression.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/src/evaluate.c b/src/evaluate.c
index d5ae071add1f..a20cc396b33f 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -347,6 +347,7 @@ static int expr_evaluate_string(struct eval_ctx *ctx, struct expr **exprp)
 	datatype_set(prefix, ctx->ectx.dtype);
 	prefix->flags |= EXPR_F_CONSTANT;
 	prefix->byteorder = BYTEORDER_HOST_ENDIAN;
+	prefix->len = expr->len;
 
 	expr_free(expr);
 	*exprp = prefix;
diff --git a/src/expression.c b/src/expression.c
index 9c9a7ced9121..deb649e1847b 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -1465,6 +1465,7 @@ void range_expr_value_high(mpz_t rop, const struct expr *expr)
 		return mpz_set(rop, expr->value);
 	case EXPR_PREFIX:
 		range_expr_value_low(rop, expr->prefix);
+		assert(expr->len >= expr->prefix_len);
 		mpz_init_bitmask(tmp, expr->len - expr->prefix_len);
 		mpz_add(rop, rop, tmp);
 		mpz_clear(tmp);
-- 
2.35.1

[PATCH nftables 3/9] segtree: split prefix and range creation to a helper function

[Florian Westphal]

No functional change intended.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/segtree.c | 95 ++++++++++++++++++++++++++++-----------------------
 1 file changed, 52 insertions(+), 43 deletions(-)

diff --git a/src/segtree.c b/src/segtree.c
index a61ea3d2854a..188cafedce45 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -773,6 +773,22 @@ out:
 	return range;
 }
 
+static struct expr *__expr_to_set_elem(struct expr *low, struct expr *expr)
+{
+	struct expr *elem = set_elem_expr_alloc(&low->location, expr);
+
+	if (low->etype == EXPR_MAPPING) {
+		interval_expr_copy(elem, low->left);
+
+		elem = mapping_expr_alloc(&low->location, elem,
+						    expr_clone(low->right));
+	} else {
+		interval_expr_copy(elem, low);
+	}
+
+	return elem;
+}
+
 int get_set_decompose(struct set *cache_set, struct set *set)
 {
 	struct expr *i, *next, *range;
@@ -980,6 +996,38 @@ next:
 	}
 }
 
+static struct expr *interval_to_prefix(struct expr *low, struct expr *i, const mpz_t range)
+{
+	unsigned int prefix_len;
+	struct expr *prefix;
+
+	prefix_len = expr_value(i)->len - mpz_scan0(range, 0);
+	prefix = prefix_expr_alloc(&low->location,
+				   expr_clone(expr_value(low)),
+						   prefix_len);
+	prefix->len = expr_value(i)->len;
+
+	return __expr_to_set_elem(low, prefix);
+}
+
+static struct expr *interval_to_range(struct expr *low, struct expr *i, mpz_t range)
+{
+	struct expr *tmp;
+
+	tmp = constant_expr_alloc(&low->location, low->dtype,
+				  low->byteorder, expr_value(low)->len,
+				  NULL);
+
+	mpz_add(range, range, expr_value(low)->value);
+	mpz_set(tmp->value, range);
+
+	tmp = range_expr_alloc(&low->location,
+			       expr_clone(expr_value(low)),
+			       tmp);
+
+	return __expr_to_set_elem(low, tmp);
+}
+
 void interval_map_decompose(struct expr *set)
 {
 	struct expr *i, *next, *low = NULL, *end, *catchall = NULL, *key;
@@ -1065,52 +1113,13 @@ void interval_map_decompose(struct expr *set)
 		else if ((!range_is_prefix(range) ||
 			  !(i->dtype->flags & DTYPE_F_PREFIX)) ||
 			 mpz_cmp_ui(p, 0)) {
-			struct expr *tmp;
-
-			tmp = constant_expr_alloc(&low->location, low->dtype,
-						  low->byteorder, expr_value(low)->len,
-						  NULL);
-
-			mpz_add(range, range, expr_value(low)->value);
-			mpz_set(tmp->value, range);
+			struct expr *expr = interval_to_range(low, i, range);
 
-			tmp = range_expr_alloc(&low->location,
-					       expr_clone(expr_value(low)),
-					       tmp);
-			tmp = set_elem_expr_alloc(&low->location, tmp);
-
-			if (low->etype == EXPR_MAPPING) {
-				interval_expr_copy(tmp, low->left);
-
-				tmp = mapping_expr_alloc(&tmp->location, tmp,
-							 expr_clone(low->right));
-			} else {
-				interval_expr_copy(tmp, low);
-			}
-
-			compound_expr_add(set, tmp);
+			compound_expr_add(set, expr);
 		} else {
-			struct expr *prefix;
-			unsigned int prefix_len;
-
-			prefix_len = expr_value(i)->len - mpz_scan0(range, 0);
-			prefix = prefix_expr_alloc(&low->location,
-						   expr_clone(expr_value(low)),
-						   prefix_len);
-			prefix->len = expr_value(i)->len;
-
-			prefix = set_elem_expr_alloc(&low->location, prefix);
-
-			if (low->etype == EXPR_MAPPING) {
-				interval_expr_copy(prefix, low->left);
-
-				prefix = mapping_expr_alloc(&low->location, prefix,
-							    expr_clone(low->right));
-			} else {
-				interval_expr_copy(prefix, low);
-			}
+			struct expr *expr = interval_to_prefix(low, i, range);
 
-			compound_expr_add(set, prefix);
+			compound_expr_add(set, expr);
 		}
 
 		if (i->flags & EXPR_F_INTERVAL_END) {
-- 
2.35.1

[PATCH nftables 4/9] evaluate: string prefix expression must retain original length

[Florian Westphal]

To make something like "eth*" work for interval sets (match
eth0, eth1, and so on...) we must treat the string as a 128 bit
integer.

Without this, segtree will do the wrong thing when applying the prefix,
because we generate the prefix based on 'eth*' as input, with a length of 3.

The correct import needs to be done on "eth\0\0\0\0\0\0\0...", i.e., if
the input buffer were an ipv6 address, it should look like "eth\0::",
not "::eth".

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/evaluate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index a20cc396b33f..788623137e58 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -338,9 +338,11 @@ static int expr_evaluate_string(struct eval_ctx *ctx, struct expr **exprp)
 		*exprp = value;
 		return 0;
 	}
+
+	data[datalen] = 0;
 	value = constant_expr_alloc(&expr->location, ctx->ectx.dtype,
 				    BYTEORDER_HOST_ENDIAN,
-				    datalen * BITS_PER_BYTE, data);
+				    expr->len, data);
 
 	prefix = prefix_expr_alloc(&expr->location, value,
 				   datalen * BITS_PER_BYTE);
-- 
2.35.1

[PATCH nftables 5/9] src: make interval sets work with string datatypes

[Florian Westphal]

Allows to interface names in interval sets:

table inet filter {
        set s {
                type ifname
                flags interval
                elements = { eth*, foo }
        }

Concatenations are not yet supported, also, listing is broken,
those strings will not be printed back because the values will remain
in big-endian order.  Followup patch will extend segtree to translate
this back to host byte order.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/expression.c |  8 ++++++--
 src/segtree.c    | 30 ++++++++++++++++++++++++++----
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/expression.c b/src/expression.c
index deb649e1847b..5d879b535990 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -1442,7 +1442,11 @@ void range_expr_value_low(mpz_t rop, const struct expr *expr)
 {
 	switch (expr->etype) {
 	case EXPR_VALUE:
-		return mpz_set(rop, expr->value);
+		mpz_set(rop, expr->value);
+		if (expr->byteorder == BYTEORDER_HOST_ENDIAN &&
+		    expr_basetype(expr)->type == TYPE_STRING)
+			mpz_switch_byteorder(rop, expr->len / BITS_PER_BYTE);
+		return;
 	case EXPR_PREFIX:
 		return range_expr_value_low(rop, expr->prefix);
 	case EXPR_RANGE:
@@ -1462,7 +1466,7 @@ void range_expr_value_high(mpz_t rop, const struct expr *expr)
 
 	switch (expr->etype) {
 	case EXPR_VALUE:
-		return mpz_set(rop, expr->value);
+		return range_expr_value_low(rop, expr);
 	case EXPR_PREFIX:
 		range_expr_value_low(rop, expr->prefix);
 		assert(expr->len >= expr->prefix_len);
diff --git a/src/segtree.c b/src/segtree.c
index 188cafedce45..b4e76bf530d6 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -70,12 +70,30 @@ struct elementary_interval {
 	struct expr			*expr;
 };
 
+static enum byteorder get_key_byteorder(const struct expr *e)
+{
+	enum datatypes basetype = expr_basetype(e)->type;
+
+	switch (basetype) {
+	case TYPE_INTEGER:
+		/* For ranges, integers MUST be in BYTEORDER_BIG_ENDIAN.
+		 * If the LHS (lookup key, e.g. 'meta mark', is host endian,
+		 * a byteorder expression is injected to convert the register
+		 * content before lookup.
+		 */
+		return BYTEORDER_BIG_ENDIAN;
+	case TYPE_STRING:
+		return BYTEORDER_HOST_ENDIAN;
+	default:
+		break;
+	}
+
+	return BYTEORDER_INVALID;
+}
+
 static void seg_tree_init(struct seg_tree *tree, const struct set *set,
 			  struct expr *init, unsigned int debug_mask)
 {
-	struct expr *first;
-
-	first = list_entry(init->expressions.next, struct expr, list);
 	tree->root	= RB_ROOT;
 	tree->keytype	= set->key->dtype;
 	tree->keylen	= set->key->len;
@@ -85,7 +103,8 @@ static void seg_tree_init(struct seg_tree *tree, const struct set *set,
 		tree->datatype	= set->data->dtype;
 		tree->datalen	= set->data->len;
 	}
-	tree->byteorder	= first->byteorder;
+
+	tree->byteorder = get_key_byteorder(set->key);
 	tree->debug_mask = debug_mask;
 }
 
@@ -608,6 +627,9 @@ static void set_insert_interval(struct expr *set, struct seg_tree *tree,
 	expr = constant_expr_alloc(&internal_location, tree->keytype,
 				   tree->byteorder, tree->keylen, NULL);
 	mpz_set(expr->value, ei->left);
+	if (tree->byteorder == BYTEORDER_HOST_ENDIAN)
+		mpz_switch_byteorder(expr->value, expr->len / BITS_PER_BYTE);
+
 	expr = set_elem_expr_alloc(&internal_location, expr);
 
 	if (ei->expr != NULL) {
-- 
2.35.1

[PATCH nftables 6/9] segtree: add string "range" reversal support

[Florian Westphal]

Previous commits allows to use set key as a range, i.e.

	key ifname
	flags interval
	elements = { eth* }

and then have it match on any interface starting with 'eth'.

Listing is broken however, we need to reverse-translate the (128bit)
number back to a string.

'eth*' is stored as interval
00687465 0000000 ..  00697465 0000000, i.e. "eth-eti",
this adds the needed endianess fixups.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/segtree.c | 47 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 6 deletions(-)

diff --git a/src/segtree.c b/src/segtree.c
index b4e76bf530d6..bed8bbcf0c8e 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -1032,6 +1032,33 @@ static struct expr *interval_to_prefix(struct expr *low, struct expr *i, const m
 	return __expr_to_set_elem(low, prefix);
 }
 
+static struct expr *interval_to_string(struct expr *low, struct expr *i, const mpz_t range)
+{
+	unsigned int len = div_round_up(i->len, BITS_PER_BYTE);
+	unsigned int prefix_len, str_len;
+	char data[len + 2];
+	struct expr *expr;
+
+	prefix_len = expr_value(i)->len - mpz_scan0(range, 0);
+
+	if (prefix_len > i->len || prefix_len % BITS_PER_BYTE)
+		return interval_to_prefix(low, i, range);
+
+	mpz_export_data(data, expr_value(low)->value, BYTEORDER_BIG_ENDIAN, len);
+
+	str_len = strnlen(data, len);
+	if (str_len >= len || str_len == 0)
+		return interval_to_prefix(low, i, range);
+
+	data[str_len] = '*';
+
+	expr = constant_expr_alloc(&low->location, low->dtype,
+				   BYTEORDER_HOST_ENDIAN,
+				   (str_len + 1) * BITS_PER_BYTE, data);
+
+	return __expr_to_set_elem(low, expr);
+}
+
 static struct expr *interval_to_range(struct expr *low, struct expr *i, mpz_t range)
 {
 	struct expr *tmp;
@@ -1130,16 +1157,24 @@ void interval_map_decompose(struct expr *set)
 
 		mpz_and(p, expr_value(low)->value, range);
 
-		if (!mpz_cmp_ui(range, 0))
+		if (!mpz_cmp_ui(range, 0)) {
+			if (expr_basetype(low)->type == TYPE_STRING)
+				mpz_switch_byteorder(expr_value(low)->value, low->len / BITS_PER_BYTE);
+
 			compound_expr_add(set, expr_get(low));
-		else if ((!range_is_prefix(range) ||
-			  !(i->dtype->flags & DTYPE_F_PREFIX)) ||
-			 mpz_cmp_ui(p, 0)) {
-			struct expr *expr = interval_to_range(low, i, range);
+		} else if (range_is_prefix(range) && !mpz_cmp_ui(p, 0)) {
+			struct expr *expr;
+
+			if (i->dtype->flags & DTYPE_F_PREFIX)
+				expr = interval_to_prefix(low, i, range);
+			else if (expr_basetype(i)->type == TYPE_STRING)
+				expr = interval_to_string(low, i, range);
+			else
+				expr = interval_to_range(low, i, range);
 
 			compound_expr_add(set, expr);
 		} else {
-			struct expr *expr = interval_to_prefix(low, i, range);
+			struct expr *expr = interval_to_range(low, i, range);
 
 			compound_expr_add(set, expr);
 		}
-- 
2.35.1

[PATCH nftables 7/9] tests: add testcases for interface names in sets

[Florian Westphal]

Add initial test case, sets with names and interfaces,
anonymous and named ones.

Check match+no-match.
netns with ppp1 and ppq veth, send packets via both interfaces.
Rule counters should have incremented on the three rules.
(that match on set that have "abcdef1" or "abcdef*" strings in them).

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 .../sets/dumps/sets_with_ifnames.nft          | 28 +++++++
 tests/shell/testcases/sets/sets_with_ifnames  | 83 +++++++++++++++++++
 2 files changed, 111 insertions(+)
 create mode 100644 tests/shell/testcases/sets/dumps/sets_with_ifnames.nft
 create mode 100755 tests/shell/testcases/sets/sets_with_ifnames

diff --git a/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft b/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft
new file mode 100644
index 000000000000..12c1aa960a66
--- /dev/null
+++ b/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft
@@ -0,0 +1,28 @@
+table inet testifsets {
+	set simple {
+		type ifname
+		elements = { "abcdef0",
+			     "abcdef1",
+			     "othername" }
+	}
+
+	set simple_wild {
+		type ifname
+		flags interval
+		elements = { "abcdef*",
+			     "othername",
+			     "ppp0" }
+	}
+
+	chain v4icmp {
+		iifname @simple counter packets 0 bytes 0
+		iifname @simple_wild counter packets 0 bytes 0
+		iifname { "eth0", "abcdef0" } counter packets 0 bytes 0
+		iifname { "abcdef*", "eth0" } counter packets 0 bytes 0
+	}
+
+	chain input {
+		type filter hook input priority filter; policy accept;
+		ip protocol icmp goto v4icmp
+	}
+}
diff --git a/tests/shell/testcases/sets/sets_with_ifnames b/tests/shell/testcases/sets/sets_with_ifnames
new file mode 100755
index 000000000000..0f9a6b5b0048
--- /dev/null
+++ b/tests/shell/testcases/sets/sets_with_ifnames
@@ -0,0 +1,83 @@
+#!/bin/bash
+
+dumpfile=$(dirname $0)/dumps/$(basename $0).nft
+
+[ -z "$NFT" ] && exit 111
+
+$NFT -f "$dumpfile" || exit 1
+
+rnd=$(mktemp -u XXXXXXXX)
+ns1="nft1ifname-$rnd"
+ns2="nft2ifname-$rnd"
+
+cleanup()
+{
+	ip netns del "$ns1"
+}
+
+trap cleanup EXIT
+
+check_elem()
+{
+	setname=$1
+	ifname=$2
+	fail=$3
+
+	if [ $fail -eq 1 ]; then
+		ip netns exec "$ns1" $NFT get element inet testifsets $setname { "$ifname" } && exit 2
+	else
+		ip netns exec "$ns1" $NFT get element inet testifsets $setname { "$ifname" } || exit 3
+	fi
+}
+
+# send pings, check all rules with sets that contain abcdef1 match.
+# there are 4 rules in this chain, 4 should match.
+check_matching_icmp_ppp()
+{
+	pkt=$((RANDOM%10))
+	pkt=$((pkt+1))
+	ip netns exec "$ns1" ping -f -c $pkt 10.1.2.2
+
+	# replies should arrive via 'abcdeg', so, should NOT increment any counters.
+	ip netns exec "$ns1" ping -f -c 100 10.2.2.2
+
+	matches=$(ip netns exec "$ns1" $NFT list chain inet testifsets v4icmp | grep "counter packets $pkt " | wc -l)
+	want=3
+
+	if [ "$matches" -ne $want ] ;then
+		echo "Excpected $matches matching rules, got $want, packets $pkt"
+		ip netns exec "$ns1" $NFT list ruleset
+		exit 1
+	fi
+}
+
+ip netns add "$ns1" || exit 111
+ip netns add "$ns2" || exit 111
+ip netns exec "$ns1" $NFT -f "$dumpfile" || exit 3
+
+for n in abcdef0 abcdef1 othername;do
+	check_elem simple $n 0
+done
+
+check_elem simple foo 1
+
+set -e
+ip -net "$ns1" link set lo up
+ip -net "$ns2" link set lo up
+ip netns exec "$ns1" ping -f -c 10 127.0.0.1
+
+ip link add abcdef1 netns $ns1 type veth peer name veth0 netns $ns2
+ip link add abcdeg  netns $ns1 type veth peer name veth1 netns $ns2
+
+ip -net "$ns1" link set abcdef1 up
+ip -net "$ns2" link set veth0 up
+ip -net "$ns1" link set abcdeg up
+ip -net "$ns2" link set veth1 up
+
+ip -net "$ns1" addr add 10.1.2.1/24 dev abcdef1
+ip -net "$ns1" addr add 10.2.2.1/24 dev abcdeg
+
+ip -net "$ns2" addr add 10.1.2.2/24 dev veth0
+ip -net "$ns2" addr add 10.2.2.2/24 dev veth1
+
+check_matching_icmp_ppp
-- 
2.35.1

[PATCH nftables 8/9] segtree: use correct byte order for 'element get'

[Florian Westphal]

Fails when the argument / set contains strings: we need to use
host byte order if element has string base type.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/segtree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/segtree.c b/src/segtree.c
index bed8bbcf0c8e..0135a07492b0 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -720,6 +720,7 @@ static void set_elem_add(const struct set *set, struct expr *init, mpz_t value,
 
 struct expr *get_set_intervals(const struct set *set, const struct expr *init)
 {
+	enum byteorder byteorder = get_key_byteorder(set->key);
 	struct expr *new_init;
 	mpz_t low, high;
 	struct expr *i;
@@ -733,7 +734,7 @@ struct expr *get_set_intervals(const struct set *set, const struct expr *init)
 		switch (i->key->etype) {
 		case EXPR_VALUE:
 			set_elem_add(set, new_init, i->key->value,
-				     i->flags, i->byteorder);
+				     i->flags, byteorder);
 			break;
 		case EXPR_CONCAT:
 			compound_expr_add(new_init, expr_clone(i));
-- 
2.35.1

[PATCH nftables 9/9] segtree: add support for get element with sets that contain ifnames

[Florian Westphal]

nft get element inet filter s { bla, prefixfoo }
table inet filter {
        set s {
                type ifname
                flags interval
                elements = { "prefixfoo*",
                             "bla" }
        }

Also add test cases for this.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 src/segtree.c                                | 59 +++++++++++++++-----
 tests/shell/testcases/sets/sets_with_ifnames | 21 ++++++-
 2 files changed, 65 insertions(+), 15 deletions(-)

diff --git a/src/segtree.c b/src/segtree.c
index 0135a07492b0..3ccf5ee129fc 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -774,6 +774,12 @@ static struct expr *get_set_interval_find(const struct set *cache_set,
 
 	list_for_each_entry(i, &set->init->expressions, list) {
 		switch (i->key->etype) {
+		case EXPR_VALUE:
+			if (expr_basetype(i->key)->type != TYPE_STRING)
+				break;
+			/* string type, check if its a range (wildcard), so
+			 * fall through.
+			 */
 		case EXPR_PREFIX:
 		case EXPR_RANGE:
 			range_expr_value_low(val, i);
@@ -796,6 +802,18 @@ out:
 	return range;
 }
 
+static struct expr *expr_value(struct expr *expr)
+{
+	switch (expr->etype) {
+	case EXPR_MAPPING:
+		return expr->left->key;
+	case EXPR_SET_ELEM:
+		return expr->key;
+	default:
+		BUG("invalid expression type %s\n", expr_name(expr));
+	}
+}
+
 static struct expr *__expr_to_set_elem(struct expr *low, struct expr *expr)
 {
 	struct expr *elem = set_elem_expr_alloc(&low->location, expr);
@@ -812,6 +830,31 @@ static struct expr *__expr_to_set_elem(struct expr *low, struct expr *expr)
 	return elem;
 }
 
+static struct expr *expr_to_set_elem(struct expr *e)
+{
+	unsigned int len = div_round_up(e->len, BITS_PER_BYTE);
+	unsigned int str_len;
+	char data[len + 1];
+	struct expr *expr;
+
+	if (expr_basetype(expr_value(e))->type != TYPE_STRING)
+		return expr_clone(e);
+
+	mpz_export_data(data, expr_value(e)->value, BYTEORDER_BIG_ENDIAN, len);
+
+	str_len = strnlen(data, len);
+	if (str_len >= len || str_len == 0)
+		return expr_clone(e);
+
+	data[str_len] = '*';
+
+	expr = constant_expr_alloc(&e->location, e->dtype,
+				   BYTEORDER_HOST_ENDIAN,
+				   (str_len + 1) * BITS_PER_BYTE, data);
+
+	return __expr_to_set_elem(e, expr);
+}
+
 int get_set_decompose(struct set *cache_set, struct set *set)
 {
 	struct expr *i, *next, *range;
@@ -846,7 +889,7 @@ int get_set_decompose(struct set *cache_set, struct set *set)
 					compound_expr_add(new_init, range);
 				else
 					compound_expr_add(new_init,
-							  expr_clone(left));
+							  expr_to_set_elem(left));
 			}
 			left = i;
 		}
@@ -856,7 +899,7 @@ int get_set_decompose(struct set *cache_set, struct set *set)
 		if (range)
 			compound_expr_add(new_init, range);
 		else
-			compound_expr_add(new_init, expr_clone(left));
+			compound_expr_add(new_init, expr_to_set_elem(left));
 	}
 
 	expr_free(set->init);
@@ -878,18 +921,6 @@ static bool range_is_prefix(const mpz_t range)
 	return ret;
 }
 
-static struct expr *expr_value(struct expr *expr)
-{
-	switch (expr->etype) {
-	case EXPR_MAPPING:
-		return expr->left->key;
-	case EXPR_SET_ELEM:
-		return expr->key;
-	default:
-		BUG("invalid expression type %s\n", expr_name(expr));
-	}
-}
-
 static int expr_value_cmp(const void *p1, const void *p2)
 {
 	struct expr *e1 = *(void * const *)p1;
diff --git a/tests/shell/testcases/sets/sets_with_ifnames b/tests/shell/testcases/sets/sets_with_ifnames
index 0f9a6b5b0048..10e6c331bdca 100755
--- a/tests/shell/testcases/sets/sets_with_ifnames
+++ b/tests/shell/testcases/sets/sets_with_ifnames
@@ -22,11 +22,22 @@ check_elem()
 	setname=$1
 	ifname=$2
 	fail=$3
+	result=$4
+
+	if [ -z "$result" ]; then
+		result=$ifname
+	fi
 
 	if [ $fail -eq 1 ]; then
 		ip netns exec "$ns1" $NFT get element inet testifsets $setname { "$ifname" } && exit 2
 	else
-		ip netns exec "$ns1" $NFT get element inet testifsets $setname { "$ifname" } || exit 3
+		result=$(ip netns exec "$ns1" $NFT get element inet testifsets $setname { "$ifname" } | grep "$result" )
+
+		if [ -z "$result" ] ; then
+			echo "empty result, expected $ifname"
+			ip netns exec "$ns1" $NFT get element inet testifsets $setname { "$ifname" }
+			exit 1
+		fi
 	fi
 }
 
@@ -61,6 +72,14 @@ done
 
 check_elem simple foo 1
 
+for n in ppp0 othername;do
+	check_elem simple_wild $n 0
+done
+
+check_elem simple_wild enoent 1
+check_elem simple_wild ppp0 0
+check_elem simple_wild abcdefghijk 0 'abcdef\*'
+
 set -e
 ip -net "$ns1" link set lo up
 ip -net "$ns2" link set lo up
-- 
2.35.1

Re: [PATCH nftables 0/9] nftables: add support for wildcard string as set keys

[Pablo Neira Ayuso]

On Sat, Apr 09, 2022 at 03:58:23PM +0200, Florian Westphal wrote:
> Allow to match something like
> 
> meta iifname { eth0, ppp* }.

This series LGTM, thanks for working on this.

> Set ranges or concatenations are not yet supported.
> Test passes on x86_64 and s390 (bigendian), however, the test fails dump
> validation:
> 
> -  iifname { "eth0", "abcdef0" } counter packets 0 bytes 0
> +  iifname { "abcdef0", "eth0" } counter packets 0 bytes 0

Hm. Is it reordering the listing?

> This happens with other tests as well.  Other tests fail
> on s390 too but there are no new failures.
> 
> I wil try to get string range support working and will
> then ook into concat set support.

OK, so then this is a WIP?

Re: [PATCH nftables 0/9] nftables: add support for wildcard string as set keys

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Sat, Apr 09, 2022 at 03:58:23PM +0200, Florian Westphal wrote:
> > Allow to match something like
> > 
> > meta iifname { eth0, ppp* }.
> 
> This series LGTM, thanks for working on this.
> 
> > Set ranges or concatenations are not yet supported.
> > Test passes on x86_64 and s390 (bigendian), however, the test fails dump
> > validation:
> > 
> > -  iifname { "eth0", "abcdef0" } counter packets 0 bytes 0
> > +  iifname { "abcdef0", "eth0" } counter packets 0 bytes 0
> 
> Hm. Is it reordering the listing?

Yes, but its like this also before my patch, there are several
test failures on s390 with nft master.

I will have a look, so far I only checked that my patch
series does not cause any additional test failures, and the only
reason why the new test fails is the output reorder on s390.

> > I wil try to get string range support working and will
> > then ook into concat set support.
> 
> OK, so then this is a WIP?

If you want all at once then yes, but do you think thats needed?

I have not looked at EXPR_RANGE or concat-with-wildcard yet and
I don't know when I will be able to do so.

Re: [PATCH nftables 0/9] nftables: add support for wildcard string as set keys

[Pablo Neira Ayuso]

On Wed, Apr 13, 2022 at 12:43:35AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > On Sat, Apr 09, 2022 at 03:58:23PM +0200, Florian Westphal wrote:
> > > Allow to match something like
> > > 
> > > meta iifname { eth0, ppp* }.
> > 
> > This series LGTM, thanks for working on this.
> > 
> > > Set ranges or concatenations are not yet supported.
> > > Test passes on x86_64 and s390 (bigendian), however, the test fails dump
> > > validation:
> > > 
> > > -  iifname { "eth0", "abcdef0" } counter packets 0 bytes 0
> > > +  iifname { "abcdef0", "eth0" } counter packets 0 bytes 0
> > 
> > Hm. Is it reordering the listing?
> 
> Yes, but its like this also before my patch, there are several
> test failures on s390 with nft master.

Why is the listing being reordered?

> I will have a look, so far I only checked that my patch
> series does not cause any additional test failures, and the only
> reason why the new test fails is the output reorder on s390.

This is also related to the set description patchset that Phil posted,
correct?

> > > I wil try to get string range support working and will
> > > then ook into concat set support.
> >
> > OK, so then this is a WIP?
> 
> If you want all at once then yes, but do you think thats needed?

If you consider that adding remaining features is feasible,
incrementally should be fine.

> I have not looked at EXPR_RANGE or concat-with-wildcard yet and
> I don't know when I will be able to do so.

Re: [PATCH nftables 0/9] nftables: add support for wildcard string as set keys

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > Yes, but its like this also before my patch, there are several
> > test failures on s390 with nft master.
> 
> Why is the listing being reordered?

No idea, I only saw that this reordering happens, i did not have time to
investigate so far.

> > I will have a look, so far I only checked that my patch
> > series does not cause any additional test failures, and the only
> > reason why the new test fails is the output reorder on s390.
> 
> This is also related to the set description patchset that Phil posted,
> correct?

No, I don't even know what patchset you are talking about.
Is it because of failing pything tests because the debug output has
endianess issues?  If so, not related.

> If you consider that adding remaining features is feasible,
> incrementally should be fine.

Hmm, if there is a technical reason as to why it does not work,
do you think we should hold it back?

It lookes like filter on "{ eth0, ppp* }" works fine as-is.

I thought that something like "eth0-eth42" would also be doable,
by treating both as 128bit bit-string.

Don't see what prevents "ppp* . 80" from working from a technical pov.

So, I *think* its fine to add the pure ifname set support now and
add the rest incrementally.

Re: [PATCH nftables 0/9] nftables: add support for wildcard string as set keys

[Pablo Neira Ayuso]

On Wed, Apr 13, 2022 at 01:30:23AM +0200, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > Yes, but its like this also before my patch, there are several
> > > test failures on s390 with nft master.
> > 
> > Why is the listing being reordered?
> 
> No idea, I only saw that this reordering happens, i did not have time to
> investigate so far.
> 
> > > I will have a look, so far I only checked that my patch
> > > series does not cause any additional test failures, and the only
> > > reason why the new test fails is the output reorder on s390.
> > 
> > This is also related to the set description patchset that Phil posted,
> > correct?
> 
> No, I don't even know what patchset you are talking about.
> Is it because of failing pything tests because the debug output has
> endianess issues?  If so, not related.
> 
> > If you consider that adding remaining features is feasible,
> > incrementally should be fine.
> 
> Hmm, if there is a technical reason as to why it does not work,
> do you think we should hold it back?
> 
> It lookes like filter on "{ eth0, ppp* }" works fine as-is.

Then, good. Better than not working.

> I thought that something like "eth0-eth42" would also be doable,
> by treating both as 128bit bit-string.
> 
> Don't see what prevents "ppp* . 80" from working from a technical pov.
> 
> So, I *think* its fine to add the pure ifname set support now and
> add the rest incrementally.

OK, then move on.

Sorry, I read you cover letter and I thought there were pending
issues.

Re: [PATCH nftables 5/9] src: make interval sets work with string datatypes

[Pablo Neira Ayuso]

On Sat, Apr 09, 2022 at 03:58:28PM +0200, Florian Westphal wrote:
> Allows to interface names in interval sets:
> 
> table inet filter {
>         set s {
>                 type ifname
>                 flags interval
>                 elements = { eth*, foo }
>         }
> 
> Concatenations are not yet supported, also, listing is broken,
> those strings will not be printed back because the values will remain
> in big-endian order.  Followup patch will extend segtree to translate
> this back to host byte order.

I think this is the only patch that will clash with the new
src/intervals.c infrastructure (set automerge deletion):

https://patchwork.ozlabs.org/project/netfilter-devel/list/?series=294779

Most of the segtree.c code, including seg_tree_init() is gone after it.

I can have a look and rebase after you push this.

> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  src/expression.c |  8 ++++++--
>  src/segtree.c    | 30 ++++++++++++++++++++++++++----
>  2 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/src/expression.c b/src/expression.c
> index deb649e1847b..5d879b535990 100644
> --- a/src/expression.c
> +++ b/src/expression.c
> @@ -1442,7 +1442,11 @@ void range_expr_value_low(mpz_t rop, const struct expr *expr)
>  {
>  	switch (expr->etype) {
>  	case EXPR_VALUE:
> -		return mpz_set(rop, expr->value);
> +		mpz_set(rop, expr->value);
> +		if (expr->byteorder == BYTEORDER_HOST_ENDIAN &&
> +		    expr_basetype(expr)->type == TYPE_STRING)
> +			mpz_switch_byteorder(rop, expr->len / BITS_PER_BYTE);
> +		return;
>  	case EXPR_PREFIX:
>  		return range_expr_value_low(rop, expr->prefix);
>  	case EXPR_RANGE:
> @@ -1462,7 +1466,7 @@ void range_expr_value_high(mpz_t rop, const struct expr *expr)
>  
>  	switch (expr->etype) {
>  	case EXPR_VALUE:
> -		return mpz_set(rop, expr->value);
> +		return range_expr_value_low(rop, expr);
>  	case EXPR_PREFIX:
>  		range_expr_value_low(rop, expr->prefix);
>  		assert(expr->len >= expr->prefix_len);
> diff --git a/src/segtree.c b/src/segtree.c
> index 188cafedce45..b4e76bf530d6 100644
> --- a/src/segtree.c
> +++ b/src/segtree.c
> @@ -70,12 +70,30 @@ struct elementary_interval {
>  	struct expr			*expr;
>  };
>  
> +static enum byteorder get_key_byteorder(const struct expr *e)
> +{
> +	enum datatypes basetype = expr_basetype(e)->type;
> +
> +	switch (basetype) {
> +	case TYPE_INTEGER:
> +		/* For ranges, integers MUST be in BYTEORDER_BIG_ENDIAN.
> +		 * If the LHS (lookup key, e.g. 'meta mark', is host endian,
> +		 * a byteorder expression is injected to convert the register
> +		 * content before lookup.
> +		 */
> +		return BYTEORDER_BIG_ENDIAN;
> +	case TYPE_STRING:
> +		return BYTEORDER_HOST_ENDIAN;
> +	default:
> +		break;
> +	}
> +
> +	return BYTEORDER_INVALID;
> +}
> +
>  static void seg_tree_init(struct seg_tree *tree, const struct set *set,
>  			  struct expr *init, unsigned int debug_mask)
>  {
> -	struct expr *first;
> -
> -	first = list_entry(init->expressions.next, struct expr, list);
>  	tree->root	= RB_ROOT;
>  	tree->keytype	= set->key->dtype;
>  	tree->keylen	= set->key->len;
> @@ -85,7 +103,8 @@ static void seg_tree_init(struct seg_tree *tree, const struct set *set,
>  		tree->datatype	= set->data->dtype;
>  		tree->datalen	= set->data->len;
>  	}
> -	tree->byteorder	= first->byteorder;
> +
> +	tree->byteorder = get_key_byteorder(set->key);
>  	tree->debug_mask = debug_mask;
>  }
>  
> @@ -608,6 +627,9 @@ static void set_insert_interval(struct expr *set, struct seg_tree *tree,
>  	expr = constant_expr_alloc(&internal_location, tree->keytype,
>  				   tree->byteorder, tree->keylen, NULL);
>  	mpz_set(expr->value, ei->left);
> +	if (tree->byteorder == BYTEORDER_HOST_ENDIAN)
> +		mpz_switch_byteorder(expr->value, expr->len / BITS_PER_BYTE);
> +
>  	expr = set_elem_expr_alloc(&internal_location, expr);
>  
>  	if (ei->expr != NULL) {
> -- 
> 2.35.1
> 

Re: [PATCH nftables 0/9] nftables: add support for wildcard string as set keys

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> OK, then move on.
> 
> Sorry, I read you cover letter and I thought there were pending
> issues.

Ok great, thanks!

Sorry for the confusion.  I meant to say that only "key ifname; flags
inerval" will work after this patchset, and that thinks like
"eth0-eth4" or "key ifname . ipv4_addr" do not.

I plan to work on that too at some point. I will let you know when I
start to work on it.