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 <phil@xxxxxx>
---
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
