[PATCH v5 0/3] parser_json: support handle for rule positioning

[PATCH v5 0/3] parser_json: support handle for rule positioning

[Alexandre Knecht]

This patch series enables handle-based rule positioning for JSON
add/insert commands.

Changes since v4:
- CTX_F_EXPR_MASK now uses inverse mask (UINT32_MAX & ~(...)) as
  suggested by Phil, for future-proof expression flag filtering
- Removed nested block in json_parse_cmd(), variables declared at
  function start per project style (Phil/Florian feedback)
- Test 0007: Removed redundant insert position check (covered by 0008),
  replaced grep|grep|awk with single sed call
- Test 0008: Added test for insert without handle, added test for
  multiple commands in single transaction, fixed error message typo
- Added .json-nft dump files for both tests

All JSON tests pass. check-tree.sh shows no new errors.

Alexandre Knecht (3):
  parser_json: support handle for rule positioning in explicit JSON
    format
  tests: shell: add JSON test for all object types
  tests: shell: add JSON test for handle-based rule positioning

 src/parser_json.c                             |  37 +++-
 .../json/0007add_insert_delete_objects_0      | 145 ++++++++++++++++
 .../testcases/json/0008rule_position_handle_0 | 162 ++++++++++++++++++
 .../0007add_insert_delete_objects_0.json-nft  |  18 ++
 .../dumps/0007add_insert_delete_objects_0.nft |   2 +
 .../dumps/0008rule_position_handle_0.json-nft |  76 ++++++++
 .../json/dumps/0008rule_position_handle_0.nft |   6 +
 7 files changed, 443 insertions(+), 3 deletions(-)
 create mode 100755 tests/shell/testcases/json/0007add_insert_delete_objects_0
 create mode 100755 tests/shell/testcases/json/0008rule_position_handle_0
 create mode 100644 tests/shell/testcases/json/dumps/0007add_insert_delete_objects_0.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/0007add_insert_delete_objects_0.nft
 create mode 100644 tests/shell/testcases/json/dumps/0008rule_position_handle_0.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/0008rule_position_handle_0.nft

-- 
2.51.1

[PATCH v5 1/3] parser_json: support handle for rule positioning in explicit JSON format

[Alexandre Knecht]

This patch enables handle-based rule positioning for JSON add/insert
commands by using a context flag to distinguish between explicit and
implicit command formats.

When processing JSON:
- Explicit commands like {"add": {"rule": ...}} set no flag, allowing
  handle fields to be converted to position for rule placement
- Implicit format (bare objects like {"rule": ...}, used in export/import)
  sets CTX_F_IMPLICIT flag, causing handles to be ignored for portability

This approach ensures that:
- Explicit rule adds with handles work for positioning
- Non-rule objects (tables, chains, sets, etc.) are unaffected
- Export/import remains compatible (handles ignored)

The semantics for explicit rule commands are:
  ADD with handle:    inserts rule AFTER the specified handle
  INSERT with handle: inserts rule BEFORE the specified handle

Implementation details:
- CTX_F_IMPLICIT flag (bit 10) marks implicit add commands
- CTX_F_EXPR_MASK uses inverse mask for future-proof expression flag filtering
- Handle-to-position conversion in json_parse_cmd_add_rule()
- Variables declared at function start per project style

Link: https://patchwork.ozlabs.org/project/netfilter-devel/patch/20251029224530.1962783-2-knecht.alexandre@gmail.com/
Suggested-by: Phil Sutter <phil@nwl.cc>
Suggested-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Alexandre Knecht <knecht.alexandre@gmail.com>
---
 src/parser_json.c | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/src/parser_json.c b/src/parser_json.c
index 7b4f3384..87266de6 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -51,6 +51,10 @@
 #define CTX_F_MAP	(1 << 7)	/* LHS of map_expr */
 #define CTX_F_CONCAT	(1 << 8)	/* inside concat_expr */
 #define CTX_F_COLLAPSED	(1 << 9)
