[nft PATCH 0/7] Misc fixes

[nft PATCH 0/7] Misc fixes

[Phil Sutter]

Patch 1 is the most relevant one as an upcoming kernel fix will trigger
the bug being fixed by it.

Patches 2-5 are related to monitor testsuite, either fixing monitor
output or adjusting the test cases.

Patch 6 adjusts the shell testsuite for use with recent kernels (having
name-based interface hooks).

Patch 7 is an accidental discovery, probably I missed to add a needed
.json.output file when implementing new tests.

Phil Sutter (7):
  netlink: Fix for potential crash parsing a flowtable
  netlink: Do not allocate a bogus flowtable priority expr
  monitor: Correctly print flowtable updates
  json: Dump flowtable hook spec only if present
  tests: monitor: Fix for single flag array avoidance
  tests: shell: Adjust to ifname-based hooks
  tests: py: Properly fix JSON equivalents for netdev/reject.t

 src/json.c                                    | 22 +++--
 src/monitor.c                                 | 14 ++--
 src/netlink.c                                 |  8 +-
 tests/monitor/testcases/flowtable-simple.t    |  2 +-
 tests/monitor/testcases/map-expr.t            |  2 +-
 tests/monitor/testcases/set-concat-interval.t |  2 +-
 tests/monitor/testcases/set-interval.t        |  2 +-
 tests/monitor/testcases/set-maps.t            |  2 +-
 tests/monitor/testcases/set-mixed.t           |  2 +-
 tests/monitor/testcases/set-multiple.t        |  4 +-
 tests/monitor/testcases/set-simple.t          |  2 +-
 tests/py/netdev/reject.t.json                 | 66 ++++++++++-----
 tests/py/netdev/reject.t.json.output          | 81 +++++++++++++++++++
 tests/shell/features/ifname_based_hooks.sh    | 12 +++
 .../chains/netdev_chain_dormant_autoremove    |  3 +
 .../flowtable/0012flowtable_variable_0        |  9 ++-
 tests/shell/testcases/listing/0020flowtable_0 |  8 +-
 tests/shell/testcases/transactions/0050rule_1 | 19 -----
 .../transactions/dumps/0050rule_1.json-nft    | 11 ---
 .../transactions/dumps/0050rule_1.nft         |  0
 20 files changed, 193 insertions(+), 78 deletions(-)
 create mode 100644 tests/py/netdev/reject.t.json.output
 create mode 100755 tests/shell/features/ifname_based_hooks.sh
 delete mode 100755 tests/shell/testcases/transactions/0050rule_1
 delete mode 100644 tests/shell/testcases/transactions/dumps/0050rule_1.json-nft
 delete mode 100644 tests/shell/testcases/transactions/dumps/0050rule_1.nft

-- 
2.49.0

[nft PATCH 1/7] netlink: Fix for potential crash parsing a flowtable

[Phil Sutter]

Kernel's flowtable message might not contain the
NFTA_FLOWTABLE_HOOK_DEVS attribute. In that case, nftnl_flowtable_get()
will return NULL for the respective nftnl attribute.

Fixes: db0697ce7f602 ("src: support for flowtable listing")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/netlink.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/netlink.c b/src/netlink.c
index bed816af3123d..0e0d32b846d6a 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1847,7 +1847,7 @@ netlink_delinearize_flowtable(struct netlink_ctx *ctx,
 	if (nftnl_flowtable_is_set(nlo, NFTNL_FLOWTABLE_FLAGS))
 		flowtable->flags = nftnl_flowtable_get_u32(nlo, NFTNL_FLOWTABLE_FLAGS);
 	dev_array = nftnl_flowtable_get(nlo, NFTNL_FLOWTABLE_DEVICES);
-	while (dev_array[len])
+	while (dev_array && dev_array[len])
 		len++;
 
 	if (len)
-- 
2.49.0

[nft PATCH 2/7] netlink: Do not allocate a bogus flowtable priority expr

[Phil Sutter]

Code accidentally treats missing NFTNL_FLOWTABLE_PRIO attribute as zero
prio value which may not be correct.

Fixes: db0697ce7f602 ("src: support for flowtable listing")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/netlink.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/netlink.c b/src/netlink.c
index 0e0d32b846d6a..be1fefc068bfd 100644
--- a/src/netlink.c
+++ b/src/netlink.c
@@ -1862,14 +1862,16 @@ netlink_delinearize_flowtable(struct netlink_ctx *ctx,
 		      sizeof(char *), qsort_device_cmp);
 	}
 
-	priority = nftnl_flowtable_get_u32(nlo, NFTNL_FLOWTABLE_PRIO);
-	flowtable->priority.expr =
+	if (nftnl_flowtable_is_set(nlo, NFTNL_FLOWTABLE_PRIO)) {
+		priority = nftnl_flowtable_get_u32(nlo, NFTNL_FLOWTABLE_PRIO);
+		flowtable->priority.expr =
 				constant_expr_alloc(&netlink_location,
 						    &integer_type,
 						    BYTEORDER_HOST_ENDIAN,
 						    sizeof(int) *
 						    BITS_PER_BYTE,
 						    &priority);
+	}
 	flowtable->hook.num =
 		nftnl_flowtable_get_u32(nlo, NFTNL_FLOWTABLE_HOOKNUM);
 	flowtable->flags =
