diff mbox series

[nft,v4,10/32] netlink_delinearize: correct type and byte-order of shifts

Message ID 20220404121410.188509-11-jeremy@azazel.net
State Changes Requested
Delegated to: Pablo Neira
Headers show
Series Extend values assignable to packet marks and payload fields | expand

Commit Message

Jeremy Sowden April 4, 2022, 12:13 p.m. UTC
Shifts are of integer type and in HBO.

Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
---
 src/netlink_delinearize.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Pablo Neira Ayuso May 23, 2022, 5:19 p.m. UTC | #1
On Mon, Apr 04, 2022 at 01:13:48PM +0100, Jeremy Sowden wrote:
> Shifts are of integer type and in HBO.
> 
> Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> ---
>  src/netlink_delinearize.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> index 12624db4c3a5..8b010fe4d168 100644
> --- a/src/netlink_delinearize.c
> +++ b/src/netlink_delinearize.c
> @@ -2618,8 +2618,17 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
>  		}
>  		expr_postprocess(ctx, &expr->right);
>  
> -		expr_set_type(expr, expr->left->dtype,
> -			      expr->left->byteorder);
> +		switch (expr->op) {
> +		case OP_LSHIFT:
> +		case OP_RSHIFT:
> +			expr_set_type(expr, &integer_type,
> +				      BYTEORDER_HOST_ENDIAN);
> +			break;
> +		default:
> +			expr_set_type(expr, expr->left->dtype,
> +				      expr->left->byteorder);

This is a fix?

If so, would it be possible to provide a standalone example that shows
what this is fixing up?

> +		}
> +
>  		break;
>  	case EXPR_RELATIONAL:
>  		switch (expr->left->etype) {
> -- 
> 2.35.1
>
Jeremy Sowden Nov. 1, 2022, 6:47 p.m. UTC | #2
On 2022-05-23, at 19:19:14 +0200, Pablo Neira Ayuso wrote:
> On Mon, Apr 04, 2022 at 01:13:48PM +0100, Jeremy Sowden wrote:
> > Shifts are of integer type and in HBO.
> > 
> > Signed-off-by: Jeremy Sowden <jeremy@azazel.net>
> > ---
> >  src/netlink_delinearize.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
> > index 12624db4c3a5..8b010fe4d168 100644
> > --- a/src/netlink_delinearize.c
> > +++ b/src/netlink_delinearize.c
> > @@ -2618,8 +2618,17 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
> >  		}
> >  		expr_postprocess(ctx, &expr->right);
> >  
> > -		expr_set_type(expr, expr->left->dtype,
> > -			      expr->left->byteorder);
> > +		switch (expr->op) {
> > +		case OP_LSHIFT:
> > +		case OP_RSHIFT:
> > +			expr_set_type(expr, &integer_type,
> > +				      BYTEORDER_HOST_ENDIAN);
> > +			break;
> > +		default:
> > +			expr_set_type(expr, expr->left->dtype,
> > +				      expr->left->byteorder);
> 
> This is a fix?
> 
> If so, would it be possible to provide a standalone example that shows
> what this is fixing up?

Without this, listing a rule like:

  ct mark set ip dscp lshift 2 or 0x10

will return:

  ct mark set ip dscp << 2 | cs2

because the type of the OR's right operand will be transitively derived
from `ip dscp`.  However, this is not valid syntax:

  # nft add rule t c ct mark set ip dscp '<<' 2 '|' cs2
  Error: Could not parse integer
  add rule t c ct mark set ip dscp << 2 | cs2
                                          ^^^

> > +		}
> > +
> >  		break;
> >  	case EXPR_RELATIONAL:
> >  		switch (expr->left->etype) {
> > -- 
> > 2.35.1
> > 
>
diff mbox series

Patch

diff --git a/src/netlink_delinearize.c b/src/netlink_delinearize.c
index 12624db4c3a5..8b010fe4d168 100644
--- a/src/netlink_delinearize.c
+++ b/src/netlink_delinearize.c
@@ -2618,8 +2618,17 @@  static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
 		}
 		expr_postprocess(ctx, &expr->right);
 
-		expr_set_type(expr, expr->left->dtype,
-			      expr->left->byteorder);
+		switch (expr->op) {
+		case OP_LSHIFT:
+		case OP_RSHIFT:
+			expr_set_type(expr, &integer_type,
+				      BYTEORDER_HOST_ENDIAN);
+			break;
+		default:
+			expr_set_type(expr, expr->left->dtype,
+				      expr->left->byteorder);
+		}
+
 		break;
 	case EXPR_RELATIONAL:
 		switch (expr->left->etype) {