+#define CTX_F_IMPLICIT	(1 << 10)	/* implicit add (export/import format) */
+
+/* Mask for flags that affect expression parsing context (all except command-level flags) */
+#define CTX_F_EXPR_MASK	(UINT32_MAX & ~(CTX_F_COLLAPSED | CTX_F_IMPLICIT))
 
 struct json_ctx {
 	struct nft_ctx *nft;
@@ -1725,10 +1729,14 @@ static struct expr *json_parse_expr(struct json_ctx *ctx, json_t *root)
 		return NULL;
 
 	for (i = 0; i < array_size(cb_tbl); i++) {
+		uint32_t expr_flags;
+
 		if (strcmp(type, cb_tbl[i].name))
 			continue;
 
-		if ((cb_tbl[i].flags & ctx->flags) != ctx->flags) {
+		/* Only check expression context flags, not command-level flags */
+		expr_flags = ctx->flags & CTX_F_EXPR_MASK;
+		if ((cb_tbl[i].flags & expr_flags) != expr_flags) {
 			json_error(ctx, "Expression type %s not allowed in context (%s).",
 				   type, ctx_flags_to_string(ctx));
 			return NULL;
@@ -3201,6 +3209,18 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root,
 		h.index.id++;
 	}
 
+	/* For explicit add/insert/create commands, handle is used for positioning.
+	 * Convert handle to position for proper rule placement.
+	 * Skip this for implicit adds (export/import format).
+	 */
+	if (!(ctx->flags & CTX_F_IMPLICIT) &&
+	    !json_unpack(root, "{s:I}", "handle", &h.handle.id)) {
+		if (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE) {
+			h.position.id = h.handle.id;
+			h.handle.id = 0;
+		}
+	}
+
 	rule = rule_alloc(int_loc, NULL);
 
 	json_unpack(root, "{s:s}", "comment", &comment);
@@ -4344,6 +4364,8 @@ static struct cmd *json_parse_cmd(struct json_ctx *ctx, json_t *root)
 	};
 	unsigned int i;
 	json_t *tmp;
+	uint32_t old_flags;
+	struct cmd *cmd;
 
 	for (i = 0; i < array_size(parse_cb_table); i++) {
 		tmp = json_object_get(root, parse_cb_table[i].key);
@@ -4352,8 +4374,17 @@ static struct cmd *json_parse_cmd(struct json_ctx *ctx, json_t *root)
 
 		return parse_cb_table[i].cb(ctx, tmp, parse_cb_table[i].op);
 	}
-	/* to accept 'list ruleset' output 1:1, try add command */
-	return json_parse_cmd_add(ctx, root, CMD_ADD);
+	/* to accept 'list ruleset' output 1:1, try add command
+	 * Mark as implicit to distinguish from explicit add commands.
+	 * This allows explicit {"add": {"rule": ...}} to use handle for positioning
+	 * while implicit {"rule": ...} (export format) ignores handles.
+	 */
+	old_flags = ctx->flags;
+	ctx->flags |= CTX_F_IMPLICIT;
+	cmd = json_parse_cmd_add(ctx, root, CMD_ADD);
+	ctx->flags = old_flags;
+
+	return cmd;
 }
 
 static int json_verify_metainfo(struct json_ctx *ctx, json_t *root)
-- 
2.51.1

[PATCH v5 2/3] tests: shell: add JSON test for all object types

[Alexandre Knecht]

Add comprehensive test for JSON add/insert/delete/replace/create
operations on all object types to ensure the handle field changes
don't break non-rule objects.

Tests coverage:
- ADD operations: table, chain, rule, set, counter, quota
- INSERT operations: rule positioning
- REPLACE operations: rule modification
- CREATE operations: table creation with conflict detection
- DELETE operations: rule, set, chain, table

The test verifies that all object types work correctly with JSON
commands and validates intermediate states. Final state is an empty
table from the CREATE test.

Signed-off-by: Alexandre Knecht <knecht.alexandre@gmail.com>
---
 .../json/0007add_insert_delete_objects_0      | 145 ++++++++++++++++++
 .../0007add_insert_delete_objects_0.json-nft  |  18 +++
 .../dumps/0007add_insert_delete_objects_0.nft |   2 +
 3 files changed, 165 insertions(+)
 create mode 100755 tests/shell/testcases/json/0007add_insert_delete_objects_0
 create mode 100644 tests/shell/testcases/json/dumps/0007add_insert_delete_objects_0.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/0007add_insert_delete_objects_0.nft

diff --git a/tests/shell/testcases/json/0007add_insert_delete_objects_0 b/tests/shell/testcases/json/0007add_insert_delete_objects_0
new file mode 100755
index 00000000..f701b062
--- /dev/null
+++ b/tests/shell/testcases/json/0007add_insert_delete_objects_0
@@ -0,0 +1,145 @@
+#!/bin/bash
+
+# NFT_TEST_REQUIRES(NFT_TEST_HAVE_json)
+
+# Comprehensive test for JSON add/insert/delete/replace operations
+# Tests that all object types work correctly with JSON commands
+
+set -e
+
+$NFT flush ruleset
+
+# ===== ADD operations =====
+
+echo "Test 1: Add table"
+$NFT -j -f - << 'EOF'
+{"nftables": [{"add": {"table": {"family": "inet", "name": "test"}}}]}
+EOF
+
+echo "Test 2: Add chain"
+$NFT -j -f - << 'EOF'
+{"nftables": [{"add": {"chain": {"family": "inet", "table": "test", "name": "input_chain", "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}}]}
+EOF
+
+echo "Test 3: Add rule"
+$NFT -j -f - << 'EOF'
+{"nftables": [{"add": {"rule": {"family": "inet", "table": "test", "chain": "input_chain", "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 22}}, {"accept": null}]}}}]}
+EOF
+
+echo "Test 4: Add set"
+$NFT -j -f - << 'EOF'
+{"nftables": [{"add": {"set": {"family": "inet", "table": "test", "name": "test_set", "type": "ipv4_addr"}}}]}
+EOF
+
+echo "Test 5: Add counter"
+$NFT -j -f - << 'EOF'
+{"nftables": [{"add": {"counter": {"family": "inet", "table": "test", "name": "test_counter"}}}]}
+EOF
+
+echo "Test 6: Add quota"
+$NFT -j -f - << 'EOF'
+{"nftables": [{"add": {"quota": {"family": "inet", "table": "test", "name": "test_quota", "bytes": 1000000}}}]}
+EOF
+
+# Verify all objects were created
+$NFT list ruleset > /dev/null || { echo "Failed to list ruleset after add operations"; exit 1; }
+
+# ===== REPLACE operations =====
+
+echo "Test 7: Replace rule"
+# Get handle of rule with dport 22
+HANDLE=$($NFT -a list chain inet test input_chain | sed -n 's/.*tcp dport 22 .* handle \([0-9]\+\)/\1/p')
+if [ -z "$HANDLE" ]; then
+	echo "Test 7 failed: could not find rule handle"
+	exit 1
+fi
+
+$NFT -j -f - << EOF
+{"nftables": [{"replace": {"rule": {"family": "inet", "table": "test", "chain": "input_chain", "handle": $HANDLE, "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 443}}, {"accept": null}]}}}]}
+EOF
+
+# Verify rule was replaced
+if ! $NFT list chain inet test input_chain | grep -q "tcp dport 443"; then
+	echo "Test 7 failed: rule not replaced correctly"
+	exit 1
+fi
+if $NFT list chain inet test input_chain | grep -q "tcp dport 22"; then
+	echo "Test 7 failed: old rule still exists"
+	exit 1
+fi
+
+# ===== CREATE operations =====
+
+echo "Test 8: Create table (should work like add)"
+$NFT -j -f - << 'EOF'
+{"nftables": [{"create": {"table": {"family": "ip", "name": "created_table"}}}]}
+EOF
+
+if ! $NFT list tables | grep -q "created_table"; then
+	echo "Test 8 failed: table not created"
+	exit 1
+fi
+
+echo "Test 9: Create table that exists (should fail)"
+if $NFT -j -f - 2>/dev/null << 'EOF'
+{"nftables": [{"create": {"table": {"family": "ip", "name": "created_table"}}}]}
+EOF
+then
+	echo "Test 9 failed: create should have failed for existing table"
+	exit 1
+fi
+
+# ===== DELETE operations =====
+
+echo "Test 10: Delete rule"
+HANDLE=$($NFT -a list chain inet test input_chain | sed -n 's/.*tcp dport 443 .* handle \([0-9]\+\)/\1/p')
+$NFT -j -f - << EOF
+{"nftables": [{"delete": {"rule": {"family": "inet", "table": "test", "chain": "input_chain", "handle": $HANDLE}}}]}
+EOF
+
+if $NFT list chain inet test input_chain | grep -q "tcp dport 443"; then
+	echo "Test 10 failed: rule not deleted"
+	exit 1
+fi
+
+echo "Test 11: Delete counter"
+$NFT -j -f - << 'EOF'
+{"nftables": [{"delete": {"counter": {"family": "inet", "table": "test", "name": "test_counter"}}}]}
+EOF
+
+if $NFT list counters | grep -q "test_counter"; then
+	echo "Test 11 failed: counter not deleted"
+	exit 1
+fi
+
+echo "Test 12: Delete set"
+$NFT -j -f - << 'EOF'
+{"nftables": [{"delete": {"set": {"family": "inet", "table": "test", "name": "test_set"}}}]}
+EOF
+
+if $NFT list sets | grep -q "test_set"; then
+	echo "Test 12 failed: set not deleted"
+	exit 1
+fi
+
+echo "Test 13: Delete chain"
+$NFT -j -f - << 'EOF'
+{"nftables": [{"delete": {"chain": {"family": "inet", "table": "test", "name": "input_chain"}}}]}
+EOF
+
+if $NFT list chains | grep -q "input_chain"; then
+	echo "Test 13 failed: chain not deleted"
+	exit 1
+fi
+
+echo "Test 14: Delete table"
+$NFT -j -f - << 'EOF'
+{"nftables": [{"delete": {"table": {"family": "inet", "name": "test"}}}]}
+EOF
+
+if $NFT list tables | grep -q "table inet test"; then
+	echo "Test 14 failed: table not deleted"
+	exit 1
+fi
+
+echo "All tests passed!"
diff --git a/tests/shell/testcases/json/dumps/0007add_insert_delete_objects_0.json-nft b/tests/shell/testcases/json/dumps/0007add_insert_delete_objects_0.json-nft
new file mode 100644
index 00000000..f449da30
--- /dev/null
+++ b/tests/shell/testcases/json/dumps/0007add_insert_delete_objects_0.json-nft
@@ -0,0 +1,18 @@
+{
+  "nftables": [
+    {
+      "metainfo": {
+        "version": "VERSION",
+        "release_name": "RELEASE_NAME",
+        "json_schema_version": 1
+      }
+    },
+    {
+      "table": {
+        "family": "ip",
+        "name": "created_table",
+        "handle": 0
+      }
+    }
+  ]
+}
diff --git a/tests/shell/testcases/json/dumps/0007add_insert_delete_objects_0.nft b/tests/shell/testcases/json/dumps/0007add_insert_delete_objects_0.nft
new file mode 100644
index 00000000..1d9aecf1
--- /dev/null
+++ b/tests/shell/testcases/json/dumps/0007add_insert_delete_objects_0.nft
@@ -0,0 +1,2 @@
+table ip created_table {
+}
-- 
2.51.1

[PATCH v5 3/3] tests: shell: add JSON test for handle-based rule positioning

[Alexandre Knecht]

Add comprehensive test for JSON handle-based rule positioning to verify
the handle field correctly positions rules with explicit add/insert
commands while being ignored in implicit format.

Test coverage:
1. ADD with handle positions AFTER the specified handle
2. INSERT with handle positions BEFORE the specified handle
3. INSERT without handle positions at beginning
4. Multiple commands in single transaction (batch behavior)
5. Implicit format ignores handle field for portability

The test uses sed for handle extraction and nft -f format for setup
as suggested in code review. Final state is a table with two rules
from the implicit format test.

Signed-off-by: Alexandre Knecht <knecht.alexandre@gmail.com>
---
 .../testcases/json/0008rule_position_handle_0 | 162 ++++++++++++++++++
 .../dumps/0008rule_position_handle_0.json-nft |  76 ++++++++
 .../json/dumps/0008rule_position_handle_0.nft |   6 +
 3 files changed, 244 insertions(+)
 create mode 100755 tests/shell/testcases/json/0008rule_position_handle_0
 create mode 100644 tests/shell/testcases/json/dumps/0008rule_position_handle_0.json-nft
 create mode 100644 tests/shell/testcases/json/dumps/0008rule_position_handle_0.nft

diff --git a/tests/shell/testcases/json/0008rule_position_handle_0 b/tests/shell/testcases/json/0008rule_position_handle_0
new file mode 100755
index 00000000..32a3752c
--- /dev/null
+++ b/tests/shell/testcases/json/0008rule_position_handle_0
@@ -0,0 +1,162 @@
+#!/bin/bash
+
+# NFT_TEST_REQUIRES(NFT_TEST_HAVE_json)
+
+# Test JSON handle-based rule positioning
+# Verifies explicit format uses handle for positioning while implicit format ignores it
+
+set -e
+
+$NFT flush ruleset
+
+echo "Test 1: ADD with handle positions AFTER"
+$NFT -f - <<EOF
+table inet test {
+	chain c {
+		tcp dport 22 accept
+		tcp dport 80 accept
+	}
+}
+EOF
+
+# Get handle of first rule (tcp dport 22)
+HANDLE=$($NFT -a list chain inet test c | sed -n 's/.*tcp dport 22 .* handle \([0-9]\+\)/\1/p')
+
+# Add after handle (should be between 22 and 80)
+$NFT -j -f - <<EOF
+{"nftables": [{"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "handle": $HANDLE, "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 443}}, {"accept": null}]}}}]}
+EOF
+
+# Verify order: 22, 443, 80
+RULES=$($NFT list chain inet test c | grep -o "tcp dport [0-9]*")
+EXPECTED="tcp dport 22
+tcp dport 443
+tcp dport 80"
+
+if [ "$RULES" = "$EXPECTED" ]; then
+	echo "PASS: Rule added after handle"
+else
+	echo "FAIL: Expected order 22,443,80, got:"
+	echo "$RULES"
+	exit 1
+fi
+
+echo "Test 2: INSERT with handle positions BEFORE"
+$NFT flush ruleset
+$NFT -f - <<EOF
+table inet test {
+	chain c {
+		tcp dport 22 accept
+		tcp dport 80 accept
+	}
+}
+EOF
+
+# Get handle of second rule (tcp dport 80)
+HANDLE=$($NFT -a list chain inet test c | sed -n 's/.*tcp dport 80 .* handle \([0-9]\+\)/\1/p')
+
+# Insert before handle
+$NFT -j -f - <<EOF
+{"nftables": [{"insert": {"rule": {"family": "inet", "table": "test", "chain": "c", "handle": $HANDLE, "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 443}}, {"accept": null}]}}}]}
+EOF
+
+# Verify order: 22, 443, 80
+RULES=$($NFT list chain inet test c | grep -o "tcp dport [0-9]*")
+if [ "$RULES" = "$EXPECTED" ]; then
+	echo "PASS: Rule inserted before handle"
+else
+	echo "FAIL: Expected order 22,443,80, got:"
+	echo "$RULES"
+	exit 1
+fi
+
+echo "Test 3: INSERT without handle positions at beginning"
+$NFT flush ruleset
+$NFT -f - <<EOF
+table inet test {
+	chain c {
+		tcp dport 22 accept
+		tcp dport 80 accept
+	}
+}
+EOF
+
+# Insert without handle (should go to beginning)
+$NFT -j -f - <<EOF
+{"nftables": [{"insert": {"rule": {"family": "inet", "table": "test", "chain": "c", "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 443}}, {"accept": null}]}}}]}
+EOF
+
+# Verify order: 443, 22, 80
+RULES=$($NFT list chain inet test c | grep -o "tcp dport [0-9]*")
+EXPECTED_INSERT="tcp dport 443
+tcp dport 22
+tcp dport 80"
+
+if [ "$RULES" = "$EXPECTED_INSERT" ]; then
+	echo "PASS: Rule inserted at beginning without handle"
+else
+	echo "FAIL: Expected order 443,22,80, got:"
+	echo "$RULES"
+	exit 1
+fi
+
+echo "Test 4: Multiple commands in single transaction"
+$NFT flush ruleset
+$NFT -f - <<EOF
+table inet test {
+	chain c {
+		tcp dport 22 accept
+	}
+}
+EOF
+
+# Get handle
+HANDLE=$($NFT -a list chain inet test c | sed -n 's/.*tcp dport 22 .* handle \([0-9]\+\)/\1/p')
+
+# Add two rules after same handle in single transaction
+$NFT -j -f - <<EOF
+{"nftables": [
+	{"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "handle": $HANDLE, "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 80}}, {"accept": null}]}}},
+	{"add": {"rule": {"family": "inet", "table": "test", "chain": "c", "handle": $HANDLE, "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 443}}, {"accept": null}]}}}
+]}
+EOF
+
+# Verify: Both should be after handle 22
+# In a transaction, both position to same handle, so added in reverse order
+# Order should be: 22, then 443, then 80 (last add goes immediately after position)
+RULES=$($NFT list chain inet test c | grep -o "tcp dport [0-9]*")
+EXPECTED_MULTI="tcp dport 22
+tcp dport 443
+tcp dport 80"
+
+if [ "$RULES" = "$EXPECTED_MULTI" ]; then
+	echo "PASS: Multiple rules in transaction positioned correctly"
+else
+	echo "FAIL: Expected order 22,443,80, got:"
+	echo "$RULES"
+	exit 1
+fi
+
+echo "Test 5: Implicit format ignores handle"
+$NFT flush ruleset
+$NFT -f - <<EOF
+table inet test {
+	chain c {
+		tcp dport 22 accept
+	}
+}
+EOF
+
+# Implicit format with non-existent handle should succeed (handle ignored)
+$NFT -j -f - <<EOF
+{"nftables": [{"rule": {"family": "inet", "table": "test", "chain": "c", "handle": 9999, "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 80}}, {"accept": null}]}}]}
+EOF
+
+if $NFT list chain inet test c | grep -q "tcp dport 80"; then
+	echo "PASS: Implicit format ignores handle"
+else
+	echo "FAIL: Implicit format should have added rule despite non-existent handle"
+	exit 1
+fi
+
+echo "All positioning tests passed!"
diff --git a/tests/shell/testcases/json/dumps/0008rule_position_handle_0.json-nft b/tests/shell/testcases/json/dumps/0008rule_position_handle_0.json-nft
new file mode 100644
index 00000000..62101fbb
--- /dev/null
+++ b/tests/shell/testcases/json/dumps/0008rule_position_handle_0.json-nft
@@ -0,0 +1,76 @@
+{
+  "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": 22
+            }
+          },
+          {
+            "accept": null
+          }
+        ]
+      }
+    },
+    {
+      "rule": {
+        "family": "inet",
+        "table": "test",
+        "chain": "c",
+        "handle": 0,
+        "expr": [
+          {
+            "match": {
+              "op": "==",
+              "left": {
+                "payload": {
+                  "protocol": "tcp",
+                  "field": "dport"
+                }
+              },
+              "right": 80
+            }
+          },
+          {
+            "accept": null
+          }
+        ]
+      }
+    }
+  ]
+}
diff --git a/tests/shell/testcases/json/dumps/0008rule_position_handle_0.nft b/tests/shell/testcases/json/dumps/0008rule_position_handle_0.nft
new file mode 100644
index 00000000..d222ad64
--- /dev/null
+++ b/tests/shell/testcases/json/dumps/0008rule_position_handle_0.nft
@@ -0,0 +1,6 @@
+table inet test {
+	chain c {
+		tcp dport 22 accept
+		tcp dport 80 accept
+	}
+}
-- 
2.51.1

Re: [PATCH v5 1/3] parser_json: support handle for rule positioning in explicit JSON format

[Phil Sutter]

Hi Alexandre,

On Mon, Jan 19, 2026 at 03:08:11PM +0100, Alexandre Knecht wrote:
> Implementation details:
> - CTX_F_IMPLICIT flag (bit 10) marks implicit add commands
> - CTX_F_EXPR_MASK uses inverse mask for future-proof expression flag filtering
> - Handle-to-position conversion in json_parse_cmd_add_rule()
> - Variables declared at function start per project style

Thanks for your follow-up, just two nits:

[...]
> diff --git a/src/parser_json.c b/src/parser_json.c
> index 7b4f3384..87266de6 100644
> --- a/src/parser_json.c
> +++ b/src/parser_json.c
[...]
> @@ -3201,6 +3209,18 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root,
>  		h.index.id++;
>  	}
>  
> +	/* For explicit add/insert/create commands, handle is used for positioning.
> +	 * Convert handle to position for proper rule placement.
> +	 * Skip this for implicit adds (export/import format).
> +	 */
> +	if (!(ctx->flags & CTX_F_IMPLICIT) &&
> +	    !json_unpack(root, "{s:I}", "handle", &h.handle.id)) {
> +		if (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE) {
> +			h.position.id = h.handle.id;
> +			h.handle.id = 0;
> +		}
> +	}

Please merge the nested if-conditionals. I suggest sorting expressions
from cheap to expensive:

|	if (!(ctx->flags & CTX_F_IMPLICIT) &&
|	    (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE) &&
|	    !json_unpack(root, "{s:I}", "handle", &h.handle.id)) {

[...]
> @@ -4344,6 +4364,8 @@ static struct cmd *json_parse_cmd(struct json_ctx *ctx, json_t *root)
>  	};
>  	unsigned int i;
>  	json_t *tmp;
> +	uint32_t old_flags;
> +	struct cmd *cmd;

Please use Reverse Christmas Tree notation, i.e. reverse-sort variable
definitions by line length.

Thanks, Phil

Re: [PATCH v5 1/3] parser_json: support handle for rule positioning in explicit JSON format

[Alexandre Knecht]

Hi Phil,

Thanks for the comment, that's pretty straightforward to fix, I'm
afraid to do a lot of spamming if I post again a new series, so can
you confirm this is what you expect ?

Merged nested if-conditionals (cheap to expensive):
- if (!(ctx->flags & CTX_F_IMPLICIT) &&
- !json_unpack(root, "{s:I}", "handle", &h.handle.id)) {
- if (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE) {
- h.position.id = h.handle.id;
- h.handle.id = 0;
- }
- }
+ if (!(ctx->flags & CTX_F_IMPLICIT) &&
+   (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE) &&
+   !json_unpack(root, "{s:I}", "handle", &h.handle.id)) {
+     h.position.id = h.handle.id;
+     h.handle.id = 0;
+ }

Reverse Christmas Tree variable declarations:
- unsigned int i;
- json_t *tmp;
uint32_t old_flags;
struct cmd *cmd;
+ unsigned int i;
+ json_t *tmp;

Or maybe there's a solution to amend this series, not kinda used to
work with git send-email, so if I can resubmit without a new whole
series, could be good ! Otherwise, I'll just create a new one once you
confirm.

Maybe I'll wait for review on tests too before submitting everything again.

Enjoy your day !

Alex

Le mar. 20 janv. 2026 à 15:08, Phil Sutter <phil@nwl.cc> a écrit :
>
> Hi Alexandre,
>
> On Mon, Jan 19, 2026 at 03:08:11PM +0100, Alexandre Knecht wrote:
> > Implementation details:
> > - CTX_F_IMPLICIT flag (bit 10) marks implicit add commands
> > - CTX_F_EXPR_MASK uses inverse mask for future-proof expression flag filtering
> > - Handle-to-position conversion in json_parse_cmd_add_rule()
> > - Variables declared at function start per project style
>
> Thanks for your follow-up, just two nits:
>
> [...]
> > diff --git a/src/parser_json.c b/src/parser_json.c
> > index 7b4f3384..87266de6 100644
> > --- a/src/parser_json.c
> > +++ b/src/parser_json.c
> [...]
> > @@ -3201,6 +3209,18 @@ static struct cmd *json_parse_cmd_add_rule(struct json_ctx *ctx, json_t *root,
> >               h.index.id++;
> >       }
> >
> > +     /* For explicit add/insert/create commands, handle is used for positioning.
> > +      * Convert handle to position for proper rule placement.
> > +      * Skip this for implicit adds (export/import format).
> > +      */
> > +     if (!(ctx->flags & CTX_F_IMPLICIT) &&
> > +         !json_unpack(root, "{s:I}", "handle", &h.handle.id)) {
> > +             if (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE) {
> > +                     h.position.id = h.handle.id;
> > +                     h.handle.id = 0;
> > +             }
> > +     }
>
> Please merge the nested if-conditionals. I suggest sorting expressions
> from cheap to expensive:
>
> |       if (!(ctx->flags & CTX_F_IMPLICIT) &&
> |           (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE) &&
> |           !json_unpack(root, "{s:I}", "handle", &h.handle.id)) {
>
> [...]
> > @@ -4344,6 +4364,8 @@ static struct cmd *json_parse_cmd(struct json_ctx *ctx, json_t *root)
> >       };
> >       unsigned int i;
> >       json_t *tmp;
> > +     uint32_t old_flags;
> > +     struct cmd *cmd;
>
> Please use Reverse Christmas Tree notation, i.e. reverse-sort variable
> definitions by line length.
>
> Thanks, Phil

Re: [PATCH v5 2/3] tests: shell: add JSON test for all object types

[Phil Sutter]

On Mon, Jan 19, 2026 at 03:08:12PM +0100, Alexandre Knecht wrote:
[...]
> diff --git a/tests/shell/testcases/json/0007add_insert_delete_objects_0 b/tests/shell/testcases/json/0007add_insert_delete_objects_0
> new file mode 100755
> index 00000000..f701b062
> --- /dev/null
> +++ b/tests/shell/testcases/json/0007add_insert_delete_objects_0
> @@ -0,0 +1,145 @@
> +#!/bin/bash
> +
> +# NFT_TEST_REQUIRES(NFT_TEST_HAVE_json)
> +
> +# Comprehensive test for JSON add/insert/delete/replace operations
> +# Tests that all object types work correctly with JSON commands
> +
> +set -e
> +
> +$NFT flush ruleset
> +
> +# ===== ADD operations =====
> +
> +echo "Test 1: Add table"
> +$NFT -j -f - << 'EOF'
> +{"nftables": [{"add": {"table": {"family": "inet", "name": "test"}}}]}
> +EOF
> +
> +echo "Test 2: Add chain"
> +$NFT -j -f - << 'EOF'
> +{"nftables": [{"add": {"chain": {"family": "inet", "table": "test", "name": "input_chain", "type": "filter", "hook": "input", "prio": 0, "policy": "accept"}}}]}
> +EOF
> +
> +echo "Test 3: Add rule"
> +$NFT -j -f - << 'EOF'
> +{"nftables": [{"add": {"rule": {"family": "inet", "table": "test", "chain": "input_chain", "expr": [{"match": {"op": "==", "left": {"payload": {"protocol": "tcp", "field": "dport"}}, "right": 22}}, {"accept": null}]}}}]}
> +EOF
> +
> +echo "Test 4: Add set"
> +$NFT -j -f - << 'EOF'
> +{"nftables": [{"add": {"set": {"family": "inet", "table": "test", "name": "test_set", "type": "ipv4_addr"}}}]}
> +EOF
> +
> +echo "Test 5: Add counter"
> +$NFT -j -f - << 'EOF'
> +{"nftables": [{"add": {"counter": {"family": "inet", "table": "test", "name": "test_counter"}}}]}
> +EOF
> +
> +echo "Test 6: Add quota"
> +$NFT -j -f - << 'EOF'
> +{"nftables": [{"add": {"quota": {"family": "inet", "table": "test", "name": "test_quota", "bytes": 1000000}}}]}
> +EOF
> +
> +# Verify all objects were created
> +$NFT list ruleset > /dev/null || { echo "Failed to list ruleset after add operations"; exit 1; }

The command will succeed even if not a single one of the previous add
commands succeeded. To compare the resulting ruleset against
expectations, use a diff like so:

| EXPECT='table inet test {
| 	counter test_counter {
| 	}
| 	quota test_quota {
| 		bytes 1000000
| 	}
| 	set test_set {
| 		type ipv4_addr
| 	}
| 	chain input_chain {
| 		type filter hook input priority filter; policy accept;
| 
| 		tcp dport 22 accept
| 	}
| }'
| $DIFF -u <(echo "$EXPECT") <($NFT list ruleset) || { ... }

Otherwise looks good to me!

Thanks, Phil

Re: [PATCH v5 3/3] tests: shell: add JSON test for handle-based rule positioning

[Phil Sutter]

On Mon, Jan 19, 2026 at 03:08:13PM +0100, Alexandre Knecht wrote:
> Add comprehensive test for JSON handle-based rule positioning to verify
> the handle field correctly positions rules with explicit add/insert
> commands while being ignored in implicit format.
> 
> Test coverage:
> 1. ADD with handle positions AFTER the specified handle
> 2. INSERT with handle positions BEFORE the specified handle
> 3. INSERT without handle positions at beginning
> 4. Multiple commands in single transaction (batch behavior)
> 5. Implicit format ignores handle field for portability
> 
> The test uses sed for handle extraction and nft -f format for setup
> as suggested in code review. Final state is a table with two rules
> from the implicit format test.
> 
> Signed-off-by: Alexandre Knecht <knecht.alexandre@gmail.com>

Acked-by: Phil Sutter <phil@nwl.cc>

Re: [PATCH v5 1/3] parser_json: support handle for rule positioning in explicit JSON format

[Phil Sutter]

Hi Alexandre,

On Tue, Jan 20, 2026 at 03:27:58PM +0100, Alexandre Knecht wrote:
> Hi Phil,
> 
> Thanks for the comment, that's pretty straightforward to fix, I'm
> afraid to do a lot of spamming if I post again a new series, so can
> you confirm this is what you expect ?
> 
> Merged nested if-conditionals (cheap to expensive):
> - if (!(ctx->flags & CTX_F_IMPLICIT) &&
> - !json_unpack(root, "{s:I}", "handle", &h.handle.id)) {
> - if (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE) {
> - h.position.id = h.handle.id;
> - h.handle.id = 0;
> - }
> - }
> + if (!(ctx->flags & CTX_F_IMPLICIT) &&
> +   (op == CMD_INSERT || op == CMD_ADD || op == CMD_CREATE) &&
> +   !json_unpack(root, "{s:I}", "handle", &h.handle.id)) {
> +     h.position.id = h.handle.id;
> +     h.handle.id = 0;
> + }

Yes, this looks correct!

> Reverse Christmas Tree variable declarations:
> - unsigned int i;
> - json_t *tmp;
> uint32_t old_flags;
> struct cmd *cmd;
> + unsigned int i;
> + json_t *tmp;

Also correct AFAICT.

> Or maybe there's a solution to amend this series, not kinda used to
> work with git send-email, so if I can resubmit without a new whole
> series, could be good ! Otherwise, I'll just create a new one once you
> confirm.

I can apply trivial changes to patches when applying them. With some
projects, people also just resubmit parts of the series - this causes a
bit of a mess for maintainers (and reviewers) though so it's not usually
done. With a small series like this, I would just resubmit the whole
thing. After all, becoming more familiar with git-send-email and the
whole process of amending changes and submitting a new version is very
good practice for contributing to OSS projects! And please don't forget,
we're here to help if you get stuck or don't know how to get started
with something.

> Maybe I'll wait for review on tests too before submitting everything again.

Just finished, only one more change in second patch requested.

Thanks, Phil