-- 
2.49.0

[nft PATCH 3/7] monitor: Correctly print flowtable updates

[Phil Sutter]

An update deleting a hook from a flowtable was indistinguishable from a
flowtable deletion.

Fixes: 73a8adfc2432e ("monitor: Recognize flowtable add/del events")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/monitor.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/src/monitor.c b/src/monitor.c
index 4ceff94824432..e3e38c2a12b78 100644
--- a/src/monitor.c
+++ b/src/monitor.c
@@ -577,14 +577,18 @@ static int netlink_events_flowtable_cb(const struct nlmsghdr *nlh, int type,
 		nft_mon_print(monh, "%s ", cmd);
 
 		switch (type) {
+		case NFT_MSG_DELFLOWTABLE:
+			if (!ft->dev_array_len) {
+				nft_mon_print(monh, "flowtable %s %s %s",
+					      family,
+					      ft->handle.table.name,
+					      ft->handle.flowtable.name);
+				break;
+			}
+			/* fall through */
 		case NFT_MSG_NEWFLOWTABLE:
 			flowtable_print_plain(ft, &monh->ctx->nft->output);
 			break;
-		case NFT_MSG_DELFLOWTABLE:
-			nft_mon_print(monh, "flowtable %s %s %s", family,
-				      ft->handle.table.name,
-				      ft->handle.flowtable.name);
-			break;
 		}
 		nft_mon_print(monh, "\n");
 		break;
-- 
2.49.0

[nft PATCH 4/7] json: Dump flowtable hook spec only if present

[Phil Sutter]

If there is no priority.expr set, assume hook.num is bogus, too.

While this is fixing JSON output, it's hard to tell what commit this is
actually fixing: Before commit 627c451b23513 ("src: allow variables in
the chain priority specification"), there was no way to detect
flowtables missing hook specs (e.g. when printing flowtable delete
monitor event).

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/json.c                                 | 22 ++++++++++++++--------
 tests/monitor/testcases/flowtable-simple.t |  2 +-
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/src/json.c b/src/json.c
index a46aed279167b..5bd5daf3f7fa6 100644
--- a/src/json.c
+++ b/src/json.c
@@ -493,18 +493,24 @@ static json_t *flowtable_print_json(const struct flowtable *ftable)
 	json_t *root, *devs = NULL;
 	int i, priority = 0;
 
+	root = nft_json_pack("{s:s, s:s, s:s, s:I}",
+			"family", family2str(ftable->handle.family),
+			"name", ftable->handle.flowtable.name,
+			"table", ftable->handle.table.name,
+			"handle", ftable->handle.handle.id);
+
 	if (ftable->priority.expr) {
+		json_t *tmp;
+
 		mpz_export_data(&priority, ftable->priority.expr->value,
 				BYTEORDER_HOST_ENDIAN, sizeof(int));
-	}
 
-	root = nft_json_pack("{s:s, s:s, s:s, s:I, s:s, s:i}",
-			"family", family2str(ftable->handle.family),
-			"name", ftable->handle.flowtable.name,
-			"table", ftable->handle.table.name,
-			"handle", ftable->handle.handle.id,
-			"hook", hooknum2str(NFPROTO_NETDEV, ftable->hook.num),
-			"prio", priority);
+		tmp = nft_json_pack("{s:s, s:i}",
+				    "hook", hooknum2str(NFPROTO_NETDEV,
+							ftable->hook.num),
+				    "prio", priority);
+		json_object_update_new(root, tmp);
+	}
 
 	for (i = 0; i < ftable->dev_array_len; i++) {
 		const char *dev = ftable->dev_array[i];
diff --git a/tests/monitor/testcases/flowtable-simple.t b/tests/monitor/testcases/flowtable-simple.t
index df8eccbd91e0a..b373cca2e0d61 100644
--- a/tests/monitor/testcases/flowtable-simple.t
+++ b/tests/monitor/testcases/flowtable-simple.t
@@ -7,4 +7,4 @@ J {"add": {"flowtable": {"family": "ip", "name": "ft", "table": "t", "handle": 0
 
 I delete flowtable ip t ft
 O -
-J {"delete": {"flowtable": {"family": "ip", "name": "ft", "table": "t", "handle": 0, "hook": "ingress", "prio": 0, "dev": "lo"}}}
+J {"delete": {"flowtable": {"family": "ip", "name": "ft", "table": "t", "handle": 0}}}
-- 
2.49.0

[nft PATCH 5/7] tests: monitor: Fix for single flag array avoidance

[Phil Sutter]

Missed to update the JSON monitor expected output.

Fixes: 6bedb12af1658 ("json: Print single set flag as non-array")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/monitor/testcases/map-expr.t            | 2 +-
 tests/monitor/testcases/set-concat-interval.t | 2 +-
 tests/monitor/testcases/set-interval.t        | 2 +-
 tests/monitor/testcases/set-maps.t            | 2 +-
 tests/monitor/testcases/set-mixed.t           | 2 +-
 tests/monitor/testcases/set-multiple.t        | 4 ++--
 tests/monitor/testcases/set-simple.t          | 2 +-
 7 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/tests/monitor/testcases/map-expr.t b/tests/monitor/testcases/map-expr.t
index d11ad0ebc0d57..904200418745e 100644
--- a/tests/monitor/testcases/map-expr.t
+++ b/tests/monitor/testcases/map-expr.t
@@ -3,4 +3,4 @@ I add table ip t
 I add map ip t m { typeof meta day . meta hour : verdict; flags interval; counter; }
 O -
 J {"add": {"table": {"family": "ip", "name": "t", "handle": 0}}}
-J {"add": {"map": {"family": "ip", "name": "m", "table": "t", "type": {"typeof": {"concat": [{"meta": {"key": "day"}}, {"meta": {"key": "hour"}}]}}, "handle": 0, "map": "verdict", "flags": ["interval"], "stmt": [{"counter": null}]}}}
+J {"add": {"map": {"family": "ip", "name": "m", "table": "t", "type": {"typeof": {"concat": [{"meta": {"key": "day"}}, {"meta": {"key": "hour"}}]}}, "handle": 0, "map": "verdict", "flags": "interval", "stmt": [{"counter": null}]}}}
diff --git a/tests/monitor/testcases/set-concat-interval.t b/tests/monitor/testcases/set-concat-interval.t
index 3542b8225ebd1..a42682f503246 100644
--- a/tests/monitor/testcases/set-concat-interval.t
+++ b/tests/monitor/testcases/set-concat-interval.t
@@ -10,6 +10,6 @@ I add map ip t s { typeof udp length . @ih,32,32 : verdict; flags interval; elem
 O add map ip t s { typeof udp length . @ih,32,32 : verdict; flags interval; }
 O add element ip t s { 20-80 . 0x14 : accept }
 O add element ip t s { 1-10 . 0xa : drop }
-J {"add": {"map": {"family": "ip", "name": "s", "table": "t", "type": {"typeof": {"concat": [{"payload": {"protocol": "udp", "field": "length"}}, {"payload": {"base": "ih", "offset": 32, "len": 32}}]}}, "handle": 0, "map": "verdict", "flags": ["interval"]}}}
+J {"add": {"map": {"family": "ip", "name": "s", "table": "t", "type": {"typeof": {"concat": [{"payload": {"protocol": "udp", "field": "length"}}, {"payload": {"base": "ih", "offset": 32, "len": 32}}]}}, "handle": 0, "map": "verdict", "flags": "interval"}}}
 J {"add": {"element": {"family": "ip", "table": "t", "name": "s", "elem": {"set": [[{"concat": [{"range": [20, 80]}, 20]}, {"accept": null}]]}}}}
 J {"add": {"element": {"family": "ip", "table": "t", "name": "s", "elem": {"set": [[{"concat": [{"range": [1, 10]}, 10]}, {"drop": null}]]}}}}
diff --git a/tests/monitor/testcases/set-interval.t b/tests/monitor/testcases/set-interval.t
index 5053c596b3b1b..84cf98c214671 100644
--- a/tests/monitor/testcases/set-interval.t
+++ b/tests/monitor/testcases/set-interval.t
@@ -10,7 +10,7 @@ I add set ip t s { type inet_service; flags interval; elements = { 20, 30-40 };
 O add set ip t s { type inet_service; flags interval; }
 O add element ip t s { 20 }
 O add element ip t s { 30-40 }
-J {"add": {"set": {"family": "ip", "name": "s", "table": "t", "type": "inet_service", "handle": 0, "flags": ["interval"]}}}
+J {"add": {"set": {"family": "ip", "name": "s", "table": "t", "type": "inet_service", "handle": 0, "flags": "interval"}}}
 J {"add": {"element": {"family": "ip", "table": "t", "name": "s", "elem": {"set": [20]}}}}
 J {"add": {"element": {"family": "ip", "table": "t", "name": "s", "elem": {"set": [{"range": [30, 40]}]}}}}
 
diff --git a/tests/monitor/testcases/set-maps.t b/tests/monitor/testcases/set-maps.t
index acda480d86dbb..aaf332f3caf98 100644
--- a/tests/monitor/testcases/set-maps.t
+++ b/tests/monitor/testcases/set-maps.t
@@ -3,7 +3,7 @@ I add table ip t
 I add map ip t portip { type inet_service: ipv4_addr; flags interval; }
 O -
 J {"add": {"table": {"family": "ip", "name": "t", "handle": 0}}}
-J {"add": {"map": {"family": "ip", "name": "portip", "table": "t", "type": "inet_service", "handle": 0, "map": "ipv4_addr", "flags": ["interval"]}}}
+J {"add": {"map": {"family": "ip", "name": "portip", "table": "t", "type": "inet_service", "handle": 0, "map": "ipv4_addr", "flags": "interval"}}}
 
 I add element ip t portip { 80-100: 10.0.0.1 }
 O -
diff --git a/tests/monitor/testcases/set-mixed.t b/tests/monitor/testcases/set-mixed.t
index 08c20116de92f..1cf3d38e34a7b 100644
--- a/tests/monitor/testcases/set-mixed.t
+++ b/tests/monitor/testcases/set-mixed.t
@@ -4,7 +4,7 @@ I add set ip t portrange { type inet_service; flags interval; }
 I add set ip t ports { type inet_service; }
 O -
 J {"add": {"table": {"family": "ip", "name": "t", "handle": 0}}}
-J {"add": {"set": {"family": "ip", "name": "portrange", "table": "t", "type": "inet_service", "handle": 0, "flags": ["interval"]}}}
+J {"add": {"set": {"family": "ip", "name": "portrange", "table": "t", "type": "inet_service", "handle": 0, "flags": "interval"}}}
 J {"add": {"set": {"family": "ip", "name": "ports", "table": "t", "type": "inet_service", "handle": 0}}}
 
 # make sure concurrent adds work
diff --git a/tests/monitor/testcases/set-multiple.t b/tests/monitor/testcases/set-multiple.t
index bd7a6246a46b4..84de98e94d139 100644
--- a/tests/monitor/testcases/set-multiple.t
+++ b/tests/monitor/testcases/set-multiple.t
@@ -4,8 +4,8 @@ I add set ip t portrange { type inet_service; flags interval; }
 I add set ip t portrange2 { type inet_service; flags interval; }
 O -
 J {"add": {"table": {"family": "ip", "name": "t", "handle": 0}}}
-J {"add": {"set": {"family": "ip", "name": "portrange", "table": "t", "type": "inet_service", "handle": 0, "flags": ["interval"]}}}
-J {"add": {"set": {"family": "ip", "name": "portrange2", "table": "t", "type": "inet_service", "handle": 0, "flags": ["interval"]}}}
+J {"add": {"set": {"family": "ip", "name": "portrange", "table": "t", "type": "inet_service", "handle": 0, "flags": "interval"}}}
+J {"add": {"set": {"family": "ip", "name": "portrange2", "table": "t", "type": "inet_service", "handle": 0, "flags": "interval"}}}
 
 # make sure concurrent adds work
 I add element ip t portrange { 1024-65535 }
diff --git a/tests/monitor/testcases/set-simple.t b/tests/monitor/testcases/set-simple.t
index 6853a0ebbb0cb..4bef144875876 100644
--- a/tests/monitor/testcases/set-simple.t
+++ b/tests/monitor/testcases/set-simple.t
@@ -3,7 +3,7 @@ I add table ip t
 I add set ip t portrange { type inet_service; flags interval; }
 O -
 J {"add": {"table": {"family": "ip", "name": "t", "handle": 0}}}
-J {"add": {"set": {"family": "ip", "name": "portrange", "table": "t", "type": "inet_service", "handle": 0, "flags": ["interval"]}}}
+J {"add": {"set": {"family": "ip", "name": "portrange", "table": "t", "type": "inet_service", "handle": 0, "flags": "interval"}}}
 
 # adding some ranges
 I add element ip t portrange { 1-10 }
-- 
2.49.0

[nft PATCH 6/7] tests: shell: Adjust to ifname-based hooks

[Phil Sutter]

Interface specs won't disappear anymore upon device removal. Drop them
manually if kernel has ifname-based hooks.

Also drop transactions/0050rule_1 test entirely: It won't fail anymore
as the flowtable is accepted despite the non-existent interfaces and
thus the test as a whole does not work anymore.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/shell/features/ifname_based_hooks.sh    | 12 ++++++++++++
 .../chains/netdev_chain_dormant_autoremove    |  3 +++
 .../flowtable/0012flowtable_variable_0        |  9 ++++++++-
 tests/shell/testcases/listing/0020flowtable_0 |  8 +++++++-
 tests/shell/testcases/transactions/0050rule_1 | 19 -------------------
 .../transactions/dumps/0050rule_1.json-nft    | 11 -----------
 .../transactions/dumps/0050rule_1.nft         |  0
 7 files changed, 30 insertions(+), 32 deletions(-)
 create mode 100755 tests/shell/features/ifname_based_hooks.sh
 delete mode 100755 tests/shell/testcases/transactions/0050rule_1
 delete mode 100644 tests/shell/testcases/transactions/dumps/0050rule_1.json-nft
 delete mode 100644 tests/shell/testcases/transactions/dumps/0050rule_1.nft

diff --git a/tests/shell/features/ifname_based_hooks.sh b/tests/shell/features/ifname_based_hooks.sh
new file mode 100755
index 0000000000000..cada6956f165b
--- /dev/null
+++ b/tests/shell/features/ifname_based_hooks.sh
@@ -0,0 +1,12 @@
+#!/bin/bash
+
+# check if netdev chains survive without a single device
+
+unshare -n bash -c "ip link add d0 type dummy; \
+	$NFT \"table netdev t { \
+		chain c { \
+			type filter hook ingress priority 0; devices = { d0 }; \
+		}; \
+	}\"; \
+	ip link del d0; \
+	$NFT list chain netdev t c"
diff --git a/tests/shell/testcases/chains/netdev_chain_dormant_autoremove b/tests/shell/testcases/chains/netdev_chain_dormant_autoremove
index 3093ce25319cf..8455f310445e9 100755
--- a/tests/shell/testcases/chains/netdev_chain_dormant_autoremove
+++ b/tests/shell/testcases/chains/netdev_chain_dormant_autoremove
@@ -9,3 +9,6 @@ ip link add dummy1 type dummy
 $NFT add table netdev test { flags dormant\; }
 $NFT add chain netdev test ingress { type filter hook ingress devices = { "dummy0", "dummy1" } priority 0\; policy drop\; }
 ip link del dummy0
+if [ "$NFT_TEST_HAVE_ifname_based_hooks" = y ]; then
+	$NFT 'delete chain netdev test ingress { devices = { "dummy0" }; }'
+fi
diff --git a/tests/shell/testcases/flowtable/0012flowtable_variable_0 b/tests/shell/testcases/flowtable/0012flowtable_variable_0
index 9c03820f128e3..ff35548ed8543 100755
--- a/tests/shell/testcases/flowtable/0012flowtable_variable_0
+++ b/tests/shell/testcases/flowtable/0012flowtable_variable_0
@@ -4,11 +4,18 @@
 
 set -e
 
+ft_deldev() {
+	$NFT "delete flowtable $1 $2 { devices = { $3 }; }"
+}
+
 iface_cleanup() {
 	ip link del dummy1 &>/dev/null || :
+	if [ "$NFT_TEST_HAVE_ifname_based_hooks" = y ]; then
+		ft_deldev filter1 Main_ft1 dummy1
+		ft_deldev filter2 Main_ft2 dummy1
+	fi
 }
 trap 'iface_cleanup' EXIT
-iface_cleanup
 
 ip link add name dummy1 type dummy
 
diff --git a/tests/shell/testcases/listing/0020flowtable_0 b/tests/shell/testcases/listing/0020flowtable_0
index 0e89f5dd01393..14b0c909a7eba 100755
--- a/tests/shell/testcases/listing/0020flowtable_0
+++ b/tests/shell/testcases/listing/0020flowtable_0
@@ -48,7 +48,13 @@ EXPECTED3="table ip filter {
 iface_cleanup() {
 	ip link del d0 &>/dev/null || :
 }
-trap 'iface_cleanup' EXIT
+ft_cleanup() {
+	if [ "$NFT_TEST_HAVE_ifname_based_hooks" = y ]; then
+		$NFT 'delete flowtable ip filter f2 { devices = { d0 }; }'
+		$NFT 'delete flowtable inet filter f2 { devices = { d0 }; }'
+	fi
+}
+trap 'iface_cleanup; ft_cleanup' EXIT
 iface_cleanup
 
 ip link add d0 type dummy
diff --git a/tests/shell/testcases/transactions/0050rule_1 b/tests/shell/testcases/transactions/0050rule_1
deleted file mode 100755
index 89e5f42fc9f4d..0000000000000
--- a/tests/shell/testcases/transactions/0050rule_1
+++ /dev/null
@@ -1,19 +0,0 @@
-#!/bin/bash
-
-set -e
-
-RULESET="table inet filter {
-	flowtable ftable {
-		hook ingress priority 0; devices = { eno1, eno0, x };
-	}
-
-chain forward {
-	type filter hook forward priority 0; policy drop;
-
-	ip protocol { tcp, udp } ct mark and 1 == 1 counter flow add @ftable
-	ip6 nexthdr { tcp, udp } ct mark and 2 == 2 counter flow add @ftable
-	ct mark and 30 == 30 ct state established,related log prefix \"nftables accept: \" level info accept
-	}
-}"
-
-$NFT -f - <<< "$RULESET" >/dev/null || exit 0
diff --git a/tests/shell/testcases/transactions/dumps/0050rule_1.json-nft b/tests/shell/testcases/transactions/dumps/0050rule_1.json-nft
deleted file mode 100644
index 546cc5977db61..0000000000000
--- a/tests/shell/testcases/transactions/dumps/0050rule_1.json-nft
+++ /dev/null
@@ -1,11 +0,0 @@
-{
-  "nftables": [
-    {
-      "metainfo": {
-        "version": "VERSION",
-        "release_name": "RELEASE_NAME",
-        "json_schema_version": 1
-      }
-    }
-  ]
-}
diff --git a/tests/shell/testcases/transactions/dumps/0050rule_1.nft b/tests/shell/testcases/transactions/dumps/0050rule_1.nft
deleted file mode 100644
index e69de29bb2d1d..0000000000000
-- 
2.49.0

[nft PATCH 7/7] tests: py: Properly fix JSON equivalents for netdev/reject.t

[Phil Sutter]

Revert commit d1a7b9e19fe65 ("tests: py: update netdev reject test
file"), the stored JSON equivalents were correct in that they matched
the standard syntax input.

In fact, we missed a .json.output file recording the expected deviation
in JSON output.

Fixes: d1a7b9e19fe65 ("tests: py: update netdev reject test file")
Fixes: 7ca3368cd7575 ("reject: Unify inet, netdev and bridge delinearization")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/py/netdev/reject.t.json        | 66 +++++++++++++++--------
 tests/py/netdev/reject.t.json.output | 81 ++++++++++++++++++++++++++++
 2 files changed, 126 insertions(+), 21 deletions(-)
 create mode 100644 tests/py/netdev/reject.t.json.output

diff --git a/tests/py/netdev/reject.t.json b/tests/py/netdev/reject.t.json
index 9968aaf834ec2..b80db03b9d3af 100644
--- a/tests/py/netdev/reject.t.json
+++ b/tests/py/netdev/reject.t.json
@@ -130,17 +130,6 @@
 
 # mark 12345 reject with tcp reset
 [
-    {
-        "match": {
-            "left": {
-                "meta": {
-                    "key": "l4proto"
-                }
-            },
-            "op": "==",
-            "right": 6
-        }
-    },
     {
         "match": {
             "left": {
@@ -162,30 +151,43 @@
 # reject
 [
     {
-        "reject": {
-            "expr": "port-unreachable",
-            "type": "icmpx"
-        }
+        "reject": null
     }
 ]
 
 # meta protocol ip reject
 [
     {
-        "reject": {
-            "expr": "port-unreachable",
-            "type": "icmp"
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "protocol"
+                }
+            },
+            "op": "==",
+            "right": "ip"
         }
+    },
+    {
+        "reject": null
     }
 ]
 
 # meta protocol ip6 reject
 [
     {
-        "reject": {
-            "expr": "port-unreachable",
-            "type": "icmpv6"
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "protocol"
+                }
+            },
+            "op": "==",
+            "right": "ip6"
         }
+    },
+    {
+        "reject": null
     }
 ]
 
@@ -231,6 +233,17 @@
 
 # meta protocol ip reject with icmp host-unreachable
 [
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "protocol"
+                }
+            },
+            "op": "==",
+            "right": "ip"
+        }
+    },
     {
         "reject": {
             "expr": "host-unreachable",
@@ -241,6 +254,17 @@
 
 # meta protocol ip6 reject with icmpv6 no-route
 [
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "protocol"
+                }
+            },
+            "op": "==",
+            "right": "ip6"
+        }
+    },
     {
         "reject": {
             "expr": "no-route",
diff --git a/tests/py/netdev/reject.t.json.output b/tests/py/netdev/reject.t.json.output
new file mode 100644
index 0000000000000..cbd73104e4432
--- /dev/null
+++ b/tests/py/netdev/reject.t.json.output
@@ -0,0 +1,81 @@
+# mark 12345 reject with tcp reset
+[
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "l4proto"
+                }
+            },
+            "op": "==",
+            "right": 6
+        }
+    },
+    {
+        "match": {
+            "left": {
+                "meta": {
+                    "key": "mark"
+                }
+            },
+            "op": "==",
+            "right": 12345
+        }
+    },
+    {
+        "reject": {
+            "type": "tcp reset"
+        }
+    }
+]
+
+# reject
+[
+    {
+        "reject": {
+            "expr": "port-unreachable",
+            "type": "icmpx"
+        }
+    }
+]
+
+# meta protocol ip reject
+[
+    {
+        "reject": {
+            "expr": "port-unreachable",
+            "type": "icmp"
+        }
+    }
+]
+
+# meta protocol ip6 reject
+[
+    {
+        "reject": {
+            "expr": "port-unreachable",
+            "type": "icmpv6"
+        }
+    }
+]
+
+# meta protocol ip reject with icmp host-unreachable
+[
+    {
+        "reject": {
+            "expr": "host-unreachable",
+            "type": "icmp"
+        }
+    }
+]
+
+# meta protocol ip6 reject with icmpv6 no-route
+[
+    {
+        "reject": {
+            "expr": "no-route",
+            "type": "icmpv6"
+        }
+    }
+]
+
-- 
2.49.0

Re: [nft PATCH 6/7] tests: shell: Adjust to ifname-based hooks

[Pablo Neira Ayuso]

On Thu, Jun 12, 2025 at 01:52:17PM +0200, Phil Sutter wrote:
[...]
> diff --git a/tests/shell/testcases/transactions/0050rule_1 b/tests/shell/testcases/transactions/0050rule_1
> deleted file mode 100755
> index 89e5f42fc9f4d..0000000000000
> --- a/tests/shell/testcases/transactions/0050rule_1
> +++ /dev/null
> @@ -1,19 +0,0 @@
> -#!/bin/bash

I would prefer this test does not go away, this is catching for a old
kernel bug if you take a look at the history, ie. it is an old
bug reproducer so...

> -
> -set -e
> -
> -RULESET="table inet filter {
> -	flowtable ftable {
> -		hook ingress priority 0; devices = { eno1, eno0, x };
> -	}
> -
> -chain forward {
> -	type filter hook forward priority 0; policy drop;
> -
> -	ip protocol { tcp, udp } ct mark and 1 == 1 counter flow add @ftable
> -	ip6 nexthdr { tcp, udp } ct mark and 2 == 2 counter flow add @ftable
> -	ct mark and 30 == 30 ct state established,related log prefix \"nftables accept: \" level info accept
> -	}
> -}"
> -
> -$NFT -f - <<< "$RULESET" >/dev/null || exit 0

