On Fri, Nov 22, 2024 at 02:43:27PM +0100, Florian Westphal wrote: > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: > > > Sure, wasn't that the reason why you iniitially wanted to restrict this to > > > --netlink=debug? What made you change your mind? > > > > With large garbage collection cycle, this counter provides a hint to > > the user to understand that slots are still being consumed by expired > > elements. > > But how / where is that relevant? > > rbtree does gc at insert time. We could extend rbtree to force gc > even if interval is huge in case we have many expired elements. > > We could do this by making __nft_rbtree_insert() count the number > of expired nodes that it saw during traversal, then force gc at commit > time even if time_after_eq() isn't met. IIRC, rbtree insert path already performs gc on-demand. > > > Maybe apply the simpler, existing v1 patches only, i.e. no exposure? > > > > My concern is that this is exposing this implementation detail of the > > rbtree, forever. Can we agree to do heuristics to hide this detail: > > > > Assuming initial 0.0.0.0 dummy element is in place (this can be > > subtracted), then, division by two gives us the number of ranges. > > Ouch. This either means more kernel complexity and lie to userspace, > or leak rbtree details into nft, basically strcmp on the new > SET_TYPE nlattr string and then display something else on frontend side. Yes, this is lying to userspace to hide the implementation details. I would really like to provide an alternative interface for the rbtree to allow for the same netlink representation as pipapo. I expected pipapo can replace rbtree by pipapo, but you mentioned in the past this could be an issue. > I'd prefer to avoid this mess. OK, then we assume this will be forever used for debugging only, unless rbtree is fully replaced. Please, let me have a look, if I fail or it is too ugly you can still ditch it and we can follow up with your approach. Thanks.
