Re: ip6 dscp fails map lookup

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Sun, Nov 05, 2023 at 06:41:39PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > >  	if (shift) {
> > > > -		if (ctx->stmt_len > 0 && div_round_up(masklen, BITS_PER_BYTE) > 1) {
> > > > +		if (masklen > BITS_PER_BYTE) {
> > > 
> > > I think this is right but binop xfer
> > > won't remove the inserted byteorder conversion
> > > in case the shift is to be removed by adjusting
> > > a constant right hand side value.
> > 
> > ctx->stmt_len > 0 is also set from stmt_evaluate_{ct,meta}() so this
> > transformation is restricted to statements.
> > 
> > I was conservative edecd58755a8 with ("evaluate: support shifts larger than
> > the width of the left operand") to restrict this transformation to
> > statements only.
> > 
> > Do you think think this applies to other cases too?
> > 
> > BTW, whatever the final fix is, Fixes: tag for this should be:
> > 
> > Fixes: 668c18f67203 ("evaluate: place byteorder conversion before rshift in payload statement")
> 
> I don't think this is a regression, I do not think it has worked before.
> 
> table ip6 t {
> 	map mapv6 {
> 		typeof ip6 dscp : meta mark
> 		elements = { cs1 : 0x0000002a }
> 	}
> 
> 	chain output {
> 		type filter hook output priority filter; policy accept;
> 		meta mark set ip6 dscp map @mapv6
> 		meta mark 0x0000002a counter
> 		ip6 dscp @mapv6 counter
> 		ip6 dscp cs1 counter
> 	}
> }
> 
> A "ping6 -q -Q 0x20 ::1 -c1" should cause all counters
> in above ruleset to increment, but that is not the case.

Thanks for your simple test, it is passing with these two patches:

https://patchwork.ozlabs.org/project/netfilter-devel/patch/20231105174148.160390-1-pablo@netfilter.org/
https://patchwork.ozlabs.org/project/netfilter-devel/patch/20231105212846.311755-1-pablo@netfilter.org/

I have extended it to also cover for dynamic sets:

table ip6 t {
        map mapv6 {
                typeof ip6 dscp : meta mark
                elements = { cs1 : 0x0000002a }
        }

        map mapv62 {
                typeof ip6 dscp : meta mark
                size 65535
                flags dynamic
        }

        chain output {
                type filter hook output priority filter; policy accept;
                meta mark set ip6 dscp map @mapv6
                meta mark 0x0000002a counter
                ip6 dscp @mapv6 counter
                ip6 dscp cs1 counter
                update @mapv62 { ip6 dscp : meta mark }
        }
}

I just quickly checked for concatenations with ip6 dscp, it seems the
byteorder conversion is also missing there, so v2 will follow up.