[nft PATCH] parser_json: support handle for rule positioning in JSON add rule

[nft PATCH] parser_json: support handle for rule positioning in JSON add rule

[Alexandre Knecht]

This patch fixes JSON-based rule positioning when using "add rule" with
a handle parameter. Previously, the handle was deleted before being used
for positioning, causing rules to always be appended at the end of the
chain instead of being placed after the specified rule handle.

The fix follows the same pattern used in json_parse_cmd_replace():
- Parse the handle field from JSON
- Convert handle to position for CMD_ADD operations
- Remove the code that was deleting the handle field

With NLM_F_APPEND set (as it always is for add operations), the kernel
interprets position as "add after this handle", which matches the CLI
behavior of "add rule position X".

Before this fix:
  nft -j add rule ... handle 2  --> rule added at end

After this fix:
  nft -j add rule ... handle 2  --> rule added after handle 2

The CLI version (nft add rule ... position X) was already working
correctly.

Tested with:
  # nft add table inet test
  # nft add chain inet test c
  # nft add rule inet test c tcp dport 80 accept
  # nft add rule inet test c tcp dport 443 accept
  # echo '{"nftables":[{"add":{"rule":{"family":"inet","table":"test","chain":"c","handle":2,"expr":[{"match":{"left":{"payload":{"protocol":"tcp","field":"dport"}},"op":"==","right":8080}},{"accept":null}]}}}]}' | nft -j -f -
  # nft -a list table inet test

Result: Rule with port 8080 correctly placed after handle 2 (port 80).

Signed-off-by: Alexandre Knecht <knecht.alexandre@gmail.com>
---
 src/parser_json.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/parser_json.c b/src/parser_json.c
index 7b4f3384..c974a9e2 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3197,10 +3197,18 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root,
 		return NULL;
 	}
 
+	/* Parse handle and index (similar to json_parse_cmd_replace) */
+	json_unpack(root, "{s:I}", "handle", &h.handle.id);
 	if (!json_unpack(root, "{s:I}", "index", &h.index.id)) {
 		h.index.id++;
 	}
 
+	/* For CMD_ADD, convert handle to position for rule positioning */
+	if ((op == CMD_ADD || op == CMD_CREATE) && h.handle.id) {
+		h.position.id = h.handle.id;
+		h.handle.id = 0;
+	}
+
 	rule = rule_alloc(int_loc, NULL);
 
 	json_unpack(root, "{s:s}", "comment", &comment);
@@ -3226,9 +3234,6 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root,
 		rule_stmt_append(rule, stmt);
 	}
 
-	if (op == CMD_ADD)
-		json_object_del(root, "handle");
-
 	return cmd_alloc(op, obj, &h, int_loc, rule);
 
 err_free_rule:
-- 
2.51.0

Re: [nft PATCH] parser_json: support handle for rule positioning in JSON add rule

[Florian Westphal]

Alexandre Knecht <knecht.alexandre@gmail.com> wrote:
>   # nft add table inet test
>   # nft add chain inet test c
>   # nft add rule inet test c tcp dport 80 accept
>   # nft add rule inet test c tcp dport 443 accept
>   # echo '{"nftables":[{"add":{"rule":{"family":"inet","table":"test","chain":"c","handle":2,"expr":[{"match":{"left":{"payload":{"protocol":"tcp","field":"dport"}},"op":"==","right":8080}},{"accept":null}]}}}]}' | nft -j -f -
>   # nft -a list table inet test

Could you turn this into a test case (tests/shell/testcases/json/)?

Thanks!

(no subject)

[Alexandre Knecht]

Hi Florian,

Thank you for your reply, please find the test case below.

I've added a shell test in tests/shell/testcases/json/ that verifies
the handle positioning works correctly. The test has been verified
in a clean container environment and passes successfully.

Thanks,
Alexandre

[nft PATCH v2] parser_json: support handle for rule positioning in JSON add rule

[Alexandre Knecht]

This patch fixes JSON-based rule positioning when using "add rule" with
a handle parameter. Previously, the handle was deleted before being used
for positioning, causing rules to always be appended at the end of the
chain instead of being placed after the specified rule handle.

