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@xxxxxx>
> ---
> 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
>
