Re: [PATCH nft,v2] cache: do not populate the cache in case of flush ruleset command

[Phil Sutter <phil@nwl.cc>]

Hi Pablo,

On Fri, Jun 14, 2019 at 03:04:38PM +0200, Pablo Neira Ayuso wrote:
> On Fri, Jun 14, 2019 at 02:59:10PM +0200, Pablo Neira Ayuso wrote:
> > On Fri, Jun 14, 2019 at 02:54:32PM +0200, Phil Sutter wrote:
> > > Hi Pablo,
> > > 
> > > On Fri, Jun 14, 2019 at 02:36:30PM +0200, Pablo Neira Ayuso wrote:
> > > > __CMD_FLUSH_RULESET is a dummy definition that used to skip the netlink
> > > > dump to populate the cache. This patch is a workaround until we have a
> > > > better infrastructure to track the state of the cache objects.
> > > 
> > > I assumed the problem wouldn't exist anymore since we're populating the
> > > cache just once. Can you maybe elaborate a bit on the problem you're
> > > trying to solve with that workaround?
> > 
> > If nft segfaults to dump the cache, 'nft flush ruleset' will not work
> > since it always fetches the cache, it will segfault too.
> > 
> > The flush ruleset command was still dumping the cache before this
> > patch.
> 
> In general, we still need to improve the cache logic, to make it finer
> grain. Now that we have a single point to populate the cache, things
> will get more simple. We need to replace the cache command level to
> cache flags or our own cache level definitions. The existing approach
> that uses of commands to define the cache level completeness has its
> own limitations. We can discuss this during the NFWS :-).

Yes, I had the same thought already. It is quite unintuitive how we link
cache completeness to certain commands. :)

Regarding your problem, maybe cache_update() should exit immediately if
passed cmd is CMD_INVALID? Unless I miss something, if cache_evaluate()
returns that value, we don't need a cache at all.

Cheers, Phil