Re: [PATCH v3] netfilter: nft_dynset: support for element deletion

[Florian Westphal <fw@xxxxxxxxx>]

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.
> 
> Signed-off-by: Ander Juaristi <a@xxxxxxxxxxxx>

Patch looks ok, but why did you add so many changes?

> -void nft_dynset_eval(const struct nft_expr *expr,
> -		     struct nft_regs *regs, const struct nft_pktinfo *pkt)
> +static bool nft_dynset_update(u8 sreg_key, int op, u64 timeout,
> +			      const struct nft_expr *expr,
> +			      const struct nft_pktinfo *pkt,
> +			      struct nft_regs *regs, struct nft_set *set)

Why is this new helper needed?

> +void nft_dynset_eval(const struct nft_expr *expr,
> +		     struct nft_regs *regs, const struct nft_pktinfo *pkt)
> +{
> +	const struct nft_dynset *priv = nft_expr_priv(expr);
> +	struct nft_set *set = priv->set;
> +
> +	if (priv->op == NFT_DYNSET_OP_DELETE) {
> +		set->ops->delete(set, &regs->data[priv->sreg_key]);
		return;
	}

... so no need for 'else' clause and no need to reformat all of this.