On Sun, Nov 05, 2023 at 06:41:39PM +0100, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> 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.
>
> Even before above commit.
After my patch:
https://patchwork.ozlabs.org/project/netfilter-devel/patch/20231105174148.160390-1-pablo@xxxxxxxxxxxxx/
chain output {
type filter hook output priority filter; policy accept;
meta mark set ip6 dscp map @mapv6
meta mark 0x0000002a counter packets 2 bytes 208
ip6 dscp @mapv6 counter packets 0 bytes 0
ip6 dscp cs1 counter packets 2 bytes 208
}
So 'ip6 dscp @mapv6' is still broken.
With your patch, simple 'ip6 dscp cs1' breaks:
@nh,0,16 & 0xfc0 & 0xfc0 == 0x2 counter packets 0 bytes 0
