* [nft PATCH] json: Accept more than two operands in binary expressions
@ 2024-03-20 14:58 Phil Sutter
2024-03-20 17:17 ` Pablo Neira Ayuso
0 siblings, 1 reply; 3+ messages in thread
From: Phil Sutter @ 2024-03-20 14:58 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 | 50 +---------
tests/py/inet/tcp.t.json.output | 34 ++-----
.../dumps/0012different_defines_0.json-nft | 8 +-
.../sets/dumps/0055tcpflags_0.json-nft | 98 +++++--------------
7 files changed, 75 insertions(+), 164 deletions(-)
diff --git a/doc/libnftables-json.adoc b/doc/libnftables-json.adoc
index 3948a0bad47c1..e3b24cc4ed60d 100644
--- a/doc/libnftables-json.adoc
+++ b/doc/libnftables-json.adoc
@@ -1343,15 +1343,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 8439c2b5931dd..9a1b158e7ac0b 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"
]
}
diff --git a/tests/py/inet/tcp.t.json.output b/tests/py/inet/tcp.t.json.output
index c471e8d8dcef5..5a16714e9145d 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"
]
},
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] 3+ messages in thread* Re: [nft PATCH] json: Accept more than two operands in binary expressions
2024-03-20 14:58 [nft PATCH] json: Accept more than two operands in binary expressions Phil Sutter
@ 2024-03-20 17:17 ` Pablo Neira Ayuso
2024-03-20 20:44 ` Phil Sutter
0 siblings, 1 reply; 3+ messages in thread
From: Pablo Neira Ayuso @ 2024-03-20 17:17 UTC (permalink / raw)
To: Phil Sutter; +Cc: netfilter-devel
On Wed, Mar 20, 2024 at 03:58:06PM +0100, Phil Sutter wrote:
> 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
This is equivalent to:
meta mark set ip dscp << 5
userspace is lacking the code to simplify this, just like it does for:
meta mark set ip dscp | 0x8 | 0xf0
results in:
meta mark set ip dscp | 0xf8
> 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 | 50 +---------
> tests/py/inet/tcp.t.json.output | 34 ++-----
> .../dumps/0012different_defines_0.json-nft | 8 +-
> .../sets/dumps/0055tcpflags_0.json-nft | 98 +++++--------------
> 7 files changed, 75 insertions(+), 164 deletions(-)
>
> diff --git a/doc/libnftables-json.adoc b/doc/libnftables-json.adoc
> index 3948a0bad47c1..e3b24cc4ed60d 100644
> --- a/doc/libnftables-json.adoc
> +++ b/doc/libnftables-json.adoc
> @@ -1343,15 +1343,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 8439c2b5931dd..9a1b158e7ac0b 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"
> ]
> }
> diff --git a/tests/py/inet/tcp.t.json.output b/tests/py/inet/tcp.t.json.output
> index c471e8d8dcef5..5a16714e9145d 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"
> ]
> },
> 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 [flat|nested] 3+ messages in thread* Re: [nft PATCH] json: Accept more than two operands in binary expressions
2024-03-20 17:17 ` Pablo Neira Ayuso
@ 2024-03-20 20:44 ` Phil Sutter
0 siblings, 0 replies; 3+ messages in thread
From: Phil Sutter @ 2024-03-20 20:44 UTC (permalink / raw)
To: Pablo Neira Ayuso; +Cc: netfilter-devel
On Wed, Mar 20, 2024 at 06:17:18PM +0100, Pablo Neira Ayuso wrote:
> On Wed, Mar 20, 2024 at 03:58:06PM +0100, Phil Sutter wrote:
> > 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
>
> This is equivalent to:
>
> meta mark set ip dscp << 5
>
> userspace is lacking the code to simplify this, just like it does for:
>
> meta mark set ip dscp | 0x8 | 0xf0
>
> results in:
>
> meta mark set ip dscp | 0xf8
You're right, of course. Simplifying the input is a different task
though, I merely made sure that JSON input/output matches what regular
syntax supports as well.
Input optimization should happen in eval phase (or so) anyway and thus
independent of where input was parsed from.
Cheers, Phil
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-03-20 20:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-20 14:58 [nft PATCH] json: Accept more than two operands in binary expressions Phil Sutter
2024-03-20 17:17 ` Pablo Neira Ayuso
2024-03-20 20:44 ` Phil Sutter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).