Hi Phil, Thanks a lot for working on this, I have a few comments. On Tue, Jun 04, 2019 at 07:31:48PM +0200, Phil Sutter wrote: > Next round of combined cache update fix and intra-transaction rule > reference support. Patch 1 looks good. > Patch 2 is new, it avoids accidential cache updates when committing a > transaction containing flush ruleset command and kernel ruleset has > changed meanwhile. Patch 2: Could you provide an example scenario for this new patch? > Patch 3 is also new: If a transaction fails in kernel, local cache is > incorrect - drop it. Patch 3 looks good! Regarding patches 4, 5 and 6. I think we can skip them if we follow the approach described by [1], given there is only one single cache_update() call after that patchset, we don't need to do the "Restore local entries after cache update" logic. [1] https://marc.info/?l=netfilter-devel&m=155975322308042&w=2 > Patch 9 is a new requirement for patch 10 due to relocation of new > functions. > > Patch 10 was changed, changelog included. Patch 10 looks fine. However, as said, I would like to avoid the patch dependencies 4, 5 and 6, they are adding more cache_update() calls and I think we should go in the opposite direction to end up with a more simple approach. Thanks!