The fix follows the same pattern used in json_parse_cmd_replace():
- Parse the handle field from JSON
- Convert handle to position for CMD_ADD operations
- Remove the code that was deleting the handle field

With NLM_F_APPEND set (as it always is for add operations), the kernel
interprets position as "add after this handle", which matches the CLI
behavior of "add rule position X".

Before this fix:
  nft -j add rule ... handle 2  --> rule added at end

After this fix:
  nft -j add rule ... handle 2  --> rule added after handle 2

The CLI version (nft add rule ... position X) was already working
correctly.

Tested with:
  # nft add table inet test
  # nft add chain inet test c
  # nft add rule inet test c tcp dport 80 accept
  # nft add rule inet test c tcp dport 443 accept
  # echo '{"nftables":[{"add":{"rule":{"family":"inet","table":"test","chain":"c","handle":2,"expr":[{"match":{"left":{"payload":{"protocol":"tcp","field":"dport"}},"op":"==","right":8080}},{"accept":null}]}}}]}' | nft -j -f -
  # nft -a list table inet test

Result: Rule with port 8080 correctly placed after handle 2 (port 80).

Signed-off-by: Alexandre Knecht <knecht.alexandre@gmail.com>
---
 src/parser_json.c                             |  11 +-
 .../json/0007rule_add_handle_position_0       |  22 ++++
 .../0007rule_add_handle_position_0.json-nft   | 101 ++++++++++++++++++
 .../dumps/0007rule_add_handle_position_0.nft  |   7 ++
 4 files changed, 138 insertions(+), 3 deletions(-)
 create mode 100755 tests/shell/testcases/json/0007rule_add_handle_position_0
 create mode 100644 tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.nft

diff --git a/src/parser_json.c b/src/parser_json.c
index 7b4f3384..c974a9e2 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -3197,10 +3197,18 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root,
 		return NULL;
 	}
 
+	/* Parse handle and index (similar to json_parse_cmd_replace) */
+	json_unpack(root, "{s:I}", "handle", &h.handle.id);
 	if (!json_unpack(root, "{s:I}", "index", &h.index.id)) {
 		h.index.id++;
 	}
 
+	/* For CMD_ADD, convert handle to position for rule positioning */
+	if ((op == CMD_ADD || op == CMD_CREATE) && h.handle.id) {
+		h.position.id = h.handle.id;
+		h.handle.id = 0;
+	}
+
 	rule = rule_alloc(int_loc, NULL);
 
 	json_unpack(root, "{s:s}", "comment", &comment);
@@ -3226,9 +3234,6 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root,
 		rule_stmt_append(rule, stmt);
 	}
 
-	if (op == CMD_ADD)
-		json_object_del(root, "handle");
-
 	return cmd_alloc(op, obj, &h, int_loc, rule);
 
 err_free_rule:
