[nft PATCH 0/5] Fixes (and fallout) from running tests/monitor in JSON mode

[nft PATCH 0/5] Fixes (and fallout) from running tests/monitor in JSON mode

[Phil Sutter]

The reported problem of object deletion notifications with bogus data is
resolved in patch four. It seems only object notifications were
affected. Patch 6 extends test suite coverage for tables and chains a
bit.

Patch one is an unrelated trivial fix, patches two and three cover for
oddities noticed while working on the actual problem.

Phil Sutter (5):
  tools: gitignore nftables.service file
  monitor: Quote device names in chain declarations, too
  mnl: Allow for updating devices on existing inet ingress hook chains
  monitor: Inform JSON printer when reporting an object delete event
  tests: monitor: Extend testcases a bit

 include/json.h                             |  5 +--
 src/evaluate.c                             |  6 ++--
 src/json.c                                 | 18 ++++++----
 src/mnl.c                                  |  2 ++
 src/monitor.c                              |  2 +-
 src/rule.c                                 |  2 +-
 tests/monitor/testcases/chain.t            | 41 ++++++++++++++++++++++
 tests/monitor/testcases/flowtable-simple.t | 12 +++++++
 tests/monitor/testcases/object.t           | 10 +++---
 tests/monitor/testcases/table.t            | 15 ++++++++
 tools/.gitignore                           |  1 +
 11 files changed, 97 insertions(+), 17 deletions(-)
 create mode 100644 tests/monitor/testcases/chain.t
 create mode 100644 tests/monitor/testcases/table.t
 create mode 100644 tools/.gitignore

-- 
2.51.0

[nft PATCH 1/5] tools: gitignore nftables.service file

[Phil Sutter]

Fixes: c4b17cf830510 ("tools: add a systemd unit for static rulesets")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tools/.gitignore | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 tools/.gitignore

diff --git a/tools/.gitignore b/tools/.gitignore
new file mode 100644
index 0000000000000..2d06c49835b15
--- /dev/null
+++ b/tools/.gitignore
@@ -0,0 +1 @@
+nftables.service
-- 
2.51.0

[nft PATCH 2/5] monitor: Quote device names in chain declarations, too

[Phil Sutter]

Fixed commit missed the fact that there are two routines printing chain
declarations.

Fixes: eb30f236d91a8 ("rule: print chain and flowtable devices in quotes")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/rule.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/rule.c b/src/rule.c
index ceb56488d37f6..d0a62a3ee002d 100644
--- a/src/rule.c
+++ b/src/rule.c
@@ -1131,7 +1131,7 @@ void chain_print_plain(const struct chain *chain, struct output_ctx *octx)
 
 			nft_print(octx, "devices = { ");
 			for (i = 0; i < chain->dev_array_len; i++) {
-				nft_print(octx, "%s", chain->dev_array[i]);
+				nft_print(octx, "\"%s\"", chain->dev_array[i]);
 				if (i + 1 != chain->dev_array_len)
 					nft_print(octx, ", ");
 			}
-- 
2.51.0

[nft PATCH 3/5] mnl: Allow for updating devices on existing inet ingress hook chains

[Phil Sutter]

