Re: [PATCH nft 3/5] src: add cache level flags

[Phil Sutter <phil@nwl.cc>]

On Mon, Jun 17, 2019 at 07:33:29PM +0200, Pablo Neira Ayuso wrote:
> On Mon, Jun 17, 2019 at 07:28:31PM +0200, Phil Sutter wrote:
> > On Mon, Jun 17, 2019 at 07:24:33PM +0200, Pablo Neira Ayuso wrote:
> > > On Mon, Jun 17, 2019 at 06:45:59PM +0200, Phil Sutter wrote:
> > > > On Mon, Jun 17, 2019 at 06:28:40PM +0200, Pablo Neira Ayuso wrote:
> > > > > On Mon, Jun 17, 2019 at 06:11:04PM +0200, Phil Sutter wrote:
> > > > > > Hi,
> > > > > > 
> > > > > > On Mon, Jun 17, 2019 at 02:25:16PM +0200, Pablo Neira Ayuso wrote:
> > > [...]
> > > > > 
> > > > > We need these for references to sets, eg.
> > > > > 
> > > > >         add rule x y ip saddr @x
> > > > > 
> > > > > same for other flowtable and object.
> > > > 
> > > > Oh, right. I got that wrong - old code is always fetching the above
> > > > items unless there's no ruleset in kernel (i.e., returned genid is 0).
> > > > 
> > > > I confused that with fetching rules which at some point started to
> > > > happen by accident with my changes.
> > > > 
> > > > > We should not use NFT_CACHE_RULE in this case, if this is what you
> > > > > suggest.
> > > > 
> > > > No, quite the opposite: I thought we could get by without fetching
> > > > anything from kernel at all.
> > > > 
> > > > Yet now I wonder why the handle guessing stops working, because the
> > > > above can't be the cause of it.
> > > 
> > > I think we should partial revert the changes that are doing the
> > > handle guessing, would you submit a patch for this?
> > 
> > There are no explicit changes allowing for it. The reason it works is
> > because user space doesn't care to check the given handle for existence
> > and in kernel space it is valid already.
> 
> OK, but this test just works because we are being lucky in
> guessing, right? I don't think we should not have a test for handles
> from the batch, for an unexisting rule in the kernel.

Yes, that's right. The test in question was added back in 2016 to make
sure the separate commands syntax works with 'nft -f'. I guess it was
simply convenient to add all the different commands to the same
transaction.

Cheers, Phil