| Message ID | 20220404121410.188509-10-jeremy@azazel.net |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | Extend values assignable to packet marks and payload fields | expand |
On Mon, Apr 04, 2022 at 01:13:47PM +0100, Jeremy Sowden wrote: > If a user uses a payload expression as a statement argument: > > nft add rule t c meta mark set ip dscp lshift 2 or 0x10 > > we may need to undo munging during delinearization. > > Signed-off-by: Jeremy Sowden <jeremy@azazel.net> > --- > src/netlink_delinearize.c | 39 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c > index 733977bc526d..12624db4c3a5 100644 > --- a/src/netlink_delinearize.c > +++ b/src/netlink_delinearize.c > @@ -2454,6 +2454,42 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx, > } > } > > +static bool payload_binop_postprocess(struct rule_pp_ctx *ctx, > + struct expr **exprp) > +{ > + struct expr *expr = *exprp; > + > + if (expr->op != OP_RSHIFT) > + return false; > + > + if (expr->left->etype == EXPR_UNARY) { > + /* > + * If the payload value was originally in a different byte-order > + * from the payload expression, there will be a byte-order > + * conversion to remove. > + */ The comment assumes this is a payload expression, the unary is stripped off here... > + struct expr *left = expr_get(expr->left->arg); > + expr_free(expr->left); > + expr->left = left; > + } > + > + if (expr->left->etype != EXPR_BINOP || expr->left->op != OP_AND) > + return false; > + > + if (expr->left->left->etype != EXPR_PAYLOAD) ... but the check for payload is coming here. I assume this postprocessing is to undo the switch from network byteorder to host byteorder for the ip dscp of the example above? Could you describe an example expression tree to depict this delinearize scenario? > + return false; > + > + expr_set_type(expr->right, &integer_type, > + BYTEORDER_HOST_ENDIAN); > + expr_postprocess(ctx, &expr->right); > + > + binop_postprocess(ctx, expr, &expr->left); > + *exprp = expr_get(expr->left); > + expr_free(expr); > + > + return true; > +} > + > static struct expr *string_wildcard_expr_alloc(struct location *loc, > const struct expr *mask, > const struct expr *expr) > @@ -2566,6 +2602,9 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp) > expr_set_type(expr, expr->arg->dtype, !expr->arg->byteorder); > break; > case EXPR_BINOP: > + if (payload_binop_postprocess(ctx, exprp)) > + break; > + > expr_postprocess(ctx, &expr->left); > switch (expr->op) { > case OP_LSHIFT: > -- > 2.35.1 >
On 2022-05-23, at 19:19:08 +0200, Pablo Neira Ayuso wrote: > On Mon, Apr 04, 2022 at 01:13:47PM +0100, Jeremy Sowden wrote: > > If a user uses a payload expression as a statement argument: > > > > nft add rule t c meta mark set ip dscp lshift 2 or 0x10 > > > > we may need to undo munging during delinearization. > > > > Signed-off-by: Jeremy Sowden <jeremy@azazel.net> > > --- > > src/netlink_delinearize.c | 39 +++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 39 insertions(+) > > > > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c > > index 733977bc526d..12624db4c3a5 100644 > > --- a/src/netlink_delinearize.c > > +++ b/src/netlink_delinearize.c > > @@ -2454,6 +2454,42 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx, > > } > > } > > > > +static bool payload_binop_postprocess(struct rule_pp_ctx *ctx, > > + struct expr **exprp) > > +{ > > + struct expr *expr = *exprp; > > + > > + if (expr->op != OP_RSHIFT) > > + return false; > > + > > + if (expr->left->etype == EXPR_UNARY) { > > + /* > > + * If the payload value was originally in a different byte-order > > + * from the payload expression, there will be a byte-order > > + * conversion to remove. > > + */ > > The comment assumes this is a payload expression, the unary is > stripped off here... > > > + struct expr *left = expr_get(expr->left->arg); > > + expr_free(expr->left); > > + expr->left = left; > > + } > > + > > + if (expr->left->etype != EXPR_BINOP || expr->left->op != OP_AND) > > + return false; > > + > > + if (expr->left->left->etype != EXPR_PAYLOAD) > > ... but the check for payload is coming here. Will fix. > I assume this postprocessing is to undo the switch from network > byteorder to host byteorder for the ip dscp of the example above? > > Could you describe an example expression tree to depict this > delinearize scenario? Currently, demunging is only done for payload statement expressions, in `stmt_payload_postprocess`. However, this patch-set will lead to the appearance of munged payload expressions in other contexts, such as the example given above in the commit message: nft add rule t c meta mark set ip dscp lshift 2 or 0x10 The expression tree for the value assigned to `meta mark` is: OR / \ LSHIFT 0x10 / \ RSHIFT 2 / \ AND 2 / \ @nh(8,8) 0xfc and the `@nh(8,8) & 0xfc >> 2` expression needs to be demunged to `ip dscp`. > > + return false; > > + > > + expr_set_type(expr->right, &integer_type, > > + BYTEORDER_HOST_ENDIAN); > > + expr_postprocess(ctx, &expr->right); > > + > > + binop_postprocess(ctx, expr, &expr->left); > > + *exprp = expr_get(expr->left); > > + expr_free(expr); > > + > > + return true; > > +} > > + > > static struct expr *string_wildcard_expr_alloc(struct location *loc, > > const struct expr *mask, > > const struct expr *expr) > > @@ -2566,6 +2602,9 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp) > > expr_set_type(expr, expr->arg->dtype, !expr->arg->byteorder); > > break; > > case EXPR_BINOP: > > + if (payload_binop_postprocess(ctx, exprp)) > > + break; > > + > > expr_postprocess(ctx, &expr->left); > > switch (expr->op) { > > case OP_LSHIFT: > > -- > > 2.35.1 > > >
diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c index 733977bc526d..12624db4c3a5 100644 --- a/src/netlink_delinearize.c +++ b/src/netlink_delinearize.c @@ -2454,6 +2454,42 @@ static void relational_binop_postprocess(struct rule_pp_ctx *ctx, } } +static bool payload_binop_postprocess(struct rule_pp_ctx *ctx, + struct expr **exprp) +{ + struct expr *expr = *exprp; + + if (expr->op != OP_RSHIFT) + return false; + + if (expr->left->etype == EXPR_UNARY) { + /* + * If the payload value was originally in a different byte-order + * from the payload expression, there will be a byte-order + * conversion to remove. + */ + struct expr *left = expr_get(expr->left->arg); + expr_free(expr->left); + expr->left = left; + } + + if (expr->left->etype != EXPR_BINOP || expr->left->op != OP_AND) + return false; + + if (expr->left->left->etype != EXPR_PAYLOAD) + return false; + + expr_set_type(expr->right, &integer_type, + BYTEORDER_HOST_ENDIAN); + expr_postprocess(ctx, &expr->right); + + binop_postprocess(ctx, expr, &expr->left); + *exprp = expr_get(expr->left); + expr_free(expr); + + return true; +} + static struct expr *string_wildcard_expr_alloc(struct location *loc, const struct expr *mask, const struct expr *expr) @@ -2566,6 +2602,9 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp) expr_set_type(expr, expr->arg->dtype, !expr->arg->byteorder); break; case EXPR_BINOP: + if (payload_binop_postprocess(ctx, exprp)) + break; + expr_postprocess(ctx, &expr->left); switch (expr->op) { case OP_LSHIFT:
If a user uses a payload expression as a statement argument: nft add rule t c meta mark set ip dscp lshift 2 or 0x10 we may need to undo munging during delinearization. Signed-off-by: Jeremy Sowden <jeremy@azazel.net> --- src/netlink_delinearize.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)