Re: [PATCH nft] src: add interface wildcard matching

[Patrick McHardy <kaber@trash.net>]

On 18.10, Florian Westphal wrote:
> Patrick McHardy <kaber@trash.net> wrote:
> > On 18.10, Florian Westphal wrote:
> > > Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> > > > Contrary to iptables, we use '*' as wildcard as in udev since the '+' can be
> > > > used as a valid interface name.
> > > 
> > > '*' can also be part of an interface name, seems only '/', ':', and ' '
> > > (space) are disallowed.
> > > 
> > > >  # nft --debug=netlink add rule test test iifname eth\*
> > > >  ip test test
> > > >    [ meta load iifname => reg 1 ]
> > > >    [ bitwise reg 1 = (reg=1 & 0x00ffffff 0x00000000 0x00000000 0x00000000 ) ^ 0x00000000 0x00000000 0x00000000 0x00000000 ]
> > > >    [ cmp eq reg 1 0x2a687465 0x00000000 0x00000000 0x00000000 ]
> > > 
> > > Why do we need a bitwise op for this?
> > > 
> > > Instead we could just ask for cmp of 3 bytes ('eth' instead of 4 'eth\0')?
> > > 
> > > You might recall ancient RFC patch for this:
> > > https://patchwork.ozlabs.org/patch/283639/
> > 
> > This is actually something I think should be implemented as general
> > optimzation. It also applies to network address matches, where we
> > can also avoid loading unnecessary data. Other cases will benefit
> > from this as well.
> 
> Sorry, I'm dense.
> 
> So what you're saying is that Pablos patch with bitwise op + cmp is fine
> and that you'd suggest to later add code to e.g. netlink serialize step
> that checks if bitop+cmp can be replaced by a single cmp operation?

I wouldn't add Pablo's patch as is because I don't think the wildcard string
special casing is the right thing to do. I'd say it should probably use a
prefix expression since that's what it actually is.

Regarding the optimization, I'm not necessarily saying later, just that it
should be done in a way that is not specific to strings.