diff --git a/tests/shell/testcases/json/0007rule_add_handle_position_0 b/tests/shell/testcases/json/0007rule_add_handle_position_0
new file mode 100755
index 00000000..e7dde7c7
--- /dev/null
+++ b/tests/shell/testcases/json/0007rule_add_handle_position_0
@@ -0,0 +1,22 @@
+#!/bin/bash
+
+# NFT_TEST_REQUIRES(NFT_TEST_HAVE_json)
+
+# Test that JSON add rule with handle parameter positions rule correctly
+
+set -e
+
+$NFT flush ruleset
+
+# Create table, chain, and two initial rules
+# The first rule (port 80) will get handle 2
+# The second rule (port 443) will get handle 3
+SETUP='{"nftables": [{"add": {"table": {"family": "inet", "name": "test"}}}, {"add": {"chain": {"family": "inet", "table": "test", "name": "c"}}}, {"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "expr": [{"match": {"left": {"payload": {"protocol": "tcp", "field": "dport"}}, "op": "==", "right": 80}}, {"accept": null}]}}}, {"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "expr": [{"match": {"left": {"payload": {"protocol": "tcp", "field": "dport"}}, "op": "==", "right": 443}}, {"accept": null}]}}}]}'
+
+$NFT -j -f - <<< "$SETUP"
+
+# Add a new rule positioned after handle 2 (the port 80 rule)
+# This rule should end up between port 80 and port 443
+ADD_RULE='{"nftables": [{"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "handle": 2, "expr": [{"match": {"left": {"payload": {"protocol": "tcp", "field": "dport"}}, "op": "==", "right": 8080}}, {"accept": null}]}}}]}'
+
+$NFT -j -f - <<< "$ADD_RULE"
diff --git a/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.json-nft b/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.json-nft
new file mode 100644
index 00000000..667e14c1
--- /dev/null
+++ b/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.json-nft
@@ -0,0 +1,101 @@
+{
+  "nftables": [
+    {
+      "metainfo": {
+        "version": "VERSION",
+        "release_name": "RELEASE_NAME",
+        "json_schema_version": 1
+      }
+    },
+    {
+      "table": {
+        "family": "inet",
+        "name": "test",
+        "handle": 0
+      }
+    },
+    {
+      "chain": {
+        "family": "inet",
+        "table": "test",
+        "name": "c",
+        "handle": 0
+      }
+    },
+    {
+      "rule": {
+        "family": "inet",
+        "table": "test",
+        "chain": "c",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "payload": {
+                  "protocol": "tcp",
+                  "field": "dport"
+                }
+              },
+              "right": 80
+            }
+          },
+          {
+            "accept": null
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "inet",
+        "table": "test",
+        "chain": "c",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "payload": {
+                  "protocol": "tcp",
+                  "field": "dport"
+                }
+              },
+              "right": 8080
+            }
+          },
+          {
+            "accept": null
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "inet",
+        "table": "test",
+        "chain": "c",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "payload": {
+                  "protocol": "tcp",
+                  "field": "dport"
+                }
+              },
+              "right": 443
+            }
+          },
+          {
+            "accept": null
+          }
+        ]
+      }
+    }
+  ]
+}
diff --git a/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.nft b/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.nft
new file mode 100644
index 00000000..d3e8e1ba
--- /dev/null
+++ b/tests/shell/testcases/json/dumps/0007rule_add_handle_position_0.nft
@@ -0,0 +1,7 @@
+table inet test {
+	chain c {
+		tcp dport 80 accept
+		tcp dport 8080 accept
+		tcp dport 443 accept
+	}
+}
-- 
2.51.0

Re: [nft PATCH v2] parser_json: support handle for rule positioning in JSON add rule

[Florian Westphal]

Alexandre Knecht <knecht.alexandre@gmail.com> wrote:
> This patch fixes JSON-based rule positioning when using "add rule" with
> a handle parameter. Previously, the handle was deleted before being used
> for positioning, causing rules to always be appended at the end of the
> chain instead of being placed after the specified rule handle.

LGTM, thanks for providing a new test for this.

Phil?  Any objections/comments?
If not I'll apply this.

Re: [nft PATCH v2] parser_json: support handle for rule positioning in JSON add rule

[Florian Westphal]

Florian Westphal <fw@strlen.de> wrote:
> Alexandre Knecht <knecht.alexandre@gmail.com> wrote:
> > This patch fixes JSON-based rule positioning when using "add rule" with
> > a handle parameter. Previously, the handle was deleted before being used
> > for positioning, causing rules to always be appended at the end of the
> > chain instead of being placed after the specified rule handle.
> 
> LGTM, thanks for providing a new test for this.

Take that back, this patch breaks

tests/shell/testcases/json/0005secmark_objref_0

Re: [nft PATCH v2] parser_json: support handle for rule positioning in JSON add rule

[Alexandre Knecht]

Hi Florian,

Thanks for your feedback! I've investigated the test failure and found an
interesting design that I'd like to discuss.

The problem:

The test 0005secmark_objref_0 (and 0001set_statements_0) fail because they
include handles in rule JSON that don't exist yet. For example:

  {"rule": {"family": "inet", "table": "x", "chain": "y", "handle": 4, ...}}

My patch makes JSON "handle" behave like CLI handle - converting it to a
positioning reference. This causes "No such file or directory" errors because
handle 4 doesn't exist yet.

