Re: [PATCH nft 2/7] netlink_delinearize: postprocess binary ands in set expressions

[Florian Westphal <fw@strlen.de>]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> Hi Florian,
> 
> On Wed, Jul 27, 2022 at 01:19:58PM +0200, Florian Westphal wrote:
> [..]
> > diff --git a/include/netlink.h b/include/netlink.h
> > index e8e0f68ae1a4..2d5532387c0c 100644
> > --- a/include/netlink.h
> > +++ b/include/netlink.h
> [...]
> > @@ -2569,6 +2582,24 @@ static void expr_postprocess(struct rule_pp_ctx *ctx, struct expr **exprp)
> >  			expr_set_type(expr->right, &integer_type,
> >  				      BYTEORDER_HOST_ENDIAN);
> >  			break;
> > +		case OP_AND:
> > +			expr_set_type(expr->right, expr->left->dtype,
> > +				      expr->left->byteorder);
> > +
> > +			/* Only process OP_AND if we are inside a concatenation.
> > +			 *
> > +			 * Else, we remove it too early, for normal contect OP_AND
> > +			 * removal needs to be performed as part of the relational
> > +			 * operation because the RHS constant might need to be adjusted
> > +			 * (shifted).
> > +			 */
> > +			if ((ctx->flags & RULE_PP_IN_CONCATENATION) &&
> > +			    expr->left->etype == EXPR_PAYLOAD &&
> > +			    expr->right->etype == EXPR_VALUE) {
> > +				__binop_postprocess(ctx, expr, expr->left, expr->right, exprp);
> > +				return;
> > +			}
> > +			break;
> 
> Not sure this flag is enough. If I load this ruleset
> 
>  table netdev nt {
>        set macset {
>                typeof vlan id
>                size 1024
>                flags dynamic,timeout
>        }
>         chain y {
>         }
>  }
>  add rule netdev nt y update @macset { vlan id timeout 5s }
> 
> listing still shows the raw expression:
> 
>  table netdev nt {
>         set macset {
>                 typeof vlan id
>                 size 1024
>                 flags dynamic,timeout
>         }
> 
>         chain y {
>                 update @macset { @ll,112,16 & 0xfff timeout 5s }
>         }
>  }
> 
> looks like the problem is related to expressions in set statements?

Good catch, I added a patch to the series that also enabled OP_AND
removal for the set elem key case.

I took the above and added it to the test case.