On Tue, Jun 14, 2022 at 11:58:14AM +0200, Stefano Brivio wrote: > On Mon, 6 Jun 2022 11:01:21 +0200 > Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote: [...] > > That sounds an incremental fix, I prefer this too. > > ...finally posted now. Thanks. [...] > > I don't see how we can obsolete "activate" operation, though, the > > existing approach works at set element granularity. > > Yes, and that's what I'm arguing against: it would be more natural, in > a transaction, to have a single commit operation for all the elements > at hand -- otherwise it's not so much of a transaction. > > To the user it's atomic (minus bugs) because we have tricks to ensure > it, but to the set back-ends it's absolutely not. I think we have this > kind of situation: > > > nft <-> core <-> set back-end <-> storage > | | | > > hash: transaction commit element commit element commit > > rbtree: transaction commit element commit element commit > ^ problematic to the > point we're > considering a > transaction approach > > pipapo: transaction commit element commit transaction commit > > The single advantage I see of the current approach is that with the > hash back-ends we don't need two copies of the hash table, but that > also has the downside of the nft_set_elem_active(&he->ext, genmask) > check in the lookup function, which should be, in relative terms, even > more expensive than it is in the pipapo implementation, given that hash > back-ends are (in most cases) faster. There is also runtime set updates from packet path. In that case, we cannot keep a copy of the data structure that is being updated from the control plane while the packet path is also adding/deleting entries from it.