Complete commit a66b5ad9540dd ("src: allow for updating devices on
existing netdev chain") in supporting inet family ingress hook chains as
well. The kernel does already but nft has to add a proper hooknum
attribute to pass the checks.

The hook.num field has to be initialized from hook.name using
str2hooknum(), which is part of chain evaluation. Calling
chain_evaluate() just for that purpose is a bit over the top, but the
hook name lookup may fail and performing chain evaluation for delete
command as well fits more into the code layout than duplicating parts of
it in mnl_nft_chain_del() or elsewhere. Just avoid the
chain_cache_find() call as its assert() triggers when deleting by
handle and also don't add to be deleted chains to cache.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 src/evaluate.c | 6 ++++--
 src/mnl.c      | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/evaluate.c b/src/evaluate.c
index b7e4f71fdfbc9..db4ac18f1dc9f 100644
--- a/src/evaluate.c
+++ b/src/evaluate.c
@@ -5758,7 +5758,9 @@ static int chain_evaluate(struct eval_ctx *ctx, struct chain *chain)
 		return table_not_found(ctx);
 
 	if (chain == NULL) {
-		if (!chain_cache_find(table, ctx->cmd->handle.chain.name)) {
+		if (ctx->cmd->op != CMD_DELETE &&
+		    ctx->cmd->op != CMD_DESTROY &&
+		    !chain_cache_find(table, ctx->cmd->handle.chain.name)) {
 			chain = chain_alloc();
 			handle_merge(&chain->handle, &ctx->cmd->handle);
 			chain_cache_add(chain, table);
@@ -6070,7 +6072,7 @@ static int cmd_evaluate_delete(struct eval_ctx *ctx, struct cmd *cmd)
 		return 0;
 	case CMD_OBJ_CHAIN:
 		chain_del_cache(ctx, cmd);
-		return 0;
+		return chain_evaluate(ctx, cmd->chain);
 	case CMD_OBJ_TABLE:
 		table_del_cache(ctx, cmd);
 		return 0;
diff --git a/src/mnl.c b/src/mnl.c
index 984dcac27b1cf..d1402c0fcb9f4 100644
--- a/src/mnl.c
+++ b/src/mnl.c
@@ -994,6 +994,8 @@ int mnl_nft_chain_del(struct netlink_ctx *ctx, struct cmd *cmd)
 		struct nlattr *nest;
 
 		nest = mnl_attr_nest_start(nlh, NFTA_CHAIN_HOOK);
+		mnl_attr_put_u32(nlh, NFTA_HOOK_HOOKNUM,
+				 htonl(cmd->chain->hook.num));
 		mnl_nft_chain_devs_build(nlh, cmd);
 		mnl_attr_nest_end(nlh, nest);
 	}
-- 
2.51.0

[nft PATCH 4/5] monitor: Inform JSON printer when reporting an object delete event

[Phil Sutter]

Since kernel commit a1050dd07168 ("netfilter: nf_tables: Reintroduce
shortened deletion notifications"), type-specific data is no longer
dumped when notifying for a deleted object. JSON output was not aware of
this and tried to print bogus data.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 include/json.h                   |  5 +++--
 src/json.c                       | 18 ++++++++++++------
 src/monitor.c                    |  2 +-
 tests/monitor/testcases/object.t | 10 +++++-----
 4 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/include/json.h b/include/json.h
index 42e1c8616c975..3b8d045f87bbc 100644
--- a/include/json.h
+++ b/include/json.h
@@ -113,7 +113,7 @@ void monitor_print_set_json(struct netlink_mon_handler *monh,
 void monitor_print_element_json(struct netlink_mon_handler *monh,
 				const char *cmd, struct set *s);
 void monitor_print_obj_json(struct netlink_mon_handler *monh,
-			    const char *cmd, struct obj *o);
+			    const char *cmd, struct obj *o, bool delete);
 void monitor_print_flowtable_json(struct netlink_mon_handler *monh,
 				  const char *cmd, struct flowtable *ft);
 void monitor_print_rule_json(struct netlink_mon_handler *monh,
@@ -252,7 +252,8 @@ static inline void monitor_print_element_json(struct netlink_mon_handler *monh,
 }
 
 static inline void monitor_print_obj_json(struct netlink_mon_handler *monh,
-					  const char *cmd, struct obj *o)
+					  const char *cmd, struct obj *o,
+					  bool delete)
 {
 	/* empty */
 }
diff --git a/src/json.c b/src/json.c
index d06fd04027140..0afce5415f541 100644
--- a/src/json.c
+++ b/src/json.c
@@ -397,7 +397,8 @@ static json_t *tunnel_erspan_print_json(const struct obj *obj)
 	return tunnel;
 }
 
-static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj)
+static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj,
+			      bool delete)
 {
 	const char *rate_unit = NULL, *burst_unit = NULL;
 	const char *type = obj_type_name(obj->type);
@@ -410,6 +411,9 @@ static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj)
 			"table", obj->handle.table.name,
 			"handle", obj->handle.handle.id);
 
+	if (delete)
+		goto out;
+
 	if (obj->comment) {
 		tmp = nft_json_pack("{s:s}", "comment", obj->comment);
 		json_object_update(root, tmp);
@@ -570,6 +574,7 @@ static json_t *obj_print_json(struct output_ctx *octx, const struct obj *obj)
 		break;
 	}
 
+out:
 	return nft_json_pack("{s:o}", type, root);
 }
 
@@ -1815,7 +1820,7 @@ static json_t *table_print_json_full(struct netlink_ctx *ctx,
 		json_array_append_new(root, tmp);
 	}
 	list_for_each_entry(obj, &table->obj_cache.list, cache.list) {
-		tmp = obj_print_json(&ctx->nft->output, obj);
+		tmp = obj_print_json(&ctx->nft->output, obj, false);
 		json_array_append_new(root, tmp);
 	}
 	list_for_each_entry(set, &table->set_cache.list, cache.list) {
@@ -1971,7 +1976,7 @@ static json_t *do_list_sets_json(struct netlink_ctx *ctx, struct cmd *cmd)
 static json_t *do_list_obj_json(struct netlink_ctx *ctx,
 				struct cmd *cmd, uint32_t type)
 {
-	json_t *root = json_array();
+	json_t *root = json_array(), *tmp;
 	struct table *table;
 	struct obj *obj;
 
@@ -1990,7 +1995,8 @@ static json_t *do_list_obj_json(struct netlink_ctx *ctx,
 			     strcmp(cmd->handle.obj.name, obj->handle.obj.name)))
 				continue;
 
-			json_array_append_new(root, obj_print_json(&ctx->nft->output, obj));
+			tmp = obj_print_json(&ctx->nft->output, obj, false);
+			json_array_append_new(root, tmp);
 		}
 	}
 
