On Thu, Jun 08, 2023 at 09:57:04PM +0200, Pablo Neira Ayuso wrote:
> The pipapo set backend follows copy-on-update approach, maintaining one
> clone of the existing datastructure that is being updated. The clone
> and current datastructures are swapped via rcu from the commit step.
>
> The existing integration with the commit protocol is flawed because
> there is no operation to clean up the clone if the transaction is
> aborted. Moreover, the datastructure swap happens on set element
> activation.
>
> This patch adds two new operations for sets: commit and abort, these new
> operations are invoked from the commit and abort steps, after the
> transactions have been digested, and it updates the pipapo set backend
> to use it.
>
> This patch adds a new ->pending_update field to sets to maintain a list
> of sets that require this new commit and abort operations.
>
> Fixes: 3c4287f62044 ("nf_tables: Add set type for arbitrary concatenation of ranges")
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
Hi Pablo,
some suggestions of some trivial follow-up items from my side.
No need for these to hold up progress of the patchset.
> ---
> include/net/netfilter/nf_tables.h | 4 ++-
> net/netfilter/nf_tables_api.c | 56 +++++++++++++++++++++++++++++++
> net/netfilter/nft_set_pipapo.c | 55 +++++++++++++++++++++---------
> 3 files changed, 99 insertions(+), 16 deletions(-)
>
> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> index 2e24ea1d744c..83db182decc8 100644
> --- a/include/net/netfilter/nf_tables.h
> +++ b/include/net/netfilter/nf_tables.h
> @@ -462,7 +462,8 @@ struct nft_set_ops {
> const struct nft_set *set,
> const struct nft_set_elem *elem,
> unsigned int flags);
> -
> + void (*commit)(const struct nft_set *set);
> + void (*abort)(const struct nft_set *set);
As a follow-up, these could be added to the kdoc for nft_set_ops.
> u64 (*privsize)(const struct nlattr * const nla[],
> const struct nft_set_desc *desc);
> bool (*estimate)(const struct nft_set_desc *desc,
> @@ -557,6 +558,7 @@ struct nft_set {
> u16 policy;
> u16 udlen;
> unsigned char *udata;
> + struct list_head pending_update;
Likewise, as a follow-up, pending_update could be added to the kdoc for
nft_set.
> /* runtime data below here */
> const struct nft_set_ops *ops ____cacheline_aligned;
> u16 flags:14,
...