maybe simply add here:

$NFT flush ruleset

to get the same behaviour in old and new kernels.

I did not look at other tests.

Please have a look at the history of other tests to check if they are
also catching very old kernel bugs.

Re: [nft PATCH 0/7] Misc fixes

[Pablo Neira Ayuso]

On Thu, Jun 12, 2025 at 01:52:11PM +0200, Phil Sutter wrote:
> Patch 1 is the most relevant one as an upcoming kernel fix will trigger
> the bug being fixed by it.
> 
> Patches 2-5 are related to monitor testsuite, either fixing monitor
> output or adjusting the test cases.
> 
> Patch 6 adjusts the shell testsuite for use with recent kernels (having
> name-based interface hooks).
> 
> Patch 7 is an accidental discovery, probably I missed to add a needed
> .json.output file when implementing new tests.

Series LGTM: Please keep test 0050_rule1 in place, I would prefer not
to lose coverage for very old bugs. Please double-check other test
updates in 6/7.

Aside from that silly nitpick of mine:

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

Thanks

Re: [nft PATCH 6/7] tests: shell: Adjust to ifname-based hooks

[Phil Sutter]

On Thu, Jun 12, 2025 at 09:33:39PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Jun 12, 2025 at 01:52:17PM +0200, Phil Sutter wrote:
> [...]
> > diff --git a/tests/shell/testcases/transactions/0050rule_1 b/tests/shell/testcases/transactions/0050rule_1
> > deleted file mode 100755
> > index 89e5f42fc9f4d..0000000000000
> > --- a/tests/shell/testcases/transactions/0050rule_1
> > +++ /dev/null
> > @@ -1,19 +0,0 @@
> > -#!/bin/bash
> 
> I would prefer this test does not go away, this is catching for a old
> kernel bug if you take a look at the history, ie. it is an old
> bug reproducer so...
> 
> > -
> > -set -e
> > -
> > -RULESET="table inet filter {
> > -	flowtable ftable {
> > -		hook ingress priority 0; devices = { eno1, eno0, x };
> > -	}
> > -
> > -chain forward {
> > -	type filter hook forward priority 0; policy drop;
> > -
> > -	ip protocol { tcp, udp } ct mark and 1 == 1 counter flow add @ftable
> > -	ip6 nexthdr { tcp, udp } ct mark and 2 == 2 counter flow add @ftable
> > -	ct mark and 30 == 30 ct state established,related log prefix \"nftables accept: \" level info accept
> > -	}
> > -}"
> > -
> > -$NFT -f - <<< "$RULESET" >/dev/null || exit 0
> 
> maybe simply add here:
> 
> $NFT flush ruleset
> 
> to get the same behaviour in old and new kernels.

