Hi Phil,
On Wed, Aug 29, 2018 at 11:52:09AM +0200, Phil Sutter wrote:
> Hi,
>
> I found constellations in which userspace ruleset cache maintenance
> causes headaches. A "simple" case is this:
>
> | # nft flush ruleset
> | # nft -i
> | nft> add table ip t
> | nft> list ruleset
> | table ip t {
> | }
> | table ip t {
> | }
>
> Bisect returned 4aba100e593f2 ("rule: reset cache iff there is an
> existing cache"). The problem here is 'add table ip t' creates a cache
> entry manually and in call to cache_update() for 'list ruleset',
> cache_release() is not called because cache->genid is still zero. So
> cache_init() just adds an entry for the table to the cache despite it
> being there already.
>
> Here's a more complex one:
>
> | # ./src/nft -i
> | nft> list ruleset
> | nft> add table ip t
> | nft> add table ip t2 ; add chain ip t2 c
> | Error: Could not process rule: Table 't2' does not exist
> | add table ip t2 ; add chain ip t2 c
> | ^^^^^^^^^^^^^^^^^^
>
> The first call to 'list ruleset' causes cache->genid to be non-zero.
> Adding the first table does not trigger a cache update, but kernel's
> genid increments as a result. The third command line contains two
> commands which are handled in a single batch: As before, adding table t2
> does not trigger a cache update. Evaluation phase for adding a chain
> though does. And since cache->genid is non-zero and does not match
> kernel's genid, cache is flushed which incorrectly removes table t2 from
> it.
>
> It seems like current cache management is way too simple and we need a
> real cache_update() routine which doesn't just (conditionally) flush and
> append entries from kernel, but drops/updates local entries and adds new
> ones. What do you think?
Yes, we currently have lazy object caching which was the initial
approach. Finer grain object tracking, without the need of constant
flushing, would be great to have indeed.
Cache bits are slightly tricky, so as usual, tests covering all these
cases would be good to have in place, to make sure we have no
regressions.
Thanks!
