[PATCH net-next v2 0/2] ipv6: Avoid rt6_probe() taking writer lock in the fast path

[PATCH net-next v2 0/2] ipv6: Avoid rt6_probe() taking writer lock in the fast path

[Martin KaFai Lau]

v1 -> v2:
1. Separate the code re-arrangement into another patch
2. Fix style

[PATCH net-next v2 1/2] ipv6: Re-arrange code in rt6_probe()

[Martin KaFai Lau]

It is a prep work for the next patch to remove write_lock
from rt6_probe().

1. Reduce the number of if(neigh) check.  From 4 to 1.
2. Bring the write_(un)lock() closer to the operations that the
   lock is protecting.

Hopefully, the above make rt6_probe() more readable.

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
Cc: Julian Anastasov <ja@ssi.bg>
Cc: YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>
---
 net/ipv6/route.c | 44 ++++++++++++++++++++------------------------
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 7f2214f..6d503db 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -545,6 +545,7 @@ static void rt6_probe_deferred(struct work_struct *w)
 
 static void rt6_probe(struct rt6_info *rt)
 {
+	struct __rt6_probe_work *work;
 	struct neighbour *neigh;
 	/*
 	 * Okay, this does not seem to be appropriate
@@ -559,34 +560,29 @@ static void rt6_probe(struct rt6_info *rt)
 	rcu_read_lock_bh();
 	neigh = __ipv6_neigh_lookup_noref(rt->dst.dev, &rt->rt6i_gateway);
 	if (neigh) {
+		work = NULL;
 		write_lock(&neigh->lock);
-		if (neigh->nud_state & NUD_VALID)
-			goto out;
-	}
-
-	if (!neigh ||
-	    time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
-		struct __rt6_probe_work *work;
-
-		work = kmalloc(sizeof(*work), GFP_ATOMIC);
-
-		if (neigh && work)
-			__neigh_set_probe_once(neigh);
-
-		if (neigh)
-			write_unlock(&neigh->lock);
-
-		if (work) {
-			INIT_WORK(&work->work, rt6_probe_deferred);
-			work->target = rt->rt6i_gateway;
-			dev_hold(rt->dst.dev);
-			work->dev = rt->dst.dev;
-			schedule_work(&work->work);
+		if (!(neigh->nud_state & NUD_VALID) &&
+		    time_after(jiffies,
+			       neigh->updated +
+			       rt->rt6i_idev->cnf.rtr_probe_interval)) {
+			work = kmalloc(sizeof(*work), GFP_ATOMIC);
+			if (work)
+				__neigh_set_probe_once(neigh);
 		}
-	} else {
-out:
 		write_unlock(&neigh->lock);
+	} else {
+		work = kmalloc(sizeof(*work), GFP_ATOMIC);
+	}
+
+	if (work) {
+		INIT_WORK(&work->work, rt6_probe_deferred);
+		work->target = rt->rt6i_gateway;
+		dev_hold(rt->dst.dev);
+		work->dev = rt->dst.dev;
+		schedule_work(&work->work);
 	}
+
 	rcu_read_unlock_bh();
 }
 #else
-- 
1.8.1

[PATCH net-next v2 2/2] ipv6: Avoid rt6_probe() taking writer lock in the fast path

[Martin KaFai Lau]

The patch checks neigh->nud_state before acquiring the writer lock.
Note that rt6_probe() is only used in CONFIG_IPV6_ROUTER_PREF.

40 udpflood processes and a /64 gateway route are used.
The gateway has NUD_PERMANENT.  Each of them is run for 30s.
At the end, the total number of finished sendto():

Before: 55M
After: 95M

Signed-off-by: Martin KaFai Lau <kafai@fb.com>
Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
CC: Julian Anastasov <ja@ssi.bg>
CC: YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>
---
 net/ipv6/route.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 6d503db..76dcff8 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -560,6 +560,9 @@ static void rt6_probe(struct rt6_info *rt)
 	rcu_read_lock_bh();
 	neigh = __ipv6_neigh_lookup_noref(rt->dst.dev, &rt->rt6i_gateway);
 	if (neigh) {
+		if (neigh->nud_state & NUD_VALID)
+			goto out;
+
 		work = NULL;
 		write_lock(&neigh->lock);
 		if (!(neigh->nud_state & NUD_VALID) &&
@@ -583,6 +586,7 @@ static void rt6_probe(struct rt6_info *rt)
 		schedule_work(&work->work);
 	}
 
+out:
 	rcu_read_unlock_bh();
 }
 #else
-- 
1.8.1

Re: [PATCH net-next v2 0/2] ipv6: Avoid rt6_probe() taking writer lock in the fast path

[David Miller]

From: Martin KaFai Lau <kafai@fb.com>
Date: Fri, 24 Jul 2015 09:57:41 -0700

> v1 -> v2:
> 1. Separate the code re-arrangement into another patch
> 2. Fix style

Looks good, series applied.

Re: [PATCH net-next v2 1/2] ipv6: Re-arrange code in rt6_probe()

[YOSHIFUJI Hideaki/吉藤英明]

Martin KaFai Lau wrote:
> It is a prep work for the next patch to remove write_lock
> from rt6_probe().
> 
> 1. Reduce the number of if(neigh) check.  From 4 to 1.
> 2. Bring the write_(un)lock() closer to the operations that the
>    lock is protecting.
> 
> Hopefully, the above make rt6_probe() more readable.
> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
> Cc: Julian Anastasov <ja@ssi.bg>
> Cc: YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>

Acked-by: YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>

--yoshfuji

> ---
>  net/ipv6/route.c | 44 ++++++++++++++++++++------------------------
>  1 file changed, 20 insertions(+), 24 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 7f2214f..6d503db 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -545,6 +545,7 @@ static void rt6_probe_deferred(struct work_struct *w)
>  
>  static void rt6_probe(struct rt6_info *rt)
>  {
> +	struct __rt6_probe_work *work;
>  	struct neighbour *neigh;
>  	/*
>  	 * Okay, this does not seem to be appropriate
> @@ -559,34 +560,29 @@ static void rt6_probe(struct rt6_info *rt)
>  	rcu_read_lock_bh();
>  	neigh = __ipv6_neigh_lookup_noref(rt->dst.dev, &rt->rt6i_gateway);
>  	if (neigh) {
> +		work = NULL;
>  		write_lock(&neigh->lock);
> -		if (neigh->nud_state & NUD_VALID)
> -			goto out;
> -	}
> -
> -	if (!neigh ||
> -	    time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
> -		struct __rt6_probe_work *work;
> -
> -		work = kmalloc(sizeof(*work), GFP_ATOMIC);
> -
> -		if (neigh && work)
> -			__neigh_set_probe_once(neigh);
> -
> -		if (neigh)
> -			write_unlock(&neigh->lock);
> -
> -		if (work) {
> -			INIT_WORK(&work->work, rt6_probe_deferred);
> -			work->target = rt->rt6i_gateway;
> -			dev_hold(rt->dst.dev);
> -			work->dev = rt->dst.dev;
> -			schedule_work(&work->work);
> +		if (!(neigh->nud_state & NUD_VALID) &&
> +		    time_after(jiffies,
> +			       neigh->updated +
> +			       rt->rt6i_idev->cnf.rtr_probe_interval)) {
> +			work = kmalloc(sizeof(*work), GFP_ATOMIC);
> +			if (work)
> +				__neigh_set_probe_once(neigh);
>  		}
> -	} else {
> -out:
>  		write_unlock(&neigh->lock);
> +	} else {
> +		work = kmalloc(sizeof(*work), GFP_ATOMIC);
> +	}
> +
> +	if (work) {
> +		INIT_WORK(&work->work, rt6_probe_deferred);
> +		work->target = rt->rt6i_gateway;
> +		dev_hold(rt->dst.dev);
> +		work->dev = rt->dst.dev;
> +		schedule_work(&work->work);
>  	}
> +
>  	rcu_read_unlock_bh();
>  }
>  #else
> 

-- 
吉藤英明 <hideaki.yoshifuji@miraclelinux.com>
ミラクル・リナックス株式会社 技術本部 サポート部

Re: [PATCH net-next v2 2/2] ipv6: Avoid rt6_probe() taking writer lock in the fast path

[YOSHIFUJI Hideaki]

Hi,

Martin KaFai Lau wrote:
> The patch checks neigh->nud_state before acquiring the writer lock.
> Note that rt6_probe() is only used in CONFIG_IPV6_ROUTER_PREF.
> 
> 40 udpflood processes and a /64 gateway route are used.
> The gateway has NUD_PERMANENT.  Each of them is run for 30s.
> At the end, the total number of finished sendto():
> 
> Before: 55M
> After: 95M

I think it is better to describe why it is okay without any locks.

--yoshfuji

> 
> Signed-off-by: Martin KaFai Lau <kafai@fb.com>
> Cc: Hannes Frederic Sowa <hannes@stressinduktion.org>
> CC: Julian Anastasov <ja@ssi.bg>
> CC: YOSHIFUJI Hideaki <hideaki.yoshifuji@miraclelinux.com>
> ---
>  net/ipv6/route.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 6d503db..76dcff8 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -560,6 +560,9 @@ static void rt6_probe(struct rt6_info *rt)
>  	rcu_read_lock_bh();
>  	neigh = __ipv6_neigh_lookup_noref(rt->dst.dev, &rt->rt6i_gateway);
>  	if (neigh) {
> +		if (neigh->nud_state & NUD_VALID)
> +			goto out;
> +
>  		work = NULL;
>  		write_lock(&neigh->lock);
>  		if (!(neigh->nud_state & NUD_VALID) &&
> @@ -583,6 +586,7 @@ static void rt6_probe(struct rt6_info *rt)
>  		schedule_work(&work->work);
>  	}
>  
> +out:
>  	rcu_read_unlock_bh();
>  }
>  #else
> 

-- 
Hideaki Yoshifuji <hideaki.yoshifuji@miraclelinux.com>
Technical Division, MIRACLE LINUX CORPORATION