[Devel,1/2] net: Move net->ct.can_alloc check up to resolve_normal_ct()

Submitted by Kirill Tkhai on Oct. 3, 2016, 2:16 p.m.

Details

Message ID 147550416809.32608.12736754253925118080.stgit@localhost.localdomain
State New
Series "Series without cover letter"
Headers show

Commit Message

Kirill Tkhai Oct. 3, 2016, 2:16 p.m.
Move it up on stack to break creation of a CT earlier.
This avoids us to search in CT hashes and speeds work up.

So, now nf_conntrack_alloc() creates a CT certanly,
__nf_conntrack_alloc() doesn't return NULL and it does not
need to be external.

Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
---
 include/net/net_namespace.h          |    2 +-
 include/net/netfilter/nf_conntrack.h |    6 ------
 net/netfilter/nf_conntrack_core.c    |   24 ++++++++++++------------
 net/netfilter/nf_synproxy_core.c     |    2 +-
 4 files changed, 14 insertions(+), 20 deletions(-)

Patch hide | download patch | download mbox

diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
index 2baa2df..550b668 100644
--- a/include/net/net_namespace.h
+++ b/include/net/net_namespace.h
@@ -303,7 +303,7 @@  static inline struct net *read_pnet(possible_net_t const *pnet)
 static inline void allow_conntrack_allocation(struct net *net)
 {
 	net->ct.can_alloc = true;
-	smp_wmb(); /* Pairs with rmb in __nf_conntrack_alloc() */
+	smp_wmb(); /* Pairs with rmb in resolve_normal_ct() */
 }
 #else
 static inline void allow_conntrack_allocation(struct net *net) { }
diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
index fefe866..9993bd3 100644
--- a/include/net/netfilter/nf_conntrack.h
+++ b/include/net/netfilter/nf_conntrack.h
@@ -253,12 +253,6 @@  struct nf_conn *nf_conntrack_alloc(struct net *net, u16 zone,
 				   const struct nf_conntrack_tuple *orig,
 				   const struct nf_conntrack_tuple *repl,
 				   gfp_t gfp);
-struct nf_conn *
-__nf_conntrack_alloc(struct net *net, u16 zone,
-		     const struct nf_conntrack_tuple *orig,
-		     const struct nf_conntrack_tuple *repl,
-		     gfp_t gfp, u32 hash, bool can_alloc);
-
 static inline int nf_ct_is_template(const struct nf_conn *ct)
 {
 	return test_bit(IPS_TEMPLATE_BIT, &ct->status);
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index d46c905..d5a283a 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -832,21 +832,15 @@  void init_nf_conntrack_hash_rnd(void)
 	cmpxchg(&nf_conntrack_hash_rnd, 0, rand);
 }
 
