Wrap datatype compatibility check into a helper function and use it for map evaluation, otherwise the following bogus error message is displayed: Error: datatype mismatch, map expects packet mark, mapping expression has type integer Add unit tests to improve coverage for this usecase. Fixes: 5d8e33ddb112 ("evaluate: relax type-checking for integer arguments in mark statements") Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> --- src/evaluate.c | 19 ++++++---- .../maps/dumps/vmap_mark_bitwise_0.nft | 26 +++++++++++++ .../shell/testcases/maps/vmap_mark_bitwise_0 | 38 +++++++++++++++++++ 3 files changed, 76 insertions(+), 7 deletions(-) create mode 100644 tests/shell/testcases/maps/dumps/vmap_mark_bitwise_0.nft create mode 100755 tests/shell/testcases/maps/vmap_mark_bitwise_0 diff --git a/src/evaluate.c b/src/evaluate.c index a537dcfd52b3..9fccd032aaee 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -1882,6 +1882,15 @@ static int mapping_expr_expand(struct eval_ctx *ctx) return 0; } +static bool datatype_compatible(const struct datatype *a, const struct datatype *b) +{ + return (a->type == TYPE_MARK && + !datatype_equal(datatype_basetype(a), datatype_basetype(b))) || + (a->type != TYPE_MARK && + b->type != TYPE_INTEGER && + !datatype_equal(b, a)); +} + static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr) { struct expr *map = *expr, *mappings; @@ -1989,7 +1998,7 @@ static int expr_evaluate_map(struct eval_ctx *ctx, struct expr **expr) expr_name(map->mappings)); } - if (!datatype_equal(map->map->dtype, map->mappings->set->key->dtype)) + if (!datatype_compatible(map->map->dtype, map->mappings->set->key->dtype)) return expr_binary_error(ctx->msgs, map->mappings, map->map, "datatype mismatch, map expects %s, " "mapping expression has type %s", @@ -2823,11 +2832,7 @@ static int __stmt_evaluate_arg(struct eval_ctx *ctx, struct stmt *stmt, dtype->desc, (*expr)->dtype->desc, (*expr)->len); - if ((dtype->type == TYPE_MARK && - !datatype_equal(datatype_basetype(dtype), datatype_basetype((*expr)->dtype))) || - (dtype->type != TYPE_MARK && - (*expr)->dtype->type != TYPE_INTEGER && - !datatype_equal((*expr)->dtype, dtype))) + if (!datatype_compatible(dtype, (*expr)->dtype)) return stmt_binary_error(ctx, *expr, stmt, /* verdict vs invalid? */ "datatype mismatch: expected %s, " "expression has type %s", @@ -4385,7 +4390,7 @@ static int stmt_evaluate_objref_map(struct eval_ctx *ctx, struct stmt *stmt) expr_name(map->mappings)); } - if (!datatype_equal(map->map->dtype, map->mappings->set->key->dtype)) + if (!datatype_compatible(map->map->dtype, map->mappings->set->key->dtype)) return expr_binary_error(ctx->msgs, map->mappings, map->map, "datatype mismatch, map expects %s, " "mapping expression has type %s", diff --git a/tests/shell/testcases/maps/dumps/vmap_mark_bitwise_0.nft b/tests/shell/testcases/maps/dumps/vmap_mark_bitwise_0.nft new file mode 100644 index 000000000000..beb5ffb098f5 --- /dev/null +++ b/tests/shell/testcases/maps/dumps/vmap_mark_bitwise_0.nft @@ -0,0 +1,26 @@ +table ip x { + counter c_o0_0 { + packets 0 bytes 0 + } + + map sctm_o0 { + type mark : verdict + elements = { 0x00000000 : jump sctm_o0_0, 0x00000001 : jump sctm_o0_1 } + } + + map sctm_o1 { + type mark : counter + elements = { 0x00000000 : "c_o0_0" } + } + + chain sctm_o0_0 { + } + + chain sctm_o0_1 { + } + + chain SET_ctmark_RPLYroute { + meta mark >> 8 & 0xf vmap @sctm_o0 + counter name meta mark >> 8 & 0xf map @sctm_o1 + } +} diff --git a/tests/shell/testcases/maps/vmap_mark_bitwise_0 b/tests/shell/testcases/maps/vmap_mark_bitwise_0 new file mode 100755 index 000000000000..3ac99d2853b2 --- /dev/null +++ b/tests/shell/testcases/maps/vmap_mark_bitwise_0 @@ -0,0 +1,38 @@ +#!/bin/bash + +set -e + +RULESET="table ip x { + chain sctm_o0_0 { + } + + chain sctm_o0_1 { + } + + map sctm_o0 { + type mark : verdict + elements = { + 0x0 : jump sctm_o0_0, + 0x1 : jump sctm_o0_1, + } + } + + counter c_o0_0 {} + + map sctm_o1 { + type mark : counter + elements = { + 0x0 : \"c_o0_0\", + } + } + + chain SET_ctmark_RPLYroute { + (meta mark >> 8) & 0xF vmap @sctm_o0 + } + + chain SET_ctmark_RPLYroute { + counter name (meta mark >> 8) & 0xF map @sctm_o1 + } +}" + +$NFT -f - <<< $RULESET -- 2.30.2