* [nft PATCH v2 1/5] json: Accept more than two operands in binary expressions
2024-03-22 16:06 [nft PATCH v2 0/5] json: Accept more than two operands in binary expressions Phil Sutter
@ 2024-03-22 16:06 ` Phil Sutter
2024-03-22 16:06 ` [nft PATCH v2 2/5] mergesort: Avoid accidental set element reordering Phil Sutter
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Phil Sutter @ 2024-03-22 16:06 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
The most common use case is ORing flags like
| syn | ack | rst
but nft seems to be fine with less intuitive stuff like
| meta mark set ip dscp << 2 << 3
so support all of them.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
doc/libnftables-json.adoc | 18 +--
src/json.c | 19 +++-
src/parser_json.c | 12 ++
tests/py/inet/tcp.t.json | 67 +----------
tests/py/inet/tcp.t.json.output | 104 ++++--------------
.../dumps/0012different_defines_0.json-nft | 8 +-
.../sets/dumps/0055tcpflags_0.json-nft | 98 ++++-------------
7 files changed, 91 insertions(+), 235 deletions(-)
diff --git a/doc/libnftables-json.adoc b/doc/libnftables-json.adoc
index a4adcde2a66a9..a8a6165fde59d 100644
--- a/doc/libnftables-json.adoc
+++ b/doc/libnftables-json.adoc
@@ -1352,15 +1352,17 @@ Perform kernel Forwarding Information Base lookups.
=== BINARY OPERATION
[verse]
-*{ "|": [* 'EXPRESSION'*,* 'EXPRESSION' *] }*
-*{ "^": [* 'EXPRESSION'*,* 'EXPRESSION' *] }*
-*{ "&": [* 'EXPRESSION'*,* 'EXPRESSION' *] }*
-*{ "+<<+": [* 'EXPRESSION'*,* 'EXPRESSION' *] }*
-*{ ">>": [* 'EXPRESSION'*,* 'EXPRESSION' *] }*
-
-All binary operations expect an array of exactly two expressions, of which the
+*{ "|": [* 'EXPRESSION'*,* 'EXPRESSIONS' *] }*
+*{ "^": [* 'EXPRESSION'*,* 'EXPRESSIONS' *] }*
+*{ "&": [* 'EXPRESSION'*,* 'EXPRESSIONS' *] }*
+*{ "+<<+": [* 'EXPRESSION'*,* 'EXPRESSIONS' *] }*
+*{ ">>": [* 'EXPRESSION'*,* 'EXPRESSIONS' *] }*
+'EXPRESSIONS' := 'EXPRESSION' | 'EXPRESSION'*,* 'EXPRESSIONS'
+
+All binary operations expect an array of at least two expressions, of which the
first element denotes the left hand side and the second one the right hand
-side.
+side. Extra elements are accepted in the given array and appended to the term
+accordingly.
=== VERDICT
[verse]
diff --git a/src/json.c b/src/json.c
index 29fbd0cfdba28..3753017169930 100644
--- a/src/json.c
+++ b/src/json.c
@@ -540,11 +540,24 @@ json_t *flagcmp_expr_json(const struct expr *expr, struct output_ctx *octx)
"right", expr_print_json(expr->flagcmp.value, octx));
}
+static json_t *
+__binop_expr_json(int op, const struct expr *expr, struct output_ctx *octx)
+{
+ json_t *a = json_array();
+
+ if (expr->etype == EXPR_BINOP && expr->op == op) {
+ json_array_extend(a, __binop_expr_json(op, expr->left, octx));
+ json_array_extend(a, __binop_expr_json(op, expr->right, octx));
+ } else {
+ json_array_append_new(a, expr_print_json(expr, octx));
+ }
+ return a;
+}
+
json_t *binop_expr_json(const struct expr *expr, struct output_ctx *octx)
{
- return json_pack("{s:[o, o]}", expr_op_symbols[expr->op],
- expr_print_json(expr->left, octx),
- expr_print_json(expr->right, octx));
+ return json_pack("{s:o}", expr_op_symbols[expr->op],
+ __binop_expr_json(expr->op, expr, octx));
}
json_t *relational_expr_json(const struct expr *expr, struct output_ctx *octx)
diff --git a/src/parser_json.c b/src/parser_json.c
index 04255688ca04c..55d65c415bf5c 100644
--- a/src/parser_json.c
+++ b/src/parser_json.c
@@ -1204,6 +1204,18 @@ static struct expr *json_parse_binop_expr(struct json_ctx *ctx,
return NULL;
}
+ if (json_array_size(root) > 2) {
+ left = json_parse_primary_expr(ctx, json_array_get(root, 0));
+ right = json_parse_primary_expr(ctx, json_array_get(root, 1));
+ left = binop_expr_alloc(int_loc, thisop, left, right);
+ for (i = 2; i < json_array_size(root); i++) {
+ jright = json_array_get(root, i);
+ right = json_parse_primary_expr(ctx, jright);
+ left = binop_expr_alloc(int_loc, thisop, left, right);
+ }
+ return left;
+ }
+
if (json_unpack_err(ctx, root, "[o, o!]", &jleft, &jright))
return NULL;
diff --git a/tests/py/inet/tcp.t.json b/tests/py/inet/tcp.t.json
index d3a846cf9a400..bd589cf0091fe 100644
--- a/tests/py/inet/tcp.t.json
+++ b/tests/py/inet/tcp.t.json
@@ -954,12 +954,12 @@
}
},
{
- "|": [ "fin", { "|": [ "syn", { "|": [ "rst", { "|": [ "psh", { "|": [ "ack", { "|": [ "urg", { "|": [ "ecn", "cwr" ] } ] } ] } ] } ] } ] } ]
+ "|": [ "fin", "syn", "rst", "psh", "ack", "urg", "ecn", "cwr" ]
}
]
},
"op": "==",
- "right": { "|": [ "fin", { "|": [ "syn", { "|": [ "rst", { "|": [ "psh", { "|": [ "ack", { "|": [ "urg", { "|": [ "ecn", "cwr" ] } ] } ] } ] } ] } ] } ] }
+ "right": { "|": [ "fin", "syn", "rst", "psh", "ack", "urg", "ecn", "cwr" ] }
}
}
]
@@ -1395,55 +1395,15 @@
"protocol": "tcp"
}
},
- {
- "|": [
- {
- "|": [
- {
- "|": [
- {
- "|": [
- {
- "|": [
- "fin",
- "syn"
- ]
- },
- "rst"
- ]
- },
- "psh"
- ]
- },
- "ack"
- ]
- },
- "urg"
- ]
- }
+ { "|": [ "fin", "syn", "rst", "psh", "ack", "urg" ] }
]
},
"op": "==",
"right": {
"set": [
- {
- "|": [
- {
- "|": [
- "fin",
- "psh"
- ]
- },
- "ack"
- ]
- },
+ { "|": [ "fin", "psh", "ack" ] },
"fin",
- {
- "|": [
- "psh",
- "ack"
- ]
- },
+ { "|": [ "psh", "ack" ] },
"ack"
]
}
@@ -1780,22 +1740,7 @@
"protocol": "tcp"
}
},
- {
- "|": [
- {
- "|": [
- {
- "|": [
- "fin",
- "syn"
- ]
- },
- "rst"
- ]
- },
- "ack"
- ]
- }
+ { "|": [ "fin", "syn", "rst", "ack" ] }
]
},
"op": "!=",
diff --git a/tests/py/inet/tcp.t.json.output b/tests/py/inet/tcp.t.json.output
index e186e127fd671..3f03c0ddd1586 100644
--- a/tests/py/inet/tcp.t.json.output
+++ b/tests/py/inet/tcp.t.json.output
@@ -155,27 +155,11 @@
},
{
"|": [
- {
- "|": [
- {
- "|": [
- {
- "|": [
- {
- "|": [
- "fin",
- "syn"
- ]
- },
- "rst"
- ]
- },
- "psh"
- ]
- },
- "ack"
- ]
- },
+ "fin",
+ "syn",
+ "rst",
+ "psh",
+ "ack",
"urg"
]
}
@@ -187,12 +171,8 @@
"fin",
{
"|": [
- {
- "|": [
- "fin",
- "psh"
- ]
- },
+ "fin",
+ "psh",
"ack"
]
},
@@ -280,17 +260,9 @@
},
{
"|": [
- {
- "|": [
- {
- "|": [
- "fin",
- "syn"
- ]
- },
- "rst"
- ]
- },
+ "fin",
+ "syn",
+ "rst",
"ack"
]
}
@@ -316,17 +288,9 @@
},
{
"|": [
- {
- "|": [
- {
- "|": [
- "fin",
- "syn"
- ]
- },
- "rst"
- ]
- },
+ "fin",
+ "syn",
+ "rst",
"ack"
]
}
@@ -352,17 +316,9 @@
},
{
"|": [
- {
- "|": [
- {
- "|": [
- "fin",
- "syn"
- ]
- },
- "rst"
- ]
- },
+ "fin",
+ "syn",
+ "rst",
"ack"
]
}
@@ -388,17 +344,9 @@
},
{
"|": [
- {
- "|": [
- {
- "|": [
- "fin",
- "syn"
- ]
- },
- "rst"
- ]
- },
+ "fin",
+ "syn",
+ "rst",
"ack"
]
}
@@ -429,17 +377,9 @@
},
{
"|": [
- {
- "|": [
- {
- "|": [
- "fin",
- "syn"
- ]
- },
- "rst"
- ]
- },
+ "fin",
+ "syn",
+ "rst",
"ack"
]
}
diff --git a/tests/shell/testcases/nft-f/dumps/0012different_defines_0.json-nft b/tests/shell/testcases/nft-f/dumps/0012different_defines_0.json-nft
index 8f3f3a81a9bc8..1b2e342047f4b 100644
--- a/tests/shell/testcases/nft-f/dumps/0012different_defines_0.json-nft
+++ b/tests/shell/testcases/nft-f/dumps/0012different_defines_0.json-nft
@@ -169,12 +169,8 @@
},
"right": {
"|": [
- {
- "|": [
- "established",
- "related"
- ]
- },
+ "established",
+ "related",
"new"
]
}
diff --git a/tests/shell/testcases/sets/dumps/0055tcpflags_0.json-nft b/tests/shell/testcases/sets/dumps/0055tcpflags_0.json-nft
index cd39f0909e120..6a3511515f785 100644
--- a/tests/shell/testcases/sets/dumps/0055tcpflags_0.json-nft
+++ b/tests/shell/testcases/sets/dumps/0055tcpflags_0.json-nft
@@ -27,39 +27,23 @@
"elem": [
{
"|": [
- {
- "|": [
- {
- "|": [
- "fin",
- "psh"
- ]
- },
- "ack"
- ]
- },
+ "fin",
+ "psh",
+ "ack",
"urg"
]
},
{
"|": [
- {
- "|": [
- "fin",
- "psh"
- ]
- },
+ "fin",
+ "psh",
"ack"
]
},
{
"|": [
- {
- "|": [
- "fin",
- "ack"
- ]
- },
+ "fin",
+ "ack",
"urg"
]
},
@@ -71,39 +55,23 @@
},
{
"|": [
- {
- "|": [
- {
- "|": [
- "syn",
- "psh"
- ]
- },
- "ack"
- ]
- },
+ "syn",
+ "psh",
+ "ack",
"urg"
]
},
{
"|": [
- {
- "|": [
- "syn",
- "psh"
- ]
- },
+ "syn",
+ "psh",
"ack"
]
},
{
"|": [
- {
- "|": [
- "syn",
- "ack"
- ]
- },
+ "syn",
+ "ack",
"urg"
]
},
@@ -116,39 +84,23 @@
"syn",
{
"|": [
- {
- "|": [
- {
- "|": [
- "rst",
- "psh"
- ]
- },
- "ack"
- ]
- },
+ "rst",
+ "psh",
+ "ack",
"urg"
]
},
{
"|": [
- {
- "|": [
- "rst",
- "psh"
- ]
- },
+ "rst",
+ "psh",
"ack"
]
},
{
"|": [
- {
- "|": [
- "rst",
- "ack"
- ]
- },
+ "rst",
+ "ack",
"urg"
]
},
@@ -161,12 +113,8 @@
"rst",
{
"|": [
- {
- "|": [
- "psh",
- "ack"
- ]
- },
+ "psh",
+ "ack",
"urg"
]
},
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [nft PATCH v2 2/5] mergesort: Avoid accidental set element reordering
2024-03-22 16:06 [nft PATCH v2 0/5] json: Accept more than two operands in binary expressions Phil Sutter
2024-03-22 16:06 ` [nft PATCH v2 1/5] " Phil Sutter
@ 2024-03-22 16:06 ` Phil Sutter
2024-03-22 16:06 ` [nft PATCH v2 3/5] tests: py: Fix some JSON equivalents Phil Sutter
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Phil Sutter @ 2024-03-22 16:06 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
In corner cases, expr_msort_cmp() may return 0 for two non-identical
elements. An example are ORed tcp flags: 'syn' and 'syn | ack' are
considered the same value since expr_msort_value() reduces the latter to
its LHS.
Keeping the above in mind and looking at how list_expr_sort() works: The
list in 'head' is cut in half, the first half put into the temporary
list 'list' and finally 'list' is merged back into 'head' considering
each element's position. Shall expr_msort_cmp() return 0 for two
elements, the one from 'list' ends up after the one in 'head', thus
reverting their previous ordering.
The practical implication is that output never matches input for the
sample set '{ syn, syn | ack }' as the sorting after delinearization in
netlink_list_setelems() keeps swapping the elements. Out of coincidence,
the commit this fixes itself illustrates the use-case this breaks,
namely tracking a ruleset in git: Each ruleset reload will trigger an
update to the stored dump.
This change breaks interval set element deletion because __set_delete()
implicitly relies upon this reordering of duplicate entries by inserting
a clone of the one to delete into the start (via list_move()) and after
sorting assumes the clone will end up right behind the original. Fix
this by calling list_move_tail() instead.
Fixes: 14ee0a979b622 ("src: sort set elements in netlink_get_setelems()")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
src/intervals.c | 2 +-
src/mergesort.c | 2 +-
.../sets/dumps/0055tcpflags_0.json-nft | 32 +++++++++----------
.../testcases/sets/dumps/0055tcpflags_0.nft | 8 ++---
4 files changed, 22 insertions(+), 22 deletions(-)
diff --git a/src/intervals.c b/src/intervals.c
index 68728349e999c..6c3f36fec02aa 100644
--- a/src/intervals.c
+++ b/src/intervals.c
@@ -466,7 +466,7 @@ static int __set_delete(struct list_head *msgs, struct expr *i, struct set *set,
unsigned int debug_mask)
{
i->flags |= EXPR_F_REMOVE;
- list_move(&i->list, &existing_set->init->expressions);
+ list_move_tail(&i->list, &existing_set->init->expressions);
list_expr_sort(&existing_set->init->expressions);
return setelem_delete(msgs, set, init, existing_set->init, debug_mask);
diff --git a/src/mergesort.c b/src/mergesort.c
index 4d0e280fdc5e2..5e676be16369b 100644
--- a/src/mergesort.c
+++ b/src/mergesort.c
@@ -78,7 +78,7 @@ void list_splice_sorted(struct list_head *list, struct list_head *head)
while (l != list) {
if (h == head ||
expr_msort_cmp(list_entry(l, typeof(struct expr), list),
- list_entry(h, typeof(struct expr), list)) < 0) {
+ list_entry(h, typeof(struct expr), list)) <= 0) {
l = l->next;
list_add_tail(l->prev, h);
continue;
diff --git a/tests/shell/testcases/sets/dumps/0055tcpflags_0.json-nft b/tests/shell/testcases/sets/dumps/0055tcpflags_0.json-nft
index 6a3511515f785..e37139f334466 100644
--- a/tests/shell/testcases/sets/dumps/0055tcpflags_0.json-nft
+++ b/tests/shell/testcases/sets/dumps/0055tcpflags_0.json-nft
@@ -28,15 +28,6 @@
{
"|": [
"fin",
- "psh",
- "ack",
- "urg"
- ]
- },
- {
- "|": [
- "fin",
- "psh",
"ack"
]
},
@@ -50,21 +41,22 @@
{
"|": [
"fin",
+ "psh",
"ack"
]
},
{
"|": [
- "syn",
+ "fin",
"psh",
"ack",
"urg"
]
},
+ "syn",
{
"|": [
"syn",
- "psh",
"ack"
]
},
@@ -78,22 +70,22 @@
{
"|": [
"syn",
+ "psh",
"ack"
]
},
- "syn",
{
"|": [
- "rst",
+ "syn",
"psh",
"ack",
"urg"
]
},
+ "rst",
{
"|": [
"rst",
- "psh",
"ack"
]
},
@@ -107,12 +99,13 @@
{
"|": [
"rst",
+ "psh",
"ack"
]
},
- "rst",
{
"|": [
+ "rst",
"psh",
"ack",
"urg"
@@ -126,11 +119,18 @@
},
{
"|": [
+ "psh",
"ack",
"urg"
]
},
- "ack"
+ "ack",
+ {
+ "|": [
+ "ack",
+ "urg"
+ ]
+ }
]
}
}
diff --git a/tests/shell/testcases/sets/dumps/0055tcpflags_0.nft b/tests/shell/testcases/sets/dumps/0055tcpflags_0.nft
index ffed5426577e4..22bf5c461b877 100644
--- a/tests/shell/testcases/sets/dumps/0055tcpflags_0.nft
+++ b/tests/shell/testcases/sets/dumps/0055tcpflags_0.nft
@@ -2,9 +2,9 @@ table ip test {
set tcp_good_flags {
type tcp_flag
flags constant
- elements = { fin | psh | ack | urg, fin | psh | ack, fin | ack | urg, fin | ack, syn | psh | ack | urg,
- syn | psh | ack, syn | ack | urg, syn | ack, syn, rst | psh | ack | urg,
- rst | psh | ack, rst | ack | urg, rst | ack, rst, psh | ack | urg,
- psh | ack, ack | urg, ack }
+ elements = { fin | ack, fin | ack | urg, fin | psh | ack, fin | psh | ack | urg, syn,
+ syn | ack, syn | ack | urg, syn | psh | ack, syn | psh | ack | urg, rst,
+ rst | ack, rst | ack | urg, rst | psh | ack, rst | psh | ack | urg, psh | ack,
+ psh | ack | urg, ack, ack | urg }
}
}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [nft PATCH v2 3/5] tests: py: Fix some JSON equivalents
2024-03-22 16:06 [nft PATCH v2 0/5] json: Accept more than two operands in binary expressions Phil Sutter
2024-03-22 16:06 ` [nft PATCH v2 1/5] " Phil Sutter
2024-03-22 16:06 ` [nft PATCH v2 2/5] mergesort: Avoid accidental set element reordering Phil Sutter
@ 2024-03-22 16:06 ` Phil Sutter
2024-03-22 16:06 ` [nft PATCH v2 4/5] tests: py: Warn if recorded JSON output matches the input Phil Sutter
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Phil Sutter @ 2024-03-22 16:06 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Make sure they match the standard syntax input as much as possible.
For some reason inet/tcp.t.json was using plain arrays in place of
binary OR expressions in many cases. These arrays are interpreted as
list expressions, which seems to be semantically identical but the goal
here is to present an accurate equivalent to the rule in standard
syntax.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
tests/py/any/meta.t.json | 2 +-
tests/py/any/tcpopt.t.json | 4 +-
tests/py/inet/tcp.t.json | 124 +++++++++++++++++++++----------------
3 files changed, 75 insertions(+), 55 deletions(-)
diff --git a/tests/py/any/meta.t.json b/tests/py/any/meta.t.json
index d50272de55a5b..60e27d48fdf03 100644
--- a/tests/py/any/meta.t.json
+++ b/tests/py/any/meta.t.json
@@ -2661,7 +2661,7 @@
}
},
"op": "==",
- "right": "17:00"
+ "right": "17:00:00"
}
},
{
diff --git a/tests/py/any/tcpopt.t.json b/tests/py/any/tcpopt.t.json
index 4466f14fac638..87074b9d216a3 100644
--- a/tests/py/any/tcpopt.t.json
+++ b/tests/py/any/tcpopt.t.json
@@ -192,7 +192,7 @@
"left": {
"tcp option": {
"field": "left",
- "name": "sack"
+ "name": "sack0"
}
},
"op": "==",
@@ -272,7 +272,7 @@
"left": {
"tcp option": {
"field": "right",
- "name": "sack"
+ "name": "sack0"
}
},
"op": "==",
diff --git a/tests/py/inet/tcp.t.json b/tests/py/inet/tcp.t.json
index bd589cf0091fe..28dd4341f08b5 100644
--- a/tests/py/inet/tcp.t.json
+++ b/tests/py/inet/tcp.t.json
@@ -1370,13 +1370,13 @@
"op": "==",
"right": {
"set": [
+ "syn",
{
"|": [
"syn",
"ack"
]
- },
- "syn"
+ }
]
}
}
@@ -1401,10 +1401,10 @@
"op": "==",
"right": {
"set": [
- { "|": [ "fin", "psh", "ack" ] },
"fin",
+ "ack",
{ "|": [ "psh", "ack" ] },
- "ack"
+ { "|": [ "fin", "psh", "ack" ] }
]
}
}
@@ -1442,17 +1442,21 @@
"protocol": "tcp"
}
},
- [
- "fin",
- "syn"
- ]
+ {
+ "|": [
+ "fin",
+ "syn"
+ ]
+ }
]
},
"op": "==",
- "right": [
- "fin",
- "syn"
- ]
+ "right": {
+ "|": [
+ "fin",
+ "syn"
+ ]
+ }
}
}
]
@@ -1469,10 +1473,12 @@
"protocol": "tcp"
}
},
- [
- "fin",
- "syn"
- ]
+ {
+ "|": [
+ "fin",
+ "syn"
+ ]
+ }
]
},
"op": "!=",
@@ -1605,12 +1611,14 @@
"protocol": "tcp"
}
},
- [
- "fin",
- "syn",
- "rst",
- "ack"
- ]
+ {
+ "|": [
+ "fin",
+ "syn",
+ "rst",
+ "ack"
+ ]
+ }
]
},
"op": "==",
@@ -1631,12 +1639,14 @@
"protocol": "tcp"
}
},
- [
- "fin",
- "syn",
- "rst",
- "ack"
- ]
+ {
+ "|": [
+ "fin",
+ "syn",
+ "rst",
+ "ack"
+ ]
+ }
]
},
"op": "==",
@@ -1658,12 +1668,14 @@
"protocol": "tcp"
}
},
- [
- "fin",
- "syn",
- "rst",
- "ack"
- ]
+ {
+ "|": [
+ "fin",
+ "syn",
+ "rst",
+ "ack"
+ ]
+ }
]
},
"op": "!=",
@@ -1684,19 +1696,23 @@
"protocol": "tcp"
}
},
- [
- "fin",
- "syn",
- "rst",
- "ack"
- ]
+ {
+ "|": [
+ "fin",
+ "syn",
+ "rst",
+ "ack"
+ ]
+ }
]
},
"op": "==",
- "right": [
- "syn",
- "ack"
- ]
+ "right": {
+ "|": [
+ "syn",
+ "ack"
+ ]
+ }
}
}
]
@@ -1713,17 +1729,21 @@
"protocol": "tcp"
}
},
- [
- "syn",
- "ack"
- ]
+ {
+ "|": [
+ "syn",
+ "ack"
+ ]
+ }
]
},
"op": "==",
- "right": [
- "syn",
- "ack"
- ]
+ "right": {
+ "|": [
+ "syn",
+ "ack"
+ ]
+ }
}
}
]
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [nft PATCH v2 4/5] tests: py: Warn if recorded JSON output matches the input
2024-03-22 16:06 [nft PATCH v2 0/5] json: Accept more than two operands in binary expressions Phil Sutter
` (2 preceding siblings ...)
2024-03-22 16:06 ` [nft PATCH v2 3/5] tests: py: Fix some JSON equivalents Phil Sutter
@ 2024-03-22 16:06 ` Phil Sutter
2024-03-22 16:06 ` [nft PATCH v2 5/5] tests: py: Drop needless recorded JSON outputs Phil Sutter
2024-04-12 12:35 ` [nft PATCH v2 0/5] json: Accept more than two operands in binary expressions Phil Sutter
5 siblings, 0 replies; 7+ messages in thread
From: Phil Sutter @ 2024-03-22 16:06 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
Actively support spring-cleaning by nagging callers.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
tests/py/nft-test.py | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/py/nft-test.py b/tests/py/nft-test.py
index a7d27c25f9fe0..1bc8955836d0d 100755
--- a/tests/py/nft-test.py
+++ b/tests/py/nft-test.py
@@ -809,6 +809,8 @@ def set_delete_elements(set_element, set_name, table, filename=None,
reason = "Invalid JSON syntax in expected output: %s" % json_expected
print_error(reason)
return [-1, warning, error, unit_tests]
+ if json_expected == json_input:
+ print_warning("Recorded JSON output matches input for: %s" % rule[0])
for table in table_list:
if rule[1].strip() == "ok":
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [nft PATCH v2 5/5] tests: py: Drop needless recorded JSON outputs
2024-03-22 16:06 [nft PATCH v2 0/5] json: Accept more than two operands in binary expressions Phil Sutter
` (3 preceding siblings ...)
2024-03-22 16:06 ` [nft PATCH v2 4/5] tests: py: Warn if recorded JSON output matches the input Phil Sutter
@ 2024-03-22 16:06 ` Phil Sutter
2024-04-12 12:35 ` [nft PATCH v2 0/5] json: Accept more than two operands in binary expressions Phil Sutter
5 siblings, 0 replies; 7+ messages in thread
From: Phil Sutter @ 2024-03-22 16:06 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
These match the input already, no need to track them.
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
tests/py/any/last.t.json.output | 7 -
tests/py/any/meta.t.json.output | 180 --------------------
tests/py/inet/tcp.t.json.output | 265 ------------------------------
tests/py/ip/numgen.t.json.output | 30 ----
tests/py/ip6/exthdr.t.json.output | 30 ----
5 files changed, 512 deletions(-)
diff --git a/tests/py/any/last.t.json.output b/tests/py/any/last.t.json.output
index b8a977edfca7f..e8ec4f478a428 100644
--- a/tests/py/any/last.t.json.output
+++ b/tests/py/any/last.t.json.output
@@ -1,10 +1,3 @@
-# last
-[
- {
- "last": null
- }
-]
-
# last used 300s
[
{
diff --git a/tests/py/any/meta.t.json.output b/tests/py/any/meta.t.json.output
index 4e9e669fdbc3b..d46935dee513d 100644
--- a/tests/py/any/meta.t.json.output
+++ b/tests/py/any/meta.t.json.output
@@ -592,24 +592,6 @@
}
]
-# meta time "1970-05-23 21:07:14" drop
-[
- {
- "match": {
- "left": {
- "meta": {
- "key": "time"
- }
- },
- "op": "==",
- "right": "1970-05-23 21:07:14"
- }
- },
- {
- "drop": null
- }
-]
-
# meta time 12341234 drop
[
{
@@ -628,96 +610,6 @@
}
]
-# meta time "2019-06-21 17:00:00" drop
-[
- {
- "match": {
- "left": {
- "meta": {
- "key": "time"
- }
- },
- "op": "==",
- "right": "2019-06-21 17:00:00"
- }
- },
- {
- "drop": null
- }
-]
-
-# meta time "2019-07-01 00:00:00" drop
-[
- {
- "match": {
- "left": {
- "meta": {
- "key": "time"
- }
- },
- "op": "==",
- "right": "2019-07-01 00:00:00"
- }
- },
- {
- "drop": null
- }
-]
-
-# meta time "2019-07-01 00:01:00" drop
-[
- {
- "match": {
- "left": {
- "meta": {
- "key": "time"
- }
- },
- "op": "==",
- "right": "2019-07-01 00:01:00"
- }
- },
- {
- "drop": null
- }
-]
-
-# meta time "2019-07-01 00:00:01" drop
-[
- {
- "match": {
- "left": {
- "meta": {
- "key": "time"
- }
- },
- "op": "==",
- "right": "2019-07-01 00:00:01"
- }
- },
- {
- "drop": null
- }
-]
-
-# meta day "Saturday" drop
-[
- {
- "match": {
- "left": {
- "meta": {
- "key": "day"
- }
- },
- "op": "==",
- "right": "Saturday"
- }
- },
- {
- "drop": null
- }
-]
-
# meta day 6 drop
[
{
@@ -736,24 +628,6 @@
}
]
-# meta hour "17:00" drop
-[
- {
- "match": {
- "left": {
- "meta": {
- "key": "hour"
- }
- },
- "op": "==",
- "right": "17:00"
- }
- },
- {
- "drop": null
- }
-]
-
# meta hour "17:00:00" drop
[
{
@@ -772,57 +646,3 @@
}
]
-# meta hour "17:00:01" drop
-[
- {
- "match": {
- "left": {
- "meta": {
- "key": "hour"
- }
- },
- "op": "==",
- "right": "17:00:01"
- }
- },
- {
- "drop": null
- }
-]
-
-# meta hour "00:00" drop
-[
- {
- "match": {
- "left": {
- "meta": {
- "key": "hour"
- }
- },
- "op": "==",
- "right": "00:00"
- }
- },
- {
- "drop": null
- }
-]
-
-# meta hour "00:01" drop
-[
- {
- "match": {
- "left": {
- "meta": {
- "key": "hour"
- }
- },
- "op": "==",
- "right": "00:01"
- }
- },
- {
- "drop": null
- }
-]
-
diff --git a/tests/py/inet/tcp.t.json.output b/tests/py/inet/tcp.t.json.output
index 3f03c0ddd1586..d487a8f1bfa09 100644
--- a/tests/py/inet/tcp.t.json.output
+++ b/tests/py/inet/tcp.t.json.output
@@ -115,32 +115,6 @@
}
]
-# tcp flags { syn, syn | ack }
-[
- {
- "match": {
- "left": {
- "payload": {
- "field": "flags",
- "protocol": "tcp"
- }
- },
- "op": "==",
- "right": {
- "set": [
- "syn",
- {
- "|": [
- "syn",
- "ack"
- ]
- }
- ]
- }
- }
- }
-]
-
# tcp flags & (fin | syn | rst | psh | ack | urg) == { fin, ack, psh | ack, fin | psh | ack }
[
{
@@ -188,242 +162,3 @@
}
}
]
-
-# tcp flags fin,syn / fin,syn
-[
- {
- "match": {
- "left": {
- "&": [
- {
- "payload": {
- "field": "flags",
- "protocol": "tcp"
- }
- },
- {
- "|": [
- "fin",
- "syn"
- ]
- }
- ]
- },
- "op": "==",
- "right": {
- "|": [
- "fin",
- "syn"
- ]
- }
- }
- }
-]
-
-# tcp flags != syn / fin,syn
-[
- {
- "match": {
- "left": {
- "&": [
- {
- "payload": {
- "field": "flags",
- "protocol": "tcp"
- }
- },
- {
- "|": [
- "fin",
- "syn"
- ]
- }
- ]
- },
- "op": "!=",
- "right": "syn"
- }
- }
-]
-
-# tcp flags & (fin | syn | rst | ack) syn
-[
- {
- "match": {
- "left": {
- "&": [
- {
- "payload": {
- "field": "flags",
- "protocol": "tcp"
- }
- },
- {
- "|": [
- "fin",
- "syn",
- "rst",
- "ack"
- ]
- }
- ]
- },
- "op": "==",
- "right": "syn"
- }
- }
-]
-
-# tcp flags & (fin | syn | rst | ack) == syn
-[
- {
- "match": {
- "left": {
- "&": [
- {
- "payload": {
- "field": "flags",
- "protocol": "tcp"
- }
- },
- {
- "|": [
- "fin",
- "syn",
- "rst",
- "ack"
- ]
- }
- ]
- },
- "op": "==",
- "right": "syn"
- }
- }
-]
-
-# tcp flags & (fin | syn | rst | ack) != syn
-[
- {
- "match": {
- "left": {
- "&": [
- {
- "payload": {
- "field": "flags",
- "protocol": "tcp"
- }
- },
- {
- "|": [
- "fin",
- "syn",
- "rst",
- "ack"
- ]
- }
- ]
- },
- "op": "!=",
- "right": "syn"
- }
- }
-]
-
-# tcp flags & (fin | syn | rst | ack) == syn | ack
-[
- {
- "match": {
- "left": {
- "&": [
- {
- "payload": {
- "field": "flags",
- "protocol": "tcp"
- }
- },
- {
- "|": [
- "fin",
- "syn",
- "rst",
- "ack"
- ]
- }
- ]
- },
- "op": "==",
- "right": {
- "|": [
- "syn",
- "ack"
- ]
- }
- }
- }
-]
-
-# tcp flags & (fin | syn | rst | ack) != syn | ack
-[
- {
- "match": {
- "left": {
- "&": [
- {
- "payload": {
- "field": "flags",
- "protocol": "tcp"
- }
- },
- {
- "|": [
- "fin",
- "syn",
- "rst",
- "ack"
- ]
- }
- ]
- },
- "op": "!=",
- "right": {
- "|": [
- "syn",
- "ack"
- ]
- }
- }
- }
-]
-
-# tcp flags & (syn | ack) == syn | ack
-[
- {
- "match": {
- "left": {
- "&": [
- {
- "payload": {
- "field": "flags",
- "protocol": "tcp"
- }
- },
- {
- "|": [
- "syn",
- "ack"
- ]
- }
- ]
- },
- "op": "==",
- "right": {
- "|": [
- "syn",
- "ack"
- ]
- }
- }
- }
-]
-
diff --git a/tests/py/ip/numgen.t.json.output b/tests/py/ip/numgen.t.json.output
index 06ad1eccae5cf..b54121ca0f721 100644
--- a/tests/py/ip/numgen.t.json.output
+++ b/tests/py/ip/numgen.t.json.output
@@ -80,33 +80,3 @@
}
]
-# dnat to numgen inc mod 7 offset 167772161
-[
- {
- "dnat": {
- "addr": {
- "numgen": {
- "mod": 7,
- "mode": "inc",
- "offset": 167772161
- }
- }
- }
- }
-]
-
-# dnat to numgen inc mod 255 offset 167772161
-[
- {
- "dnat": {
- "addr": {
- "numgen": {
- "mod": 255,
- "mode": "inc",
- "offset": 167772161
- }
- }
- }
- }
-]
-
diff --git a/tests/py/ip6/exthdr.t.json.output b/tests/py/ip6/exthdr.t.json.output
index c9f5b49b915c8..813402a26c7f4 100644
--- a/tests/py/ip6/exthdr.t.json.output
+++ b/tests/py/ip6/exthdr.t.json.output
@@ -1,33 +1,3 @@
-# exthdr hbh == exists
-[
- {
- "match": {
- "left": {
- "exthdr": {
- "name": "hbh"
- }
- },
- "op": "==",
- "right": true
- }
- }
-]
-
-# exthdr hbh == missing
-[
- {
- "match": {
- "left": {
- "exthdr": {
- "name": "hbh"
- }
- },
- "op": "==",
- "right": false
- }
- }
-]
-
# exthdr hbh 1
[
{
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [nft PATCH v2 0/5] json: Accept more than two operands in binary expressions
2024-03-22 16:06 [nft PATCH v2 0/5] json: Accept more than two operands in binary expressions Phil Sutter
` (4 preceding siblings ...)
2024-03-22 16:06 ` [nft PATCH v2 5/5] tests: py: Drop needless recorded JSON outputs Phil Sutter
@ 2024-04-12 12:35 ` Phil Sutter
5 siblings, 0 replies; 7+ messages in thread
From: Phil Sutter @ 2024-04-12 12:35 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Fri, Mar 22, 2024 at 05:06:40PM +0100, Phil Sutter wrote:
> This needed rebase of patch 1 to cover for intermediate changes got a
> bit out of hand and resulted in this series:
>
> Patch 2 fixes a bug in set element sorting triggered by binop expression
> elements being treated equal in value if the LHS parts match.
>
> Patch 3 collates general bugs in recorded JSON equivalents for py
> testsuite.
>
> Patch 4 adds detection of needless recorded JSON outputs to
> nft-tests.py, patch 5 then performs the cleanup to eliminate the
> warnings it generates.
>
> Phil Sutter (5):
> json: Accept more than two operands in binary expressions
> mergesort: Avoid accidental set element reordering
> tests: py: Fix some JSON equivalents
> tests: py: Warn if recorded JSON output matches the input
> tests: py: Drop needless recorded JSON outputs
Series applied.
^ permalink raw reply [flat|nested] 7+ messages in thread