[PATCH nft 0/3] nftables: add support for wildcard interfaces

[PATCH nft 0/3] nftables: add support for wildcard interfaces

[Florian Westphal]

First patch is needed so kernel doesn't think end is before start,
second patch allows to dump "x .foo*" correctly, without this
nft tries to represent the start/end "name" with a range.
This doesn't work well because end range uses \ff padding.

Patch 3 adds tests.

Florian Westphal (3):
  netlink: swap byteorder for host-endian concat data
  segtree: add pretty-print support for wildcard strings in concatenated
    sets
  sets_with_ifnames: add test case for concatenated range

 src/netlink.c                                 |  4 +
 src/segtree.c                                 | 38 +++++++-
 .../sets/dumps/sets_with_ifnames.nft          | 25 ++++-
 tests/shell/testcases/sets/sets_with_ifnames  | 93 ++++++++++++++-----
 4 files changed, 134 insertions(+), 26 deletions(-)

-- 
2.35.1

[PATCH nft 1/3] netlink: swap byteorder for host-endian concat data

[Florian Westphal]

All data must be passed in network byte order, else matching
won't work respectively kernel will reject the interval because
it thinks that start is after end

This is needed to allow use of 'ppp*' in interval sets with
concatenations.

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

diff --git a/src/netlink.c b/src/netlink.c
index 240c937e3ac5..89d864ed046a 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -259,6 +259,10 @@ static int netlink_gen_concat_data_expr(int end, const struct expr *i,
 			mpz_t v;
 
 			mpz_init_bitmask(v, i->len - i->prefix_len);
+
+			if (i->byteorder == BYTEORDER_HOST_ENDIAN)
+				mpz_switch_byteorder(v, i->len / BITS_PER_BYTE);
+
 			mpz_add(v, i->prefix->value, v);
 			count = netlink_export_pad(data, v, i);
 			mpz_clear(v);
-- 
2.35.1

[PATCH nft 2/3] segtree: add pretty-print support for wildcard strings in concatenated sets

[Florian Westphal]

For concat ranges, something like 'ppp*' is translated as a range
from 'ppp\0\0\0...' to 'ppp\ff\ff\ff...'.

In order to display this properly, check for presence of string base
type and convert to symbolic expression, with appended '*' character.

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

diff --git a/src/segtree.c b/src/segtree.c
index 4602becc10e6..f9cac373a5f0 100644
--- a/src/segtree.c
+++ b/src/segtree.c
@@ -345,6 +345,8 @@ void concat_range_aggregate(struct expr *set)
 		list_for_each_entry_safe(r1, r1_next,
 					 &expr_value(start)->expressions,
 					 list) {
+			bool string_type = false;
+
 			mpz_init(range);
 			mpz_init(p);
 
@@ -356,16 +358,48 @@ void concat_range_aggregate(struct expr *set)
 				goto next;
 			}
 
+			if (expr_basetype(r1)->type == TYPE_STRING &&
+			    expr_basetype(r2)->type == TYPE_STRING) {
+				string_type = true;
+				mpz_switch_byteorder(r1->value, r1->len / BITS_PER_BYTE);
+				mpz_switch_byteorder(r2->value, r2->len / BITS_PER_BYTE);
+			}
+
 			mpz_sub(range, r2->value, r1->value);
 			mpz_sub_ui(range, range, 1);
 			mpz_and(p, r1->value, range);
 
 			/* Check if we are forced, or if it's anyway preferable,
-			 * to express the range as two points instead of a
-			 * netmask.
+			 * to express the range as a wildcard string, or two points
+			 * instead of a netmask.
 			 */
 			prefix_len = range_mask_len(r1->value, r2->value,
 						    r1->len);
+			if (string_type) {
+				mpz_switch_byteorder(r1->value, r1->len / BITS_PER_BYTE);
+				mpz_switch_byteorder(r2->value, r2->len / BITS_PER_BYTE);
+			}
+
+			if (prefix_len >= 0 &&
+			    (prefix_len % BITS_PER_BYTE) == 0 &&
+			    string_type) {
+				unsigned int str_len = prefix_len / BITS_PER_BYTE;
+				char data[str_len + 2];
+
+				mpz_export_data(data, r1->value, BYTEORDER_HOST_ENDIAN, str_len);
+				data[str_len] = '*';
+
+				tmp = constant_expr_alloc(&r1->location, r1->dtype,
+							  BYTEORDER_HOST_ENDIAN,
+							  (str_len + 1) * BITS_PER_BYTE, data);
+				tmp->len = r2->len;
+				list_replace(&r2->list, &tmp->list);
+				r2_next = tmp->list.next;
+				expr_free(r2);
+				free_r1 = 1;
+				goto next;
+			}
+
 			if (prefix_len < 0 ||
 			    !(r1->dtype->flags & DTYPE_F_PREFIX)) {
 				tmp = range_expr_alloc(&r1->location, r1,
-- 
2.35.1

[PATCH nft 3/3] sets_with_ifnames: add test case for concatenated range

[Florian Westphal]

Refactor existing test case for simple interace name ranges
(without concatenations) to also cover "addr . ifname".

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 .../sets/dumps/sets_with_ifnames.nft          | 25 ++++-
 tests/shell/testcases/sets/sets_with_ifnames  | 93 ++++++++++++++-----
 2 files changed, 94 insertions(+), 24 deletions(-)

diff --git a/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft b/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft
index 12c1aa960a66..6b073ae2d090 100644
--- a/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft
+++ b/tests/shell/testcases/sets/dumps/sets_with_ifnames.nft
@@ -14,6 +14,21 @@ table inet testifsets {
 			     "ppp0" }
 	}
 
+	set concat {
+		type ipv4_addr . ifname
+		elements = { 10.1.2.2 . "abcdef0",
+			     10.1.2.2 . "abcdef1" }
+	}
+
+	set concat_wild {
+		type ipv4_addr . ifname
+		flags interval
+		elements = { 10.1.2.2 . "abcdef*",
+			     10.1.2.1 . "bar",
+			     1.1.2.0/24 . "abcdef0",
+			     12.2.2.0/24 . "abcdef*" }
+	}
+
 	chain v4icmp {
 		iifname @simple counter packets 0 bytes 0
 		iifname @simple_wild counter packets 0 bytes 0
@@ -21,8 +36,16 @@ table inet testifsets {
 		iifname { "abcdef*", "eth0" } counter packets 0 bytes 0
 	}
 
+	chain v4icmpc {
+		ip saddr . iifname @concat counter packets 0 bytes 0
+		ip saddr . iifname @concat_wild counter packets 0 bytes 0
+		ip saddr . iifname { 10.1.2.2 . "abcdef0" } counter packets 0 bytes 0
+		ip saddr . iifname { 10.1.2.2 . "abcdef*" } counter packets 0 bytes 0
+	}
+
 	chain input {
 		type filter hook input priority filter; policy accept;
-		ip protocol icmp goto v4icmp
+		ip protocol icmp jump v4icmp
+		ip protocol icmp goto v4icmpc
 	}
 }
diff --git a/tests/shell/testcases/sets/sets_with_ifnames b/tests/shell/testcases/sets/sets_with_ifnames
index 10e6c331bdca..f4ef4db59f51 100755
--- a/tests/shell/testcases/sets/sets_with_ifnames
+++ b/tests/shell/testcases/sets/sets_with_ifnames
@@ -17,28 +17,58 @@ cleanup()
 
 trap cleanup EXIT
 
-check_elem()
+# check a given element is (not) present in the set.
+lookup_elem()
 {
-	setname=$1
-	ifname=$2
-	fail=$3
-	result=$4
+	local setname=$1
+	local value=$2
+	local fail=$3
+	local expect_result=$4
+	local msg=$5
+
+	result=$(ip netns exec "$ns1" $NFT get element inet testifsets $setname { "$value" } 2>/dev/null | grep "$expect_result" )
 
-	if [ -z "$result" ]; then
-		result=$ifname
+	if [ -z "$result" ] && [ $fail -ne 1 ] ; then
+		echo "empty result, expected $expect_result $msg"
+		ip netns exec "$ns1" $NFT get element inet testifsets $setname { "$value" }
+		exit 1
 	fi
+}
 
-	if [ $fail -eq 1 ]; then
-		ip netns exec "$ns1" $NFT get element inet testifsets $setname { "$ifname" } && exit 2
-	else
-		result=$(ip netns exec "$ns1" $NFT get element inet testifsets $setname { "$ifname" } | grep "$result" )
+check_elem_get()
+{
+	local setname=$1
+	local value=$2
+	local fail=$3
+	local expect_result=$4
+
+	# when query is 'abcde', and set has 'abc*', result is
+	# 'abc*', not 'abcde', so returned element can be different.
+	if [ -z "$expect_result" ]; then
+		expect_result=$ifname
+	fi
 
-		if [ -z "$result" ] ; then
-			echo "empty result, expected $ifname"
-			ip netns exec "$ns1" $NFT get element inet testifsets $setname { "$ifname" }
-			exit 1
-		fi
+	lookup_elem "$setname" "$value" "$fail" "$expect_result" ""
+}
+
+# same, but also delete and re-add the element.
+check_elem()
+{
+	local setname=$1
+	local value=$2
+
+	lookup_elem "$setname" "$value" "0" "$value" "initial check"
+
+	ip netns exec "$ns1" $NFT delete element inet testifsets $setname { "$value" }
+	if [ $? -ne 0 ]; then
+		ip netns exec "$ns1" $NFT list ruleset
+		echo "delete element $setname { $value } failed"
+		exit 1
 	fi
+
+	ip netns exec "$ns1" $NFT add element inet testifsets $setname { "$value" }
+
+	lookup_elem "$setname" "$value" "0" "$value" "check after add/del"
 }
 
 # send pings, check all rules with sets that contain abcdef1 match.
@@ -56,8 +86,18 @@ check_matching_icmp_ppp()
 	want=3
 
 	if [ "$matches" -ne $want ] ;then
-		echo "Excpected $matches matching rules, got $want, packets $pkt"
 		ip netns exec "$ns1" $NFT list ruleset
+		echo "Expected $want matching rules, got $matches, packets $pkt in v4icmp"
+		exit 1
+	fi
+
+	# same, for concat set type.
+
+	matches=$(ip netns exec "$ns1" $NFT list chain inet testifsets v4icmpc | grep "counter packets $pkt " | wc -l)
+
+	if [ "$matches" -ne $want ] ;then
+		ip netns exec "$ns1" $NFT list ruleset
+		echo "Expected $want matching rules, got $matches, packets $pkt in v4icmpc"
 		exit 1
 	fi
 }
@@ -67,18 +107,25 @@ 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
+	check_elem simple $n
 done
 
-check_elem simple foo 1
+check_elem_get simple foo 1
 
 for n in ppp0 othername;do
-	check_elem simple_wild $n 0
+	check_elem simple_wild $n
 done
 
-check_elem simple_wild enoent 1
-check_elem simple_wild ppp0 0
-check_elem simple_wild abcdefghijk 0 'abcdef\*'
+check_elem_get simple_wild enoent 1
+check_elem simple_wild ppp0
+check_elem_get simple_wild abcdefghijk 0 'abcdef\*'
+
+check_elem_get concat '1.2.3.4 . "enoent"' 1
+check_elem_get concat '10.1.2.2 . "abcdef"' 1
+check_elem_get concat '10.1.2.1 . "abcdef1"' 1
+
+check_elem concat '10.1.2.2 . "abcdef0"'
+check_elem concat '10.1.2.2 . "abcdef1"'
 
 set -e
 ip -net "$ns1" link set lo up
-- 
2.35.1

Re: [PATCH nft 0/3] nftables: add support for wildcard interfaces

[Pablo Neira Ayuso]

On Fri, Apr 29, 2022 at 08:32:36PM +0200, Florian Westphal wrote:
> First patch is needed so kernel doesn't think end is before start,
> second patch allows to dump "x .foo*" correctly, without this
> nft tries to represent the start/end "name" with a range.
> This doesn't work well because end range uses \ff padding.
> 
> Patch 3 adds tests.

LGTM, thanks