Re: [PATCH] netfilter: Reverse nft_set_lookup_byid list traversal

[Jan-Philipp Litza <jpl@plutex.de>]

Hi Pablo,

> If the .nft file contains lots of (linear syntax):
> 
> add rule x y ... { ... }
> ...
> add rule x y ... { ... }
> 
> then, this patch is a real gain. In this case, nft currently places
> the new anonymous set right before the rule, so your patch makes it
> perform nicely.
> 
> I hesitate with the nested syntax, ie.
> 
> table x {
>        chain y {
>                 ... { ... }
>                 ...
>                 ... { ... }
>        }
> }
> 
> In this case, nft adds all the anonymous sets at the beginning of the
> netlink message, then rules don't find it right at the end.

Maybe I don't quite understand "at the beginning of the netlink message"
the way you meant it, but we are actually using nested syntax - just
with hundreds of (short) chains - and the performance gains I cited were
from this ruleset, which basically looks like

table filter {
	chain if1 {
		tcp dport 22 ip saddr { x, y, z } accept
	}
}
table filter {
	chain if2 {
		ip saddr { a, b, c } accept
		tcp dport 80 accept
	}
}
...

(Yes, the "table filter" is repeated every time, because the ruleset is
generated. Don't know if that matters.)

So I suspect that nft adds the anonymous sets maybe not immediately
before the elements, but maybe at the beginning of the chain (or the
beginning of the table block, which we repeat).

But maybe, if I have one chain with hundreds of rules, then this patch
degrades loading performance.

> Probably it's better to convert this code to use a rhashtable for fast
> lookups on the transaction so we don't mind about what userspace does
> in the future.

I totally agree. As a non-kernel-hacker, however, this was out of reach
for me. ;-)

Best regards,
Jan-Philipp Litza