Current behavior vs. expected:

In CLI, this already fails as expected:

  $ nft add rule inet x y handle 4 tcp dport 80 accept
  Error: Could not process rule: No such file or directory

But in JSON (before my patch), handles were silently ignored for ADD operations.

The current JSON design :

As Phil Sutter noted in commit fb557b55 (2018): "For a programmatic API like
JSON, this should be fine" - JSON always exports handles regardless of -a flag.

I couldn't agree more with JSON being a programmatic API.

I verified:

  $ nft -j list ruleset    # includes "handle": 2
  $ nft -a -j list ruleset # same output, includes "handle": 2

This creates a conflict:
1. Export/import: JSON exports include handles (for reference/metadata)
2. Positioning: JSON should support adding rules at specific positions
(like CLI)
3. Ambiguity: when "nft -j -f" receives "handle": X, he doesn't know
if it is for positioning
   or just metadata from an export?

Why we shouldn't solve this in code:

Doing a handle lookup before deciding whether to use it for positioning could
cause race conditions when adding multiple rules or add slowness. The solution
must come from the input format itself to be unambiguous.

Proposed solution:

Reintroduce the "position" field in JSON for explicit positioning:
- "handle": ignored for ADD operations (treated as metadata from exports)
- "position": used for positioning (add after this handle)
- "index": already supported for absolute positioning

Why "position" makes sense for JSON:

While the "position" keyword was deprecated in CLI (commit effb881c, 2018) in
favor of "handle" for consistency, it still exists for backwards compatibility
and is even documented in the official wiki:
https://wiki.nftables.org/wiki-nftables/index.php/Simple_rule_management

For JSON, "position" could serve a clearer purpose:
- Separates positioning intent from metadata
- Allows safe export/import (handles are preserved but ignored for positioning)
- Provides explicit semantics: "position": X means "add/insert
relative to handle X"

Example:

Export/import scenario (handles are metadata):
  {"add": {"rule": {"family": "inet", "table": "test", "chain": "c",
                    "handle": 4, "expr": [...]}}}
  → Handle 4 is ignored, rule appended

Explicit positioning:
  {"add": {"rule": {"family": "inet", "table": "test", "chain": "c",
                    "position": 2, "expr": [...]}}}
  → Rule added after handle 2

What do you think about this approach? I can implement it if you agree it's
the right direction.

Thanks for your time!

Alexandre

Re: [nft PATCH v2] parser_json: support handle for rule positioning in JSON add rule

[Florian Westphal]

Alexandre Knecht <knecht.alexandre@gmail.com> wrote:
 Export/import scenario (handles are metadata):
>   {"add": {"rule": {"family": "inet", "table": "test", "chain": "c",
>                     "handle": 4, "expr": [...]}}}
>   → Handle 4 is ignored, rule appended
> 
> Explicit positioning:
>   {"add": {"rule": {"family": "inet", "table": "test", "chain": "c",
>                     "position": 2, "expr": [...]}}}
>   → Rule added after handle 2
> 
> What do you think about this approach? I can implement it if you agree it's
> the right direction.

I think its a sensible strategy, yes.

Reactivating the handle is a no-go as it will break existing cases.

Could you also add a test case that validates the various relative
positioning outcomes?

i.e. given:

rule handle 1
rule handle 2
rule handle 3

- check that positinging at 1 results in
rule 1
rule N
rule 2
rule 3

- check that positining at rule 1 results in

rule 1
rule N2
rule N
rule 2
rule 3

- check that positinging at rule 1 will fail
in case that rule was already deleted.

- check that positinging at rule 2 will insert
after handle 2 and not N2.

My only question is how "Position" is treated with insert (instead of
add).

It should NOT be ignored, it should either be rejected outright (and
everywhere its not expected) or it should have different meaning, ie.
prepend (insert occurs before the given handle).

Re: [nft PATCH v2] parser_json: support handle for rule positioning in JSON add rule

[Phil Sutter]

Hi,

