On Thu, Mar 10, 2022 at 01:21:57PM +0100, Florian Westphal wrote: > Phil Sutter <phil@xxxxxx> wrote: > > This is kind of a double-edged blade: the obvious downside is that > > *tables-restore won't detect user-defined chain name and extension > > clashes anymore. The upside is a tremendous performance improvement > > restoring large rulesets. The same crooked ruleset as mentioned in > > earlier patches (50k chains, 130k rules of which 90k jump to a chain) > > yields these numbers: > > > > variant unoptimized non-targets cache announced chains > > ---------------------------------------------------------------- > > legacy 1m12s 37s 2.5s > > nft 1m35s 53s 8s > > I think the benefits outweight the possible issues. > > > Note that iptables-legacy-restore allows the clashes already as long as > > the name does not match a standard target, but with this patch it stops > > warning about it. > > Hmm. That seems fixable by refusing the announce in the clash case? When parsing a chain line, iptables-restore does not know there is a clash because this series effectively disables that check. Due to the non-targets hash, any chain name is looked up (as target) only once anyway, so keeping that check in iptables-restore yields the same performance as without the annotate. > > iptables-nft-restore does not care at all, even allows > > adding a chain named 'ACCEPT' (and rules can't reach it because '-j > > ACCEPT' translates to a native nftables verdict). The latter is a bug by > > itself. > > Agree, thats a bug, it should not allow users to do that. ACK, I'll find a fix. In legacy, libiptc (TC_CREATE_CHAIN) does it, so an nft-specific one it will be. Thanks, Phil