Ah, good point. It's better to skip the test if ifname_based_hooks
feature is present instead of dropping it.

> I did not look at other tests.
> 
> Please have a look at the history of other tests to check if they are
> also catching very old kernel bugs.

The other two tests I touched merely remove flowtable hooks before
returning.

Thanks, Phil

Re: [nft PATCH 0/7] Misc fixes

[Phil Sutter]

On Thu, Jun 12, 2025 at 09:37:43PM +0200, Pablo Neira Ayuso wrote:
> On Thu, Jun 12, 2025 at 01:52:11PM +0200, Phil Sutter wrote:
> > Patch 1 is the most relevant one as an upcoming kernel fix will trigger
> > the bug being fixed by it.
> > 
> > Patches 2-5 are related to monitor testsuite, either fixing monitor
> > output or adjusting the test cases.
> > 
> > Patch 6 adjusts the shell testsuite for use with recent kernels (having
> > name-based interface hooks).
> > 
> > Patch 7 is an accidental discovery, probably I missed to add a needed
> > .json.output file when implementing new tests.
> 
> Series LGTM: Please keep test 0050_rule1 in place, I would prefer not
> to lose coverage for very old bugs. Please double-check other test
> updates in 6/7.

ACK, will do.

> Aside from that silly nitpick of mine:
> 
> Reviewed-by: Pablo Neira Ayuso <pablo@netfilter.org>

