[PATCH nf] netfilter: nf_tables: return immediately on empty commit

[PATCH nf] netfilter: nf_tables: return immediately on empty commit

[Florian Westphal]

When running 'nft flush ruleset' while no rules exist, we will increment
the generation counter and announce a new genid to userspace, yet
nothing had changed in the first place.

Signed-off-by: Florian Westphal <fw@strlen.de>
---
 net/netfilter/nf_tables_api.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index dcd9cb68d826..1904e081853c 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -6536,6 +6536,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
 	struct nft_chain *chain;
 	struct nft_table *table;
 
+	if (list_empty(&net->nft.commit_list)) {
+		mutex_unlock(&net->nft.commit_mutex);
+		return 0;
+	}
+
 	/* 0. Validate ruleset, otherwise roll back for error reporting. */
 	if (nf_tables_validate(net) < 0)
 		return -EAGAIN;
-- 
2.19.2

Re: [PATCH nf] netfilter: nf_tables: return immediately on empty commit

[Pablo Neira Ayuso]

Hi Florian,

On Thu, Mar 07, 2019 at 11:20:11PM +0100, Florian Westphal wrote:
> When running 'nft flush ruleset' while no rules exist, we will increment
> the generation counter and announce a new genid to userspace, yet
> nothing had changed in the first place.
> 
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>  net/netfilter/nf_tables_api.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index dcd9cb68d826..1904e081853c 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -6536,6 +6536,11 @@ static int nf_tables_commit(struct net *net, struct sk_buff *skb)
>  	struct nft_chain *chain;
>  	struct nft_table *table;
>  
> +	if (list_empty(&net->nft.commit_list)) {
> +		mutex_unlock(&net->nft.commit_mutex);
> +		return 0;
> +	}

This looks good to catch this case.

I think we should probably address these too:

term1# nft add table x
term1# nft add table x

term2# nft monitor
add table ip x
# new generation 1587 by process 14422 (nft)
# new generation 1588 by process 14423 (nft)

So re-insertion of existing objects should not trigger unnecessary
generation counter bump. We can probably count the number of events
that have been sent from the commit path before we bump it.

Re: [PATCH nf] netfilter: nf_tables: return immediately on empty commit

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> term1# nft add table x
> term1# nft add table x
> 
> term2# nft monitor
> add table ip x
> # new generation 1587 by process 14422 (nft)
> # new generation 1588 by process 14423 (nft)
> 
> So re-insertion of existing objects should not trigger unnecessary
> generation counter bump. We can probably count the number of events
> that have been sent from the commit path before we bump it.

Thats strange.  Why does the second add result in the creation
of a transaction object in the first place?

2nd add should result in a call to "upd_table", and it should
find that no attributes are being changed, and no transaction
object should be created, i.e. the commit should end up with
empty transaction list.

Re: [PATCH nf] netfilter: nf_tables: return immediately on empty commit

[Pablo Neira Ayuso]

On Thu, Mar 07, 2019 at 11:20:11PM +0100, Florian Westphal wrote:
> When running 'nft flush ruleset' while no rules exist, we will increment
> the generation counter and announce a new genid to userspace, yet
> nothing had changed in the first place.

Applied, thanks Florian.