On Wed, 3 Apr 2024 10:41:08 +0200 Florian Westphal <fw@xxxxxxxxx> wrote: > This set type keeps two copies of the sets' content, > priv->match (live version, used to match from packet path) > priv->clone (work-in-progress version of the 'future' priv->match). > > All additions and removals are done on priv->clone. When transaction > completes, priv->clone becomes priv->match and a new clone is allocated > for use by next transaction. > > Problem is that the cloning requires GFP_KERNEL allocations but we > cannot fail at either commit or abort time. > > This patch defers the clone until we get an insertion or removal > request. This allows us to handle OOM situations correctly. > > This also allows to remove ->dirty in a followup change: > > If ->clone exists, ->dirty is always true > If ->clone is NULL, ->dirty is always false, no elements were added > or removed (except catchall elements which are external to the specific > set backend). > > Signed-off-by: Florian Westphal <fw@xxxxxxxxx> > --- > net/netfilter/nft_set_pipapo.c | 62 ++++++++++++++++++++++------------ > 1 file changed, 41 insertions(+), 21 deletions(-) > > diff --git a/net/netfilter/nft_set_pipapo.c b/net/netfilter/nft_set_pipapo.c > index 2cc905e92889..eef6a978561f 100644 > --- a/net/netfilter/nft_set_pipapo.c > +++ b/net/netfilter/nft_set_pipapo.c > @@ -615,6 +615,9 @@ nft_pipapo_get(const struct net *net, const struct nft_set *set, > struct nft_pipapo_match *m = priv->clone; > struct nft_pipapo_elem *e; > > + if (!m) > + m = rcu_dereference(priv->match); > + > e = pipapo_get(net, set, m, (const u8 *)elem->key.val.data, > nft_genmask_cur(net), get_jiffies_64(), > GFP_ATOMIC); > @@ -1259,6 +1262,23 @@ static bool nft_pipapo_transaction_mutex_held(const struct nft_set *set) > #endif > } > > +static struct nft_pipapo_match *pipapo_clone(struct nft_pipapo_match *old); > + > +static struct nft_pipapo_match *pipapo_maybe_clone(const struct nft_set *set) Nit: /** * pipapo_maybe_clone() - Build clone for pending data changes, if not existing * @set: nftables API set representation * * Return: newly created or existing clone, if any. NULL on allocation failure */ The rest of the series looks good (and like a big improvement) to me, thanks! For all the patches minus 3/9, Reviewed-by: Stefano Brivio <sbrivio@xxxxxxxxxx> -- Stefano