@@ -2207,11 +2213,11 @@ void monitor_print_element_json(struct netlink_mon_handler *monh,
 }
 
 void monitor_print_obj_json(struct netlink_mon_handler *monh,
-			    const char *cmd, struct obj *o)
+			    const char *cmd, struct obj *o, bool delete)
 {
 	struct output_ctx *octx = &monh->ctx->nft->output;
 
-	monitor_print_json(monh, cmd, obj_print_json(octx, o));
+	monitor_print_json(monh, cmd, obj_print_json(octx, o, delete));
 }
 
 void monitor_print_flowtable_json(struct netlink_mon_handler *monh,
diff --git a/src/monitor.c b/src/monitor.c
index e58f62252ca2d..fafeeebe914b8 100644
--- a/src/monitor.c
+++ b/src/monitor.c
@@ -549,7 +549,7 @@ static int netlink_events_obj_cb(const struct nlmsghdr *nlh, int type,
 		nft_mon_print(monh, "\n");
 		break;
 	case NFTNL_OUTPUT_JSON:
-		monitor_print_obj_json(monh, cmd, obj);
+		monitor_print_obj_json(monh, cmd, obj, type == NFT_MSG_DELOBJ);
 		if (!nft_output_echo(&monh->ctx->nft->output))
 			nft_mon_print(monh, "\n");
 		break;
diff --git a/tests/monitor/testcases/object.t b/tests/monitor/testcases/object.t
index 53a9f8c59836b..b60dc9899ab2a 100644
--- a/tests/monitor/testcases/object.t
+++ b/tests/monitor/testcases/object.t
@@ -9,7 +9,7 @@ J {"add": {"counter": {"family": "ip", "name": "c", "table": "t", "handle": 0, "
 
 I delete counter ip t c
 O -
-J {"delete": {"counter": {"family": "ip", "name": "c", "table": "t", "handle": 0, "packets": 0, "bytes": 0}}}
+J {"delete": {"counter": {"family": "ip", "name": "c", "table": "t", "handle": 0}}}
 
 # FIXME: input/output shouldn't be asynchronous here
 I add quota ip t q 25 mbytes
@@ -18,7 +18,7 @@ J {"add": {"quota": {"family": "ip", "name": "q", "table": "t", "handle": 0, "by
 
 I delete quota ip t q
 O -
-J {"delete": {"quota": {"family": "ip", "name": "q", "table": "t", "handle": 0, "bytes": 26214400, "used": 0, "inv": false}}}
+J {"delete": {"quota": {"family": "ip", "name": "q", "table": "t", "handle": 0}}}
 
 # FIXME: input/output shouldn't be asynchronous here
 I add limit ip t l rate 1/second
@@ -27,7 +27,7 @@ J {"add": {"limit": {"family": "ip", "name": "l", "table": "t", "handle": 0, "ra
 
 I delete limit ip t l
 O -
-J {"delete": {"limit": {"family": "ip", "name": "l", "table": "t", "handle": 0, "rate": 1, "per": "second", "burst": 5}}}
+J {"delete": {"limit": {"family": "ip", "name": "l", "table": "t", "handle": 0}}}
 
 I add ct helper ip t cth { type "sip" protocol tcp; l3proto ip; }
 O -
@@ -35,7 +35,7 @@ J {"add": {"ct helper": {"family": "ip", "name": "cth", "table": "t", "handle":
 
 I delete ct helper ip t cth
 O -
-J {"delete": {"ct helper": {"family": "ip", "name": "cth", "table": "t", "handle": 0, "type": "sip", "protocol": "tcp", "l3proto": "ip"}}}
+J {"delete": {"ct helper": {"family": "ip", "name": "cth", "table": "t", "handle": 0}}}
 
 I add ct timeout ip t ctt { protocol udp; l3proto ip; policy = { unreplied : 15s, replied : 12s }; }
 O -
@@ -43,4 +43,4 @@ J {"add": {"ct timeout": {"family": "ip", "name": "ctt", "table": "t", "handle":
 
 I delete ct timeout ip t ctt
 O -
-J {"delete": {"ct timeout": {"family": "ip", "name": "ctt", "table": "t", "handle": 0, "protocol": "udp", "l3proto": "ip", "policy": {"unreplied": 15, "replied": 12}}}}
+J {"delete": {"ct timeout": {"family": "ip", "name": "ctt", "table": "t", "handle": 0}}}
-- 
2.51.0

[nft PATCH 5/5] tests: monitor: Extend testcases a bit

[Phil Sutter]

Try to cover for reduced table and chain deletion notifications by
creating them with data which is omitted by the kernel during deletion.

Also try to expose the difference in reported flowtable hook deletion
vs. flowtable deletion.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tests/monitor/testcases/chain.t            | 41 ++++++++++++++++++++++
 tests/monitor/testcases/flowtable-simple.t | 12 +++++++
 tests/monitor/testcases/table.t            | 15 ++++++++
 3 files changed, 68 insertions(+)
 create mode 100644 tests/monitor/testcases/chain.t
 create mode 100644 tests/monitor/testcases/table.t

diff --git a/tests/monitor/testcases/chain.t b/tests/monitor/testcases/chain.t
new file mode 100644
index 0000000000000..975ccf1d33919
--- /dev/null
+++ b/tests/monitor/testcases/chain.t
@@ -0,0 +1,41 @@
+I add table inet t
+O -
+J {"add": {"table": {"family": "inet", "name": "t", "handle": 0}}}
+
+I add chain inet t c
+O -
+J {"add": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0}}}
+
+I delete chain inet t c
+O -
+J {"delete": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0}}}
+
+I add chain inet t c { type filter hook input priority filter; }
+O add chain inet t c { type filter hook input priority 0; policy accept; }
+J {"add": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0, "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}}
+
+I delete chain inet t c
+O -
+J {"delete": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0}}}
+
+I add chain inet t c { type filter hook ingress priority filter; devices = { "lo" }; }
+O add chain inet t c { type filter hook ingress devices = { "lo" } priority 0; policy accept; }
+J {"add": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0, "dev": "lo", "type": "filter", "hook": "ingress", "prio": 0, "policy": "accept"}}}
+
+I delete chain inet t c
+O -
+J {"delete": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0}}}
+
+I add chain inet t c { type filter hook ingress priority filter; devices = { "eth1", "lo" }; }
+O add chain inet t c { type filter hook ingress devices = { "eth1", "lo" } priority 0; policy accept; }
+J {"add": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0, "dev": ["eth1", "lo"], "type": "filter", "hook": "ingress", "prio": 0, "policy": "accept"}}}
+
+I delete chain inet t c { type filter hook ingress priority filter; devices = { "eth1" }; }
+O delete chain inet t c { type filter hook ingress devices = { "eth1" } priority 0; policy accept; }
+J {"delete": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0, "dev": "eth1", "type": "filter", "hook": "ingress", "prio": 0, "policy": "accept"}}}
+
+I delete chain inet t c
+O -
+J {"delete": {"chain": {"family": "inet", "table": "t", "name": "c", "handle": 0}}}
+
+
diff --git a/tests/monitor/testcases/flowtable-simple.t b/tests/monitor/testcases/flowtable-simple.t
index 11254c51fcab7..e1889ae5d0076 100644
--- a/tests/monitor/testcases/flowtable-simple.t
+++ b/tests/monitor/testcases/flowtable-simple.t
@@ -8,3 +8,15 @@ 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}}}
+
+I add flowtable ip t ft { hook ingress priority 0; devices = { "eth1", "lo" }; }
+O -
+J {"add": {"flowtable": {"family": "ip", "name": "ft", "table": "t", "handle": 0, "hook": "ingress", "prio": 0, "dev": ["eth1", "lo"]}}}
+
+I delete flowtable ip t ft { hook ingress priority 0; devices = { "eth1" }; }
+O -
+J {"delete": {"flowtable": {"family": "ip", "name": "ft", "table": "t", "handle": 0, "hook": "ingress", "prio": 0, "dev": "eth1"}}}
+
+I delete flowtable ip t ft
+O -
+J {"delete": {"flowtable": {"family": "ip", "name": "ft", "table": "t", "handle": 0}}}
diff --git a/tests/monitor/testcases/table.t b/tests/monitor/testcases/table.t
new file mode 100644
index 0000000000000..35a0f510436dc
--- /dev/null
+++ b/tests/monitor/testcases/table.t
@@ -0,0 +1,15 @@
+I add table ip t
+O -
+J {"add": {"table": {"family": "ip", "name": "t", "handle": 0}}}
+
+I delete table ip t
+O -
+J {"delete": {"table": {"family": "ip", "name": "t", "handle": 0}}}
+
+I add table ip t { comment "foo bar"; flags dormant; }
+O add table ip t { flags dormant; }
+J {"add": {"table": {"family": "ip", "name": "t", "handle": 0, "flags": ["dormant"], "comment": "foo bar"}}}
+
+I delete table ip t
+O -
+J {"delete": {"table": {"family": "ip", "name": "t", "handle": 0}}}
-- 
2.51.0

Re: [nft PATCH 0/5] Fixes (and fallout) from running tests/monitor in JSON mode

[Phil Sutter]

On Fri, Aug 29, 2025 at 04:25:08PM +0200, Phil Sutter wrote:
> The reported problem of object deletion notifications with bogus data is
> resolved in patch four. It seems only object notifications were
> affected. Patch 6 extends test suite coverage for tables and chains a
> bit.
> 
> Patch one is an unrelated trivial fix, patches two and three cover for
> oddities noticed while working on the actual problem.
> 
> Phil Sutter (5):
>   tools: gitignore nftables.service file
>   monitor: Quote device names in chain declarations, too

Applied these two unrelated trivial fixes.

>   mnl: Allow for updating devices on existing inet ingress hook chains
>   monitor: Inform JSON printer when reporting an object delete event
>   tests: monitor: Extend testcases a bit
> 
>  include/json.h                             |  5 +--
>  src/evaluate.c                             |  6 ++--
>  src/json.c                                 | 18 ++++++----
>  src/mnl.c                                  |  2 ++
>  src/monitor.c                              |  2 +-
>  src/rule.c                                 |  2 +-
>  tests/monitor/testcases/chain.t            | 41 ++++++++++++++++++++++
>  tests/monitor/testcases/flowtable-simple.t | 12 +++++++
>  tests/monitor/testcases/object.t           | 10 +++---
>  tests/monitor/testcases/table.t            | 15 ++++++++
>  tools/.gitignore                           |  1 +
>  11 files changed, 97 insertions(+), 17 deletions(-)
>  create mode 100644 tests/monitor/testcases/chain.t
>  create mode 100644 tests/monitor/testcases/table.t
>  create mode 100644 tools/.gitignore
> 
> -- 
> 2.51.0
> 
> 
>