-struct nf_conn *
+static struct nf_conn *
 __nf_conntrack_alloc(struct net *net, u16 zone,
 		     const struct nf_conntrack_tuple *orig,
 		     const struct nf_conntrack_tuple *repl,
-		     gfp_t gfp, u32 hash, bool can_alloc)
+		     gfp_t gfp, u32 hash)
 {
 	unsigned int ct_max = net->ct.max ? net->ct.max : init_net.ct.max;
 	struct nf_conn *ct;
 
-	if (!net->ct.can_alloc && !can_alloc) {
-		/* No rules loaded */
-		return NULL;
-	}
-	smp_rmb(); /* Pairs with wmb in allow_conntrack_allocation() */
-
 	if (unlikely(!nf_conntrack_hash_rnd)) {
 		init_nf_conntrack_hash_rnd();
 		/* recompute the hash as nf_conntrack_hash_rnd is initialized */
@@ -915,14 +909,13 @@  __nf_conntrack_alloc(struct net *net, u16 zone,
 	return ERR_PTR(-ENOMEM);
 #endif
 }
-EXPORT_SYMBOL_GPL(__nf_conntrack_alloc);
 
 struct nf_conn *nf_conntrack_alloc(struct net *net, u16 zone,
 				   const struct nf_conntrack_tuple *orig,
 				   const struct nf_conntrack_tuple *repl,
 				   gfp_t gfp)
 {
-	return __nf_conntrack_alloc(net, zone, orig, repl, gfp, 0, false);
+	return __nf_conntrack_alloc(net, zone, orig, repl, gfp, 0);
 }
 EXPORT_SYMBOL_GPL(nf_conntrack_alloc);
 
@@ -969,8 +962,8 @@  init_conntrack(struct net *net, struct nf_conn *tmpl,
 	}
 
 	ct = __nf_conntrack_alloc(net, zone, tuple, &repl_tuple, GFP_ATOMIC,
-				  hash, false);
-	if (IS_ERR_OR_NULL(ct))
+				  hash);
+	if (IS_ERR(ct))
 		return (struct nf_conntrack_tuple_hash *)ct;
 
 	if (tmpl && nfct_synproxy(tmpl)) {
@@ -1071,6 +1064,13 @@  resolve_normal_ct(struct net *net, struct nf_conn *tmpl,
 	if (!net_ipt_permitted(net, VE_NF_CONNTRACK))
 		return NULL;
 
+	if (!net->ct.can_alloc) {
+		/* No rules loaded */
+		return NULL;
+	}
+	smp_rmb(); /* Pairs with wmb in allow_conntrack_allocation() */
+
+
 	if (!nf_ct_get_tuple(skb, skb_network_offset(skb),
 			     dataoff, l3num, protonum, &tuple, l3proto,
 			     l4proto)) {
diff --git a/net/netfilter/nf_synproxy_core.c b/net/netfilter/nf_synproxy_core.c
index 3184505..52e20c9 100644
--- a/net/netfilter/nf_synproxy_core.c
+++ b/net/netfilter/nf_synproxy_core.c
@@ -353,7 +353,7 @@  static int __net_init synproxy_net_init(struct net *net)
 	int err = -ENOMEM;
 
 	memset(&t, 0, sizeof(t));
-	ct = __nf_conntrack_alloc(net, 0, &t, &t, GFP_KERNEL, 0, true);
+	ct = nf_conntrack_alloc(net, 0, &t, &t, GFP_KERNEL);
 	if (IS_ERR(ct)) {
 		err = PTR_ERR(ct);
 		goto err1;

Comments

Pavel Tikhomirov Oct. 3, 2016, 4:13 p.m.
On 10/03/2016 05:16 PM, Kirill Tkhai wrote:
> Move it up on stack to break creation of a CT earlier.
> This avoids us to search in CT hashes and speeds work up.
>
> So, now nf_conntrack_alloc() creates a CT certanly,
> __nf_conntrack_alloc() doesn't return NULL and it does not
> need to be external.
>

Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>

> Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com>
> ---
>  include/net/net_namespace.h          |    2 +-
>  include/net/netfilter/nf_conntrack.h |    6 ------
>  net/netfilter/nf_conntrack_core.c    |   24 ++++++++++++------------
>  net/netfilter/nf_synproxy_core.c     |    2 +-
>  4 files changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
> index 2baa2df..550b668 100644
> --- a/include/net/net_namespace.h
> +++ b/include/net/net_namespace.h
> @@ -303,7 +303,7 @@ static inline struct net *read_pnet(possible_net_t const *pnet)
>  static inline void allow_conntrack_allocation(struct net *net)
>  {
>  	net->ct.can_alloc = true;
> -	smp_wmb(); /* Pairs with rmb in __nf_conntrack_alloc() */
> +	smp_wmb(); /* Pairs with rmb in resolve_normal_ct() */
>  }
>  #else
>  static inline void allow_conntrack_allocation(struct net *net) { }
> diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h
> index fefe866..9993bd3 100644
> --- a/include/net/netfilter/nf_conntrack.h
> +++ b/include/net/netfilter/nf_conntrack.h
> @@ -253,12 +253,6 @@ struct nf_conn *nf_conntrack_alloc(struct net *net, u16 zone,
>  				   const struct nf_conntrack_tuple *orig,
>  				   const struct nf_conntrack_tuple *repl,
>  				   gfp_t gfp);
> -struct nf_conn *
> -__nf_conntrack_alloc(struct net *net, u16 zone,
> -		     const struct nf_conntrack_tuple *orig,
> -		     const struct nf_conntrack_tuple *repl,
> -		     gfp_t gfp, u32 hash, bool can_alloc);
> -
>  static inline int nf_ct_is_template(const struct nf_conn *ct)
>  {
>  	return test_bit(IPS_TEMPLATE_BIT, &ct->status);
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index d46c905..d5a283a 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -832,21 +832,15 @@ void init_nf_conntrack_hash_rnd(void)
>  	cmpxchg(&nf_conntrack_hash_rnd, 0, rand);
>  }
>
> -struct nf_conn *
> +static struct nf_conn *
>  __nf_conntrack_alloc(struct net *net, u16 zone,
>  		     const struct nf_conntrack_tuple *orig,
>  		     const struct nf_conntrack_tuple *repl,
> -		     gfp_t gfp, u32 hash, bool can_alloc)
> +		     gfp_t gfp, u32 hash)
>  {
>  	unsigned int ct_max = net->ct.max ? net->ct.max : init_net.ct.max;
>  	struct nf_conn *ct;
>
> -	if (!net->ct.can_alloc && !can_alloc) {
> -		/* No rules loaded */
> -		return NULL;
> -	}
> -	smp_rmb(); /* Pairs with wmb in allow_conntrack_allocation() */
> -
>  	if (unlikely(!nf_conntrack_hash_rnd)) {
>  		init_nf_conntrack_hash_rnd();
>  		/* recompute the hash as nf_conntrack_hash_rnd is initialized */
> @@ -915,14 +909,13 @@ __nf_conntrack_alloc(struct net *net, u16 zone,
>  	return ERR_PTR(-ENOMEM);
>  #endif
>  }
> -EXPORT_SYMBOL_GPL(__nf_conntrack_alloc);
>
>  struct nf_conn *nf_conntrack_alloc(struct net *net, u16 zone,
>  				   const struct nf_conntrack_tuple *orig,
>  				   const struct nf_conntrack_tuple *repl,
>  				   gfp_t gfp)
>  {
> -	return __nf_conntrack_alloc(net, zone, orig, repl, gfp, 0, false);
> +	return __nf_conntrack_alloc(net, zone, orig, repl, gfp, 0);
>  }
>  EXPORT_SYMBOL_GPL(nf_conntrack_alloc);
>
> @@ -969,8 +962,8 @@ init_conntrack(struct net *net, struct nf_conn *tmpl,
>  	}
>
>  	ct = __nf_conntrack_alloc(net, zone, tuple, &repl_tuple, GFP_ATOMIC,
> -				  hash, false);
> -	if (IS_ERR_OR_NULL(ct))
> +				  hash);
> +	if (IS_ERR(ct))
>  		return (struct nf_conntrack_tuple_hash *)ct;
>
>  	if (tmpl && nfct_synproxy(tmpl)) {
> @@ -1071,6 +1064,13 @@ resolve_normal_ct(struct net *net, struct nf_conn *tmpl,
>  	if (!net_ipt_permitted(net, VE_NF_CONNTRACK))
>  		return NULL;
>
> +	if (!net->ct.can_alloc) {
> +		/* No rules loaded */
> +		return NULL;
> +	}
> +	smp_rmb(); /* Pairs with wmb in allow_conntrack_allocation() */
> +
> +
>  	if (!nf_ct_get_tuple(skb, skb_network_offset(skb),
>  			     dataoff, l3num, protonum, &tuple, l3proto,
>  			     l4proto)) {
> diff --git a/net/netfilter/nf_synproxy_core.c b/net/netfilter/nf_synproxy_core.c
> index 3184505..52e20c9 100644
> --- a/net/netfilter/nf_synproxy_core.c
> +++ b/net/netfilter/nf_synproxy_core.c
> @@ -353,7 +353,7 @@ static int __net_init synproxy_net_init(struct net *net)
>  	int err = -ENOMEM;
>
>  	memset(&t, 0, sizeof(t));
> -	ct = __nf_conntrack_alloc(net, 0, &t, &t, GFP_KERNEL, 0, true);
> +	ct = nf_conntrack_alloc(net, 0, &t, &t, GFP_KERNEL);
>  	if (IS_ERR(ct)) {
>  		err = PTR_ERR(ct);
>  		goto err1;
>