[RFC] netfilter: nf_tables: defer all object release via rcu

Now that all objects are released in the reverse order via the
transaction infrastructure, we can enqueue the release via
call_rcu to save one synchronize_rcu. For small rule-sets loaded
via nft -f, it now takes between 50ms and 100ms less here.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---

This is a follow up patch that applies on top of the new transaction
infrastructure.

It has downside effect which is that I had to remove anonymous set event
notification since we lack of the skb/nlh/nla context from there.

I think we can adapt the nf_tables_set_notify to skip report and
portid when we don't have any context, so we still send events for the
anonymous sets. I believe the remaining pointers in the context area
should be valid for several reasons:

1) The objects are released in reverse order according to the commands.

2) We hold a reference to the afi family module, so the module has to be
   there until we delete the table, which should be the last object
   in the queue to be released.

3) The table and chain pointers should correspond to objects that still exists
   (they cannot be deleted as use counter is > 1) or that are in the rcu queue
   to be released after us.

That patch would be slightly larger though and set_notify needs to be
adjusted.

 include/net/netfilter/nf_tables.h |    2 +
 net/netfilter/nf_tables_api.c     |  100 ++++++++++++++++++++++---------------
 2 files changed, 62 insertions(+), 40 deletions(-)

Hi  Pablo,

2 minor issues.

>   include/net/netfilter/nf_tables.h |    2 +
>   net/netfilter/nf_tables_api.c     |  100 ++++++++++++++++++++++---------------
>   2 files changed, 62 insertions(+), 40 deletions(-)
>
> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> index b08f2a9..65656f7 100644
> --- a/include/net/netfilter/nf_tables.h
> +++ b/include/net/netfilter/nf_tables.h
> @@ -391,12 +391,14 @@ struct nft_rule {
>   /**
>    *	struct nft_trans - nf_tables object update in transaction
>    *
> + *	rcu_head: rcu head to defer release of transaction data

an '@' is missing

>    *	@list: used internally
>    *	@msg_type: message type
>    *	@ctx: transaction context
>    *	@data: internal information related to the transaction
>    */
>   struct nft_trans {
> +	struct rcu_head			rcu_head;
>   	struct list_head		list;
>   	int				msg_type;
>   	struct nft_ctx			ctx;
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 5606ae30..fd03212 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -2612,7 +2612,8 @@ static void nft_set_destroy(struct nft_set *set)
>   static void nf_tables_set_destroy(const struct nft_ctx *ctx, struct nft_set *set)
>   {
>   	list_del(&set->list);
> -	nf_tables_set_notify(ctx, set, NFT_MSG_DELSET);
> +	if (!set->flags & NFT_SET_ANONYMOUS)
> +		nf_tables_set_notify(ctx, set, NFT_MSG_DELSET);

It's a fix for anonymous set, so it's should be on another patch.



Tomasz
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Thu, Apr 10, 2014 at 03:01:47PM +0300, Tomasz Bursztyka wrote:
> Hi  Pablo,
> 
> 2 minor issues.
> 
> >  include/net/netfilter/nf_tables.h |    2 +
> >  net/netfilter/nf_tables_api.c     |  100 ++++++++++++++++++++++---------------
> >  2 files changed, 62 insertions(+), 40 deletions(-)
> >
> >diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> >index b08f2a9..65656f7 100644
> >--- a/include/net/netfilter/nf_tables.h
> >+++ b/include/net/netfilter/nf_tables.h
> >@@ -391,12 +391,14 @@ struct nft_rule {
> >  /**
> >   *	struct nft_trans - nf_tables object update in transaction
> >   *
> >+ *	rcu_head: rcu head to defer release of transaction data
> 
> an '@' is missing

Right.

> >   *	@list: used internally
> >   *	@msg_type: message type
> >   *	@ctx: transaction context
> >   *	@data: internal information related to the transaction
> >   */
> >  struct nft_trans {
> >+	struct rcu_head			rcu_head;
> >  	struct list_head		list;
> >  	int				msg_type;
> >  	struct nft_ctx			ctx;
> >diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> >index 5606ae30..fd03212 100644
> >--- a/net/netfilter/nf_tables_api.c
> >+++ b/net/netfilter/nf_tables_api.c
> >@@ -2612,7 +2612,8 @@ static void nft_set_destroy(struct nft_set *set)
> >  static void nf_tables_set_destroy(const struct nft_ctx *ctx, struct nft_set *set)
> >  {
> >  	list_del(&set->list);
> >-	nf_tables_set_notify(ctx, set, NFT_MSG_DELSET);
> >+	if (!set->flags & NFT_SET_ANONYMOUS)
> >+		nf_tables_set_notify(ctx, set, NFT_MSG_DELSET);
> 
> It's a fix for anonymous set, so it's should be on another patch.

Not a fix, we currently have anonymous set notifications which are
possible.  Please, have a look at the changelog in this patch, it
discusses the reason for this and alternative solutions.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html