Hi Pablo, On Fri, Sep 20, 2019 at 01:57:02PM +0200, Pablo Neira Ayuso wrote: [...] > Looking at patches from 8/24 onwards, I think it's time to introduce > cache flags logic to iptables. > > In patch 9/14 there is a new field have_chain_cache. > > In patch 10/14 there is have_rule_cache. > > Then moving on, the logic is based on checking for this booleans and > then checking if the caches are initialized or not. > > I think if we move towards cache flags logic (the flags tell us what > if we need no cache, partial cache (only tables, tables + chains) or > full cache. > > This would make this logic easier to understand and more maintainable. I am not entirely sure this is a feasible approach: On one hand, we certainly can introduce cache "levels" to distinguish between: - no cache - tables - chains - rules simply because all these naturally build upon each others. On the other hand, in my series I pushed the envelope (and watched it bend) a bit further: There are basically two modes of caching: - "traditional" breadth-first, compatible with above: e.g. all tables, if needed all chains as well, etc. - a new depth-first mode which allows to fetch e.g. a certain chain's rules but no other chains/rules. While the first mode is fine, second one really gives us the edge in situations where legacy iptables is faster simply because "number crunching" is more optimized there. This second mode doesn't have explicit completion indicators like the first one (with 'have_cache', etc.). In fact, the code treats it like a fire-and-forget situation: If e.g. the same chain is requested twice, the second time cache is simply fetched again but nftnl_chain_list_cb() discards the fetched chain if one with same name already exists in table's chain list. Actually, the only application which requires more attention is xtables-restore with --noflush. It is able to run multiple commands consecutively and these may need kernel ruleset as context. Still we don't want to fetch the full kernel ruleset upon each invocation because it's how users speed up their ruleset manipulations. In summary, things boil down to the following options: A) Avoid caching, fetch only the most necessary things to do the job. B) Build a full cache if needed anyway or if we can't predict. C) Create a fake cache if we know kernel's ruleset is irrelevant. I'll give it another try, aligning cache update logic to the above and see if things become clean enough to be considered maintainable. Cheers, Phil