On Sun, Nov 02, 2025 at 01:41:48PM +0100, Florian Westphal wrote:
> Alexandre Knecht <knecht.alexandre@gmail.com> wrote:
>  Export/import scenario (handles are metadata):
> >   {"add": {"rule": {"family": "inet", "table": "test", "chain": "c",
> >                     "handle": 4, "expr": [...]}}}
> >   → Handle 4 is ignored, rule appended
> > 
> > Explicit positioning:
> >   {"add": {"rule": {"family": "inet", "table": "test", "chain": "c",
> >                     "position": 2, "expr": [...]}}}
> >   → Rule added after handle 2
> > 
> > What do you think about this approach? I can implement it if you agree it's
> > the right direction.
> 
> I think its a sensible strategy, yes.
> 
> Reactivating the handle is a no-go as it will break existing cases.

Though libnftables-json.5 has this description of rule's "handle"
property:

"The rule’s handle. In delete/replace commands, it serves as an
identifier of the rule to delete/replace. In add/insert commands, it
serves as an identifier of an existing rule to append/prepend the rule
to."

So one might declare lack of handle support in "add" commands a bug and
the use-cases depending on it as broken.

We must not stop ignoring handles in implicit add commands as that
breaks 'nft -j list ruleset | nft -j -f -' but we can distinguish
those.

> Could you also add a test case that validates the various relative
> positioning outcomes?
> 
> i.e. given:
> 
> rule handle 1
> rule handle 2
> rule handle 3
> 
> - check that positinging at 1 results in
> rule 1
> rule N
> rule 2
> rule 3
> 
> - check that positining at rule 1 results in
> 
> rule 1
> rule N2
> rule N
> rule 2
> rule 3
> 
> - check that positinging at rule 1 will fail
> in case that rule was already deleted.
> 
> - check that positinging at rule 2 will insert
> after handle 2 and not N2.

I'd suggest extending testcases/rule_management/0001addinsertposition_0
in tests/shell to cover the missing parts above and create a copy for
JSON syntax.

> My only question is how "Position" is treated with insert (instead of
> add).
> 
> It should NOT be ignored, it should either be rejected outright (and
> everywhere its not expected) or it should have different meaning, ie.
> prepend (insert occurs before the given handle).

With insert command, handle property is recognized and behaves identical
to standard syntax. In general, I think JSON syntax parser should
deviate as little as necessary from standard syntax one. Same name but
different function will likely confuse users. Although not documented
anymore, standard syntax still accepts "position" and treats it as
synonym to "handle".

So maybe just do this (untested):

--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -4045,7 +4045,8 @@ static struct cmd *json_parse_cmd_replace(struct json_ctx *ctx,
                return NULL;
        }
 
-       if ((op == CMD_INSERT || op == CMD_ADD) && h.handle.id) {
+       if (h.handle.id &&
+           (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE)) {
                h.position.id = h.handle.id;
                h.handle.id = 0;
        }
@@ -4328,9 +4329,9 @@ static struct cmd *json_parse_cmd(struct json_ctx *ctx, json_t *root)
                enum cmd_ops op;
                struct cmd *(*cb)(struct json_ctx *ctx, json_t *, enum cmd_ops);
        } parse_cb_table[] = {
-               { "add", CMD_ADD, json_parse_cmd_add },
+               { "add", CMD_ADD, json_parse_cmd_replace },
                { "replace", CMD_REPLACE, json_parse_cmd_replace },
-               { "create", CMD_CREATE, json_parse_cmd_add },
+               { "create", CMD_CREATE, json_parse_cmd_replace },
                { "insert", CMD_INSERT, json_parse_cmd_replace },
                { "delete", CMD_DELETE, json_parse_cmd_add },
                { "list", CMD_LIST, json_parse_cmd_list },

Cheers, Phil

Re: [nft PATCH v2] parser_json: support handle for rule positioning in JSON add rule

[Alexandre Knecht]

Hi,

Thanks for your precious feedback, I submitted the new patch with the
requested test cases. Hope it will suit you !

Link: https://patchwork.ozlabs.org/project/netfilter-devel/patch/20251106083551.137310-1-knecht.alexandre@gmail.com/

Enjoy the rest of your day !