Re: [nft PATCH 1/4] xt: Delay libxtables access until translation

[Phil Sutter <phil@nwl.cc>]

Hi Pablo,


On Thu, Dec 08, 2022 at 10:21:02PM +0100, Pablo Neira Ayuso wrote:
> On Thu, Nov 24, 2022 at 05:56:38PM +0100, Phil Sutter wrote:
> > There is no point in spending efforts setting up the xt match/target
> > when it is not printed afterwards. So just store the statement data from
> > libnftnl in struct xt_stmt and perform the extension lookup from
> > xt_stmt_xlate() instead.
> 
> There is nft -i and nft monitor which keep a ruleset cache. Both are
> sort of incomplete: nft -i resorts to cleaning up the cache based on
> the generation number and nft monitor still needs to be updated to
> keep track of incremental ruleset updates via netlink events. Sooner
> or later these two will get better support for incremental ruleset
> updates.
> 
> I mean, in those two cases, every call to print the translation will
> trigger the allocation of the xt structures, fill them and then call
> .xlate. I agree it is a bit more work, I guess this won't case any
> noticeable penalty, but it might be work that needs to be done over
> and over again when ruleset uses xt match / target.

So you're saying the overhead when printing a rule might be more
significant than when fetching it. I doubt this simply because the same
rule is usually printed at most once and there are multiple other
commands requiring a rule cache.

IMO we may also just leave the code as-is and wait for someone to
complain about bad performance with rulesets containing many compat
expressions. Depending on the actual report, we may also follow a hybrid
approach and do the match/target lookup only when needed and cache it
for later use.

My patch made most sense with an nft in mind which does not need xtables
support for saving/restoring compat expressions. Users depending on this
for whatever reason will execute the xlate code path in any case now.

Cheers, Phil