Hi Jeremy,
On Tue, Mar 10, 2020 at 09:30:08AM +0000, Jeremy Sowden wrote:
> On 2020-03-10, at 03:39:13 +0100, Pablo Neira Ayuso wrote:
> > On Tue, Mar 03, 2020 at 09:48:44AM +0000, Jeremy Sowden wrote:
> > [...]
> > > diff --git a/tests/py/any/ct.t.payload b/tests/py/any/ct.t.payload
> > > index 661591257804..17a1c382ea65 100644
> > > --- a/tests/py/any/ct.t.payload
> > > +++ b/tests/py/any/ct.t.payload
> > > @@ -359,6 +359,39 @@ ip test-ip4 output
> > > [ lookup reg 1 set __map%d dreg 1 ]
> > > [ ct set mark with reg 1 ]
> > >
> > > +# ct mark set ct mark and 0xffff0000 or meta mark and 0xffff
> > > +ip
> > > + [ ct load mark => reg 1 ]
> > > + [ bitwise reg 1 = (reg=1 & 0xffff0000 ) ^ 0x00000000 ]
> >
> > These two are: ct mark and 0xffff0000
> >
> > > + [ meta load mark => reg 2 ]
> > > + [ bitwise reg 2 = (reg=2 & 0x0000ffff ) ^ 0xffffffff ]
> >
> > Refetch.
> >
> > > + [ meta load mark => reg 3 ]
> > > + [ bitwise reg 3 = (reg=3 & 0x0000ffff ) ^ 0x00000000 ]
> >
> > These two are: meta mark and 0xffff
> >
> > > + [ bitwise reg 1 = (reg=1 & reg 2 ) ^ reg 3 ]
> >
> > This one is triggering the refetch from meta load in reg 2, right?
> >
> > If so, probably extend nft_bitwise to support for 'or' from two
> > registers would make things more simple?
> >
> > [ bitwise reg 1 = (reg 1 | reg 3) ]
> >
> > This one requires two registers as input for this new OR operation.
> >
> > > + [ ct set mark with reg 1 ]
> > > +
> > [...]
> > > diff --git a/tests/py/ip/ip.t.payload b/tests/py/ip/ip.t.payload
> > > index d627b22f2614..d6c5d14d52ac 100644
> > > --- a/tests/py/ip/ip.t.payload
> > > +++ b/tests/py/ip/ip.t.payload
> > [...]
> > > +# iif "lo" ip dscp set ip dscp or 0x3
> > > +ip
> > > + [ meta load iif => reg 1 ]
> > > + [ cmp eq reg 1 0x00000001 ]
> > > + [ payload load 2b @ network header + 0 => reg 1 ]
> > > + [ bitwise reg 1 = (reg=1 & 0x000003ff ) ^ 0x00000000 ]
> > > + [ payload load 1b @ network header + 1 => reg 2 ]
> > > + [ bitwise reg 2 = (reg=2 & 0x000000fc ) ^ 0x00000000 ]
> > > + [ bitwise reg 2 = ( reg 2 >> 0x00000002 ) ]
> > > + [ bitwise reg 2 = (reg=2 & 0x000000fc ) ^ 0x00000003 ]
> > > + [ bitwise reg 2 = ( reg 2 << 0x00000002 ) ]
> > > + [ bitwise reg 1 = (reg=1 & 0x0000ffff ) ^ reg 2 ]
> > > + [ payload write reg 1 => 2b @ network header + 0 csum_type 1 csum_off 10 csum_flags 0x0 ]
> >
> > Probably extending nft_bitwise again is the way to go to simplify
> > this?
> >
> > 1) fetch two bytes from payload => reg 1.
> > 2) reg 2 = ( reg 1 | 0x000c )
> >
> > userspace 0x3 << 2 => 0x0c, then extend this to two bytes => 0x000c
> >
> > This is an OR with immediate value.
> >
> > 3) payload write reg 1
> >
> > This one requires two immediates.
> >
> > Then, how does 'ip dscp set ip dscp and 0x01' bytecode looks like?
> >
> > 1) fetch two bytes => reg 1.
> > 2) reg 1 = (reg 1 & 0xff07) ^ 0x0
> >
> > userspace 0x01 => 0x04 (after << 2). Then, 0x04 & 0xff03 = 0xff07.
> >
> > This case should be possible to support it with the existing bitwise.
> >
> > The delinearization path will need to calculate the closest field
> > matching, but there is already code for this in the userspace tree (it
> > was required when matching ip dscp using bitwise operation).
> >
> > Would it be possible to simplify all this through new kernel
> > extension? If so, I'm sorry for wasting resources, this might go to a
> > different direction than _MREG and _XREG.
>
> No problem. :)
>
> > Moreover, for field updates like in these examples, I wonder if it is
> > worth to introduce a new syntax, ie.
> >
> > ip dscp |= 0x01
> > ip dscp or_eq 0x01
> >
> > ip dscp &= 0x01
> > ip dscp and_eq 0x01
> >
> > | and & might be a problem for the shell, for the native nft cli this
> > should be fine. But this is a different issue.
>
> Thanks for the feedback, Pablo.
Do you think it would be to keep back this one from the nf-next tree
until you evaluate an alternative way to extend nft_bitwise?
commit 8d1f378a51fcf2f5e44e06ff726a91c885d248cc
Author: Jeremy Sowden <jeremy@xxxxxxxxxx>
Date: Mon Feb 24 12:49:31 2020 +0000
netfilter: bitwise: add support for passing mask and xor values in registers.
Thanks.
