With a bit of code reorganization, relational meta OPs OP_RANGE, OP_FLAGCMP and OP_LOOKUP become unused and can be removed. The only meta OP left is OP_IMPLICIT which is usually treated as alias to OP_EQ. Though it needs to stay in place for one reason: When matching against a bitmask (e.g. TCP flags or conntrack states), it has a different meaning:
| nft --debug=netlink add rule ip t c tcp flags syn | ip t c | [ meta load l4proto => reg 1 ] | [ cmp eq reg 1 0x00000006 ] | [ payload load 1b @ transport header + 13 => reg 1 ] | [ bitwise reg 1 = (reg=1 & 0x00000002 ) ^ 0x00000000 ] | [ cmp neq reg 1 0x00000000 ] | nft --debug=netlink add rule ip t c tcp flags == syn | ip t c | [ meta load l4proto => reg 1 ] | [ cmp eq reg 1 0x00000006 ] | [ payload load 1b @ transport header + 13 => reg 1 ] | [ cmp eq reg 1 0x00000002 ] OP_IMPLICIT creates a match which just checks the given flag is present, while OP_EQ creates a match which ensures the given flag and no other is present. Signed-off-by: Phil Sutter <[email protected]> --- include/expression.h | 6 --- src/evaluate.c | 114 +++++++--------------------------------------- src/expression.c | 8 +--- src/netlink_delinearize.c | 21 ++++----- src/netlink_linearize.c | 20 +++++--- src/parser_bison.y | 2 +- src/rule.c | 13 +++++- 7 files changed, 53 insertions(+), 131 deletions(-) diff --git a/include/expression.h b/include/expression.h index 29dd0346a9b6b..f0ba6fc112601 100644 --- a/include/expression.h +++ b/include/expression.h @@ -85,12 +85,6 @@ enum ops { OP_GT, OP_LTE, OP_GTE, - /* Range comparison */ - OP_RANGE, - /* Flag comparison */ - OP_FLAGCMP, - /* Set lookup */ - OP_LOOKUP, __OP_MAX }; #define OP_MAX (__OP_MAX - 1) diff --git a/src/evaluate.c b/src/evaluate.c index a2c1c7283d6ad..cbe15fedcf2b3 100644 --- a/src/evaluate.c +++ b/src/evaluate.c @@ -1565,28 +1565,6 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr) return -1; right = rel->right; - if (rel->op == OP_IMPLICIT) { - switch (right->ops->type) { - case EXPR_RANGE: - rel->op = OP_RANGE; - break; - case EXPR_SET: - case EXPR_SET_REF: - rel->op = OP_LOOKUP; - break; - case EXPR_LIST: - rel->op = OP_FLAGCMP; - break; - default: - if (right->dtype->basetype != NULL && - right->dtype->basetype->type == TYPE_BITMASK) - rel->op = OP_FLAGCMP; - else - rel->op = OP_EQ; - break; - } - } - if (!expr_is_constant(right)) return expr_binary_error(ctx->msgs, right, rel, "Right hand side of relational " @@ -1598,56 +1576,34 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr) "constant value", expr_op_symbols[rel->op]); + if (!datatype_equal(left->dtype, right->dtype)) + return expr_binary_error(ctx->msgs, right, left, + "datatype mismatch, expected %s, " + "expression has type %s", + left->dtype->desc, + right->dtype->desc); + switch (rel->op) { - case OP_LOOKUP: - /* A literal set expression implicitly declares the set */ - if (right->ops->type == EXPR_SET) - right = rel->right = - implicit_set_declaration(ctx, "__set%d", - left, right); - else if (!datatype_equal(left->dtype, right->dtype)) - return expr_binary_error(ctx->msgs, right, left, - "datatype mismatch, expected %s, " - "set has type %s", - left->dtype->desc, - right->dtype->desc); - - /* Data for range lookups needs to be in big endian order */ - if (right->set->flags & NFT_SET_INTERVAL && - byteorder_conversion(ctx, &rel->left, - BYTEORDER_BIG_ENDIAN) < 0) - return -1; - left = rel->left; - break; case OP_EQ: - if (!datatype_equal(left->dtype, right->dtype)) - return expr_binary_error(ctx->msgs, right, left, - "datatype mismatch, expected %s, " - "expression has type %s", - left->dtype->desc, - right->dtype->desc); + case OP_IMPLICIT: /* * Update protocol context for payload and meta iiftype * equality expressions. */ - relational_expr_pctx_update(&ctx->pctx, rel); - - if (left->ops->type == EXPR_CONCAT) - return 0; + if (expr_is_singleton(right)) + relational_expr_pctx_update(&ctx->pctx, rel); /* fall through */ case OP_NEQ: - case OP_FLAGCMP: - if (!datatype_equal(left->dtype, right->dtype)) - return expr_binary_error(ctx->msgs, right, left, - "datatype mismatch, expected %s, " - "expression has type %s", - left->dtype->desc, - right->dtype->desc); - switch (right->ops->type) { case EXPR_RANGE: - goto range; + if (byteorder_conversion(ctx, &rel->left, BYTEORDER_BIG_ENDIAN) < 0) + return -1; + if (byteorder_conversion(ctx, &right->left, BYTEORDER_BIG_ENDIAN) < 0) + return -1; + if (byteorder_conversion(ctx, &right->right, BYTEORDER_BIG_ENDIAN) < 0) + return -1; + break; case EXPR_PREFIX: if (byteorder_conversion(ctx, &right->prefix, left->byteorder) < 0) return -1; @@ -1657,12 +1613,10 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr) return -1; break; case EXPR_SET: - assert(rel->op == OP_NEQ); right = rel->right = implicit_set_declaration(ctx, "__set%d", left, right); /* fall through */ case EXPR_SET_REF: - assert(rel->op == OP_NEQ); /* Data for range lookups needs to be in big endian order */ if (right->set->flags & NFT_SET_INTERVAL && byteorder_conversion(ctx, &rel->left, BYTEORDER_BIG_ENDIAN) < 0) @@ -1676,13 +1630,6 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr) case OP_GT: case OP_LTE: case OP_GTE: - if (!datatype_equal(left->dtype, right->dtype)) - return expr_binary_error(ctx->msgs, right, left, - "datatype mismatch, expected %s, " - "expression has type %s", - left->dtype->desc, - right->dtype->desc); - switch (left->ops->type) { case EXPR_CONCAT: return expr_binary_error(ctx->msgs, left, rel, @@ -1706,33 +1653,6 @@ static int expr_evaluate_relational(struct eval_ctx *ctx, struct expr **expr) if (byteorder_conversion(ctx, &rel->right, BYTEORDER_BIG_ENDIAN) < 0) return -1; break; - case OP_RANGE: - if (!datatype_equal(left->dtype, right->dtype)) - return expr_binary_error(ctx->msgs, right, left, - "datatype mismatch, expected %s, " - "expression has type %s", - left->dtype->desc, - right->dtype->desc); - -range: - switch (left->ops->type) { - case EXPR_CONCAT: - return expr_binary_error(ctx->msgs, left, rel, - "Relational expression (%s) is undefined" - "for %s expressions", - expr_op_symbols[rel->op], - left->ops->name); - default: - break; - } - - if (byteorder_conversion(ctx, &rel->left, BYTEORDER_BIG_ENDIAN) < 0) - return -1; - if (byteorder_conversion(ctx, &right->left, BYTEORDER_BIG_ENDIAN) < 0) - return -1; - if (byteorder_conversion(ctx, &right->right, BYTEORDER_BIG_ENDIAN) < 0) - return -1; - break; default: BUG("invalid relational operation %u\n", rel->op); } diff --git a/src/expression.c b/src/expression.c index d7f54ad713732..5f023d2ad88e7 100644 --- a/src/expression.c +++ b/src/expression.c @@ -496,8 +496,6 @@ const char *expr_op_symbols[] = { [OP_GT] = ">", [OP_LTE] = "<=", [OP_GTE] = ">=", - [OP_RANGE] = "within range", - [OP_LOOKUP] = NULL, }; static void unary_expr_print(const struct expr *expr, struct output_ctx *octx) @@ -562,10 +560,6 @@ static void binop_arg_print(const struct expr *op, const struct expr *arg, static bool must_print_eq_op(const struct expr *expr) { - if (expr->right->dtype->basetype != NULL && - expr->right->dtype->basetype->type == TYPE_BITMASK) - return true; - return expr->left->ops->type == EXPR_BINOP; } @@ -645,7 +639,7 @@ void relational_expr_pctx_update(struct proto_ctx *ctx, const struct expr *left = expr->left; assert(expr->ops->type == EXPR_RELATIONAL); - assert(expr->op == OP_EQ); + assert(expr->op == OP_EQ || expr->op == OP_IMPLICIT); if (left->ops->pctx_update && (left->flags & EXPR_F_PROTOCOL)) diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index d65aacf8b6168..6b5c4aaae108f 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -323,7 +323,7 @@ static void netlink_parse_lookup(struct netlink_parse_ctx *ctx, if (dreg != NFT_REG_VERDICT) return netlink_set_register(ctx, dreg, expr); } else { - expr = relational_expr_alloc(loc, OP_LOOKUP, left, right); + expr = relational_expr_alloc(loc, OP_EQ, left, right); } if (nftnl_expr_is_set(nle, NFTNL_EXPR_LOOKUP_FLAGS)) { @@ -1514,9 +1514,14 @@ static void ct_meta_common_postprocess(struct rule_pp_ctx *ctx, const struct expr *left = expr->left; struct expr *right = expr->right; + if (right->ops->type == EXPR_SET || right->ops->type == EXPR_SET_REF) + expr_set_type(right, left->dtype, left->byteorder); + switch (expr->op) { case OP_EQ: - if (expr->right->ops->type == EXPR_RANGE) + if (expr->right->ops->type == EXPR_RANGE || + expr->right->ops->type == EXPR_SET || + expr->right->ops->type == EXPR_SET_REF) break; relational_expr_pctx_update(&ctx->pctx, expr); @@ -1533,14 +1538,6 @@ static void ct_meta_common_postprocess(struct rule_pp_ctx *ctx, payload_dependency_store(&ctx->pdctx, ctx->stmt, base); } break; - case OP_NEQ: - if (right->ops->type != EXPR_SET && right->ops->type != EXPR_SET_REF) - break; - /* fall through */ - case OP_LOOKUP: - expr_set_type(right, left->dtype, left->byteorder); - break; - default: break; } @@ -1719,13 +1716,13 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx, struct expr *e /* Flag comparison: data & flags != 0 * * Split the flags into a list of flag values and convert the - * op to OP_FLAGCMP. + * op to OP_EQ. */ expr_free(value); expr->left = expr_get(binop->left); expr->right = binop_tree_to_list(NULL, binop->right); - expr->op = OP_FLAGCMP; + expr->op = OP_EQ; expr_free(binop); } else if (binop->left->dtype->flags & DTYPE_F_PREFIX && diff --git a/src/netlink_linearize.c b/src/netlink_linearize.c index 5edb2d3d6fe86..5836231d5f3cf 100644 --- a/src/netlink_linearize.c +++ b/src/netlink_linearize.c @@ -297,6 +297,7 @@ static enum nft_cmp_ops netlink_gen_cmp_op(enum ops op) { switch (op) { case OP_EQ: + case OP_IMPLICIT: return NFT_CMP_EQ; case OP_NEQ: return NFT_CMP_NEQ; @@ -316,6 +317,9 @@ static enum nft_cmp_ops netlink_gen_cmp_op(enum ops op) static void netlink_gen_range(struct netlink_linearize_ctx *ctx, const struct expr *expr, enum nft_registers dreg); +static void netlink_gen_flagcmp(struct netlink_linearize_ctx *ctx, + const struct expr *expr, + enum nft_registers dreg); static struct expr *netlink_gen_prefix(struct netlink_linearize_ctx *ctx, const struct expr *expr, @@ -362,6 +366,8 @@ static void netlink_gen_cmp(struct netlink_linearize_ctx *ctx, case EXPR_SET: case EXPR_SET_REF: return netlink_gen_lookup(ctx, expr, dreg); + case EXPR_LIST: + return netlink_gen_flagcmp(ctx, expr, dreg); case EXPR_PREFIX: sreg = get_register(ctx, expr->left); if (expr_basetype(expr->left)->type != TYPE_STRING) { @@ -376,6 +382,11 @@ static void netlink_gen_cmp(struct netlink_linearize_ctx *ctx, } break; default: + if (expr->op == OP_IMPLICIT && + expr->right->dtype->basetype != NULL && + expr->right->dtype->basetype->type == TYPE_BITMASK) + return netlink_gen_flagcmp(ctx, expr, dreg); + sreg = get_register(ctx, expr->left); len = div_round_up(expr->right->len, BITS_PER_BYTE); right = expr->right; @@ -421,8 +432,8 @@ static void netlink_gen_range(struct netlink_linearize_ctx *ctx, nld.value, nld.len); nftnl_rule_add_expr(ctx->nlr, nle); break; - case OP_RANGE: case OP_EQ: + case OP_IMPLICIT: nle = alloc_nft_expr("cmp"); netlink_put_register(nle, NFTNL_EXPR_CMP_SREG, sreg); nftnl_expr_set_u32(nle, NFTNL_EXPR_CMP_OP, @@ -491,6 +502,7 @@ static void netlink_gen_relational(struct netlink_linearize_ctx *ctx, enum nft_registers dreg) { switch (expr->op) { + case OP_IMPLICIT: case OP_EQ: case OP_NEQ: case OP_LT: @@ -498,12 +510,6 @@ static void netlink_gen_relational(struct netlink_linearize_ctx *ctx, case OP_LTE: case OP_GTE: return netlink_gen_cmp(ctx, expr, dreg); - case OP_RANGE: - return netlink_gen_range(ctx, expr, dreg); - case OP_FLAGCMP: - return netlink_gen_flagcmp(ctx, expr, dreg); - case OP_LOOKUP: - return netlink_gen_lookup(ctx, expr, dreg); default: BUG("invalid relational operation %u\n", expr->op); } diff --git a/src/parser_bison.y b/src/parser_bison.y index 5f84d794079a7..1854a8700a048 100644 --- a/src/parser_bison.y +++ b/src/parser_bison.y @@ -3173,7 +3173,7 @@ relational_expr : expr /* implicit */ rhs_expr } | expr /* implicit */ list_rhs_expr { - $$ = relational_expr_alloc(&@$, OP_FLAGCMP, $1, $2); + $$ = relational_expr_alloc(&@$, OP_IMPLICIT, $1, $2); } | expr relational_op rhs_expr { diff --git a/src/rule.c b/src/rule.c index c5bf65933ba88..4334efacbbdf7 100644 --- a/src/rule.c +++ b/src/rule.c @@ -2130,6 +2130,16 @@ static int payload_match_stmt_cmp(const void *p1, const void *p2) return e1->left->payload.offset - e2->left->payload.offset; } +static bool relational_ops_match(const struct expr *e1, const struct expr *e2) +{ + enum ops op1, op2; + + op1 = e1->op == OP_IMPLICIT ? OP_EQ : e1->op; + op2 = e2->op == OP_IMPLICIT ? OP_EQ : e2->op; + + return op1 == op2; +} + static void payload_do_merge(struct stmt *sa[], unsigned int n) { struct expr *last, *this, *expr1, *expr2; @@ -2144,7 +2154,7 @@ static void payload_do_merge(struct stmt *sa[], unsigned int n) this = stmt->expr; if (!payload_can_merge(last->left, this->left) || - last->op != this->op) { + !relational_ops_match(last, this)) { last = this; j = i; continue; @@ -2227,6 +2237,7 @@ static void payload_try_merge(const struct rule *rule) continue; switch (stmt->expr->op) { case OP_EQ: + case OP_IMPLICIT: case OP_NEQ: break; default: -- 2.16.1 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