Thanks for your review! I'll push the series along with the other crash
fix once done with the above.

Thanks, Phil

Re: [nft PATCH 0/7] Misc fixes

[Phil Sutter]

On Thu, Jun 12, 2025 at 01:52:11PM +0200, Phil Sutter wrote:
> Patch 1 is the most relevant one as an upcoming kernel fix will trigger
> the bug being fixed by it.
> 
> Patches 2-5 are related to monitor testsuite, either fixing monitor
> output or adjusting the test cases.
> 
> Patch 6 adjusts the shell testsuite for use with recent kernels (having
> name-based interface hooks).
> 
> Patch 7 is an accidental discovery, probably I missed to add a needed
> .json.output file when implementing new tests.
> 
> Phil Sutter (7):
>   netlink: Fix for potential crash parsing a flowtable
>   netlink: Do not allocate a bogus flowtable priority expr
>   monitor: Correctly print flowtable updates
>   json: Dump flowtable hook spec only if present
>   tests: monitor: Fix for single flag array avoidance
>   tests: shell: Adjust to ifname-based hooks
>   tests: py: Properly fix JSON equivalents for netdev/reject.t

Series applied.

Re: [nft PATCH 3/7] monitor: Correctly print flowtable updates

[Pablo Neira Ayuso]

On Thu, Jun 12, 2025 at 01:52:14PM +0200, Phil Sutter wrote:
> An update deleting a hook from a flowtable was indistinguishable from a
> flowtable deletion.

tests/monitor fails:

--- /tmp/tmp.CxT9laP7kj/tmp.qTOOOcfTUY  2025-06-15 11:44:55.690784518 +0200
+++ /tmp/tmp.CxT9laP7kj/tmp.JdiYcpuAKK  2025-06-15 11:44:56.337658195 +0200
@@ -1 +1,2 @@
-delete flowtable ip t ft
+delete flowtable ip t ft { hook ingress priority 0; devices = { lo }; }
+# new generation 3 by process 2954068 (nft)

> Fixes: 73a8adfc2432e ("monitor: Recognize flowtable add/del events")
> Signed-off-by: Phil Sutter <phil@nwl.cc>
> ---
>  src/monitor.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/src/monitor.c b/src/monitor.c
> index 4ceff94824432..e3e38c2a12b78 100644
> --- a/src/monitor.c
> +++ b/src/monitor.c
> @@ -577,14 +577,18 @@ static int netlink_events_flowtable_cb(const struct nlmsghdr *nlh, int type,
>  		nft_mon_print(monh, "%s ", cmd);
>  
>  		switch (type) {
> +		case NFT_MSG_DELFLOWTABLE:
> +			if (!ft->dev_array_len) {
> +				nft_mon_print(monh, "flowtable %s %s %s",
> +					      family,
> +					      ft->handle.table.name,
> +					      ft->handle.flowtable.name);
> +				break;
> +			}
> +			/* fall through */
>  		case NFT_MSG_NEWFLOWTABLE:
>  			flowtable_print_plain(ft, &monh->ctx->nft->output);
>  			break;
> -		case NFT_MSG_DELFLOWTABLE:
> -			nft_mon_print(monh, "flowtable %s %s %s", family,
> -				      ft->handle.table.name,
> -				      ft->handle.flowtable.name);
> -			break;
>  		}
>  		nft_mon_print(monh, "\n");
>  		break;
> -- 
> 2.49.0
> 

Re: [nft PATCH 3/7] monitor: Correctly print flowtable updates

[Phil Sutter]

Hi Pablo,

On Sun, Jun 15, 2025 at 11:47:20AM +0200, Pablo Neira Ayuso wrote:
> On Thu, Jun 12, 2025 at 01:52:14PM +0200, Phil Sutter wrote:
> > An update deleting a hook from a flowtable was indistinguishable from a
> > flowtable deletion.
> 
> tests/monitor fails:
> 
> --- /tmp/tmp.CxT9laP7kj/tmp.qTOOOcfTUY  2025-06-15 11:44:55.690784518 +0200
> +++ /tmp/tmp.CxT9laP7kj/tmp.JdiYcpuAKK  2025-06-15 11:44:56.337658195 +0200
> @@ -1 +1,2 @@
> -delete flowtable ip t ft
> +delete flowtable ip t ft { hook ingress priority 0; devices = { lo }; }
> +# new generation 3 by process 2954068 (nft)

Ah crap, this requires the kernel patch 'netfilter: nf_tables:
Reintroduce shortened deletion notifications'.

I don't see how user space could work around the old kernel behaviour,
so monitor testsuite will fail for old kernels with either this patch
applied or as soon as we add a test for a flowtable update removing a
hook spec.

The only way out I see is to accept the extra data unchecked in monitor
testsuite, i.e. practically disabling the tests for flowtable deletion
or updates, which obviously sucks. No idea how to move forward now.

Sorry, Phil