Ander Juaristi <a@xxxxxxxxxxxx> wrote:
> This patch implements the delete operation from the ruleset.
>
> It implements a new delete() function in nft_set_rhash. It is simpler
> to use than the already existing remove(), because it only takes the set
> and the key as arguments, whereas remove() expects a full
> nft_set_elem structure.
Looks good, I plan to review all your posted patches on monday.
some nits below,
> --- a/include/net/netfilter/nf_tables.h
> +++ b/include/net/netfilter/nf_tables.h
> @@ -312,6 +312,8 @@ struct nft_set_ops {
> const struct nft_expr *expr,
> struct nft_regs *regs,
> const struct nft_set_ext **ext);
> + bool (*delete)(const struct nft_set *set,
> + const u32 *key);
Can you add a small comment here that explains the functions at the
start are those used on packet path, and rest are control plane/slow
path ones?
> +static bool nft_dynset_update(uint8_t sreg_key, int op, u64 timeout,
Can you use plain u8 here? In case you haven't seen it, there is
scripts/checkpatch.pl in the kernel tree.
> + if (he == NULL)
> + return false;
> +
> + rhashtable_remove_fast(&priv->ht, &he->node, nft_rhash_params);
> + return true;
Perhaps add a small comment here that rhashtable_remove_fast retval
is ignored intentionally?
I.e., don't make this return false in case two cpus race to remove same
entry.
Otherwise, this looks really good, thanks for working on this!
