[PATCH 1/3] ipv6: remove some useless RCU read lock

[PATCH 1/3] ipv6: remove some useless RCU read lock

[Cong Wang]

After this commit:
	commit 97cac0821af4474ec4ba3a9e7a36b98ed9b6db88
	Author: David S. Miller <davem@davemloft.net>
	Date:   Mon Jul 2 22:43:47 2012 -0700

	    ipv6: Store route neighbour in rt6_info struct.

we no longer use RCU to protect route neighbour.

Cc: "David S. Miller" <davem@davemloft.net>
Signed-off-by: Cong Wang <amwang@redhat.com>
---
 net/ipv6/ip6_output.c |   13 ++-----------
 net/ipv6/route.c      |   15 ++-------------
 2 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index a4f6263..3dd4a37 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -123,16 +123,11 @@ static int ip6_finish_output2(struct sk_buff *skb)
 				skb->len);
 	}
 
-	rcu_read_lock();
 	rt = (struct rt6_info *) dst;
 	neigh = rt->n;
-	if (neigh) {
-		int res = dst_neigh_output(dst, neigh, skb);
+	if (neigh)
+		return dst_neigh_output(dst, neigh, skb);
 
-		rcu_read_unlock();
-		return res;
-	}
-	rcu_read_unlock();
 	IP6_INC_STATS_BH(dev_net(dst->dev),
 			 ip6_dst_idev(dst), IPSTATS_MIB_OUTNOROUTES);
 	kfree_skb(skb);
@@ -983,7 +978,6 @@ static int ip6_dst_lookup_tail(struct sock *sk,
 	 * dst entry and replace it instead with the
 	 * dst entry of the nexthop router
 	 */
-	rcu_read_lock();
 	rt = (struct rt6_info *) *dst;
 	n = rt->n;
 	if (n && !(n->nud_state & NUD_VALID)) {
@@ -991,7 +985,6 @@ static int ip6_dst_lookup_tail(struct sock *sk,
 		struct flowi6 fl_gw6;
 		int redirect;
 
-		rcu_read_unlock();
 		ifp = ipv6_get_ifaddr(net, &fl6->saddr,
 				      (*dst)->dev, 1);
 
@@ -1011,8 +1004,6 @@ static int ip6_dst_lookup_tail(struct sock *sk,
 			if ((err = (*dst)->error))
 				goto out_err_release;
 		}
-	} else {
-		rcu_read_unlock();
 	}
 #endif
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 339d921..b130bf2 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -451,10 +451,9 @@ static void rt6_probe(struct rt6_info *rt)
 	 * Router Reachability Probe MUST be rate-limited
 	 * to no more than one per minute.
 	 */
-	rcu_read_lock();
 	neigh = rt ? rt->n : NULL;
 	if (!neigh || (neigh->nud_state & NUD_VALID))
-		goto out;
+		return;
 	read_lock_bh(&neigh->lock);
 	if (!(neigh->nud_state & NUD_VALID) &&
 	    time_after(jiffies, neigh->updated + rt->rt6i_idev->cnf.rtr_probe_interval)) {
@@ -470,8 +469,6 @@ static void rt6_probe(struct rt6_info *rt)
 	} else {
 		read_unlock_bh(&neigh->lock);
 	}
-out:
-	rcu_read_unlock();
 }
 #else
 static inline void rt6_probe(struct rt6_info *rt)
@@ -498,7 +495,6 @@ static inline int rt6_check_neigh(struct rt6_info *rt)
 	struct neighbour *neigh;
 	int m;
 
-	rcu_read_lock();
 	neigh = rt->n;
 	if (rt->rt6i_flags & RTF_NONEXTHOP ||
 	    !(rt->rt6i_flags & RTF_GATEWAY))
@@ -516,7 +512,6 @@ static inline int rt6_check_neigh(struct rt6_info *rt)
 		read_unlock_bh(&neigh->lock);
 	} else
 		m = 0;
-	rcu_read_unlock();
 	return m;
 }
 
@@ -2496,15 +2491,11 @@ static int rt6_fill_node(struct net *net,
 	if (rtnetlink_put_metrics(skb, dst_metrics_ptr(&rt->dst)) < 0)
 		goto nla_put_failure;
 
-	rcu_read_lock();
 	n = rt->n;
 	if (n) {
-		if (nla_put(skb, RTA_GATEWAY, 16, &n->primary_key) < 0) {
-			rcu_read_unlock();
+		if (nla_put(skb, RTA_GATEWAY, 16, &n->primary_key) < 0)
 			goto nla_put_failure;
-		}
 	}
-	rcu_read_unlock();
 
 	if (rt->dst.dev &&
 	    nla_put_u32(skb, RTA_OIF, rt->dst.dev->ifindex))
@@ -2706,14 +2697,12 @@ static int rt6_info_route(struct rt6_info *rt, void *p_arg)
 #else
 	seq_puts(m, "00000000000000000000000000000000 00 ");
 #endif
-	rcu_read_lock();
 	n = rt->n;
 	if (n) {
 		seq_printf(m, "%pi6", n->primary_key);
 	} else {
 		seq_puts(m, "00000000000000000000000000000000");
 	}
-	rcu_read_unlock();
 	seq_printf(m, " %08x %08x %08x %08x %8s\n",
 		   rt->rt6i_metric, atomic_read(&rt->dst.__refcnt),
 		   rt->dst.__use, rt->rt6i_flags,
-- 
1.7.7.6

[PATCH 2/3] ipv6, route: remove BACKTRACK() macro

[Cong Wang]

It doesn't save any code, nor it helps readability.

Signed-off-by: Cong Wang <amwang@redhat.com>
---
 net/ipv6/route.c |   48 ++++++++++++++++++++++++++++--------------------
 1 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index b130bf2..71267e9 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -675,24 +675,6 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
 }
 #endif
 
-#define BACKTRACK(__net, saddr)			\
-do { \
-	if (rt == __net->ipv6.ip6_null_entry) {	\
-		struct fib6_node *pn; \
-		while (1) { \
-			if (fn->fn_flags & RTN_TL_ROOT) \
-				goto out; \
-			pn = fn->parent; \
-			if (FIB6_SUBTREE(pn) && FIB6_SUBTREE(pn) != fn) \
-				fn = fib6_lookup(FIB6_SUBTREE(pn), NULL, saddr); \
-			else \
-				fn = pn; \
-			if (fn->fn_flags & RTN_RTINFO) \
-				goto restart; \
-		} \
-	} \
-} while (0)
-
 static struct rt6_info *ip6_pol_route_lookup(struct net *net,
 					     struct fib6_table *table,
 					     struct flowi6 *fl6, int flags)
@@ -705,7 +687,20 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
 restart:
 	rt = fn->leaf;
 	rt = rt6_device_match(net, rt, &fl6->saddr, fl6->flowi6_oif, flags);
-	BACKTRACK(net, &fl6->saddr);
+	if (rt == net->ipv6.ip6_null_entry) {
+		struct fib6_node *pn;
+		while (1) {
+			if (fn->fn_flags & RTN_TL_ROOT)
+				goto out;
+			pn = fn->parent;
+			if (FIB6_SUBTREE(pn) && FIB6_SUBTREE(pn) != fn)
+				fn = fib6_lookup(FIB6_SUBTREE(pn), NULL, &fl6->saddr);
+			else
+				fn = pn;
+			if (fn->fn_flags & RTN_RTINFO)
+				goto restart;
+		}
+	}
 out:
 	dst_use(&rt->dst, jiffies);
 	read_unlock_bh(&table->tb6_lock);
@@ -867,7 +862,20 @@ restart_2:
 restart:
 	rt = rt6_select(fn, oif, strict | reachable);
 
-	BACKTRACK(net, &fl6->saddr);
+	if (rt == net->ipv6.ip6_null_entry) {
+		struct fib6_node *pn;
+		while (1) {
+			if (fn->fn_flags & RTN_TL_ROOT)
+				goto out;
+			pn = fn->parent;
+			if (FIB6_SUBTREE(pn) && FIB6_SUBTREE(pn) != fn)
+				fn = fib6_lookup(FIB6_SUBTREE(pn), NULL, &fl6->saddr);
+			else
+				fn = pn;
+			if (fn->fn_flags & RTN_RTINFO)
+				goto restart;
+		}
+	}
 	if (rt == net->ipv6.ip6_null_entry ||
 	    rt->rt6i_flags & RTF_CACHE)
 		goto out;
-- 
1.7.7.6

Re: [PATCH 2/3] ipv6, route: remove BACKTRACK() macro

[Shan Wei]

Cong Wang said, at 2012/9/10 20:48:
> It doesn't save any code, nor it helps readability.
> 
> Signed-off-by: Cong Wang <amwang@redhat.com>

this macro, don't reduce showing lines?
with it, more readability on terminal, right?

P.S. when doing backport work, some bored patches influence me.
this patch and [PATCH 3/3] are typical. :-)

> ---
>  net/ipv6/route.c |   48 ++++++++++++++++++++++++++++--------------------
>  1 files changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index b130bf2..71267e9 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -675,24 +675,6 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
>  }
>  #endif
>  
> -#define BACKTRACK(__net, saddr)			\
> -do { \
> -	if (rt == __net->ipv6.ip6_null_entry) {	\
> -		struct fib6_node *pn; \
> -		while (1) { \
> -			if (fn->fn_flags & RTN_TL_ROOT) \
> -				goto out; \
> -			pn = fn->parent; \
> -			if (FIB6_SUBTREE(pn) && FIB6_SUBTREE(pn) != fn) \
> -				fn = fib6_lookup(FIB6_SUBTREE(pn), NULL, saddr); \
> -			else \
> -				fn = pn; \
> -			if (fn->fn_flags & RTN_RTINFO) \
> -				goto restart; \
> -		} \
> -	} \
> -} while (0)
> -
>  static struct rt6_info *ip6_pol_route_lookup(struct net *net,
>  					     struct fib6_table *table,
>  					     struct flowi6 *fl6, int flags)
> @@ -705,7 +687,20 @@ static struct rt6_info *ip6_pol_route_lookup(struct net *net,
>  restart:
>  	rt = fn->leaf;
>  	rt = rt6_device_match(net, rt, &fl6->saddr, fl6->flowi6_oif, flags);
> -	BACKTRACK(net, &fl6->saddr);
> +	if (rt == net->ipv6.ip6_null_entry) {
> +		struct fib6_node *pn;
> +		while (1) {
> +			if (fn->fn_flags & RTN_TL_ROOT)
> +				goto out;
> +			pn = fn->parent;
> +			if (FIB6_SUBTREE(pn) && FIB6_SUBTREE(pn) != fn)
> +				fn = fib6_lookup(FIB6_SUBTREE(pn), NULL, &fl6->saddr);
> +			else
> +				fn = pn;
> +			if (fn->fn_flags & RTN_RTINFO)
> +				goto restart;
> +		}
> +	}
>  out:
>  	dst_use(&rt->dst, jiffies);
>  	read_unlock_bh(&table->tb6_lock);
> @@ -867,7 +862,20 @@ restart_2:
>  restart:
>  	rt = rt6_select(fn, oif, strict | reachable);
>  
> -	BACKTRACK(net, &fl6->saddr);
> +	if (rt == net->ipv6.ip6_null_entry) {
> +		struct fib6_node *pn;
> +		while (1) {
> +			if (fn->fn_flags & RTN_TL_ROOT)
> +				goto out;
> +			pn = fn->parent;
> +			if (FIB6_SUBTREE(pn) && FIB6_SUBTREE(pn) != fn)
> +				fn = fib6_lookup(FIB6_SUBTREE(pn), NULL, &fl6->saddr);
> +			else
> +				fn = pn;
> +			if (fn->fn_flags & RTN_RTINFO)
> +				goto restart;
> +		}
> +	}
>  	if (rt == net->ipv6.ip6_null_entry ||
>  	    rt->rt6i_flags & RTF_CACHE)
>  		goto out;
> 

Re: [PATCH 2/3] ipv6, route: remove BACKTRACK() macro

[Cong Wang]

On Mon, 2012-09-10 at 21:43 +0800, Shan Wei wrote:
> Cong Wang said, at 2012/9/10 20:48:
> > It doesn't save any code, nor it helps readability.
> > 
> > Signed-off-by: Cong Wang <amwang@redhat.com>
> 
> this macro, don't reduce showing lines?
> with it, more readability on terminal, right?

I can't agree, try to read the goto inside, you have to scroll your
screen down when you read the second BACTRACK().

> 
> P.S. when doing backport work, some bored patches influence me.
> this patch and [PATCH 3/3] are typical. :-)
> 

Why not just cherry-pick it too? `git cherry-pick` is handy. ;)

RE: [PATCH 2/3] ipv6, route: remove BACKTRACK() macro

[David Laight]

> Subject: [PATCH 2/3] ipv6, route: remove BACKTRACK() macro
> 
> It doesn't save any code, nor it helps readability.
> 
> Signed-off-by: Cong Wang <amwang@redhat.com>
> ---
>  net/ipv6/route.c |   48 ++++++++++++++++++++++++++++--------------------
>  1 files changed, 28 insertions(+), 20 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index b130bf2..71267e9 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -675,24 +675,6 @@ int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
>  }
>  #endif
> 
> -#define BACKTRACK(__net, saddr)			\
> -do { \
> -	if (rt == __net->ipv6.ip6_null_entry) {	\
> -		struct fib6_node *pn; \
> -		while (1) { \
> -			if (fn->fn_flags & RTN_TL_ROOT) \
> -				goto out; \
> -			pn = fn->parent; \
> -			if (FIB6_SUBTREE(pn) && FIB6_SUBTREE(pn) != fn) \
> -				fn = fib6_lookup(FIB6_SUBTREE(pn), NULL, saddr); \
> -			else \
> -				fn = pn; \
> -			if (fn->fn_flags & RTN_RTINFO) \
> -				goto restart; \
> -		} \
> -	} \
> -} while (0)
> -

It does, however, seem to avoid replicating some relatively complex logic.
OTOH goto out of a #define and hidden parameters (rt, fn ...) are not nice.

Maybe the 'while (1)' part can be an inline function?

	David

RE: [PATCH 2/3] ipv6, route: remove BACKTRACK() macro

[Cong Wang]

On Mon, 2012-09-10 at 14:54 +0100, David Laight wrote:
> 
> It does, however, seem to avoid replicating some relatively complex logic.
> OTOH goto out of a #define and hidden parameters (rt, fn ...) are not nice.
> 
> Maybe the 'while (1)' part can be an inline function?
> 

Yeah, actually that goto is the main reason why I think it should be
removed, at least I myself have some difficult to read this code which
jumps out. ;)

Re: [PATCH 2/3] ipv6, route: remove BACKTRACK() macro

[David Miller]

From: Cong Wang <amwang@redhat.com>
Date: Mon, 10 Sep 2012 20:48:45 +0800

> It doesn't save any code, nor it helps readability.
> 
> Signed-off-by: Cong Wang <amwang@redhat.com>

I'm not applying this.

Having two copies of the same exact logic means we will accumulate
bugs in the future if someone fixes the problem only in one
copy.

Re: [PATCH 2/3] ipv6, route: remove BACKTRACK() macro

[Cong Wang]

On Mon, 2012-09-10 at 16:32 -0400, David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> Date: Mon, 10 Sep 2012 20:48:45 +0800
> 
> > It doesn't save any code, nor it helps readability.
> > 
> > Signed-off-by: Cong Wang <amwang@redhat.com>
> 
> I'm not applying this.
> 
> Having two copies of the same exact logic means we will accumulate
> bugs in the future if someone fixes the problem only in one
> copy.

Makes sense, but BACKTRACK() is not well written, as it jumps out of its
definition. :(

Thanks.

Re: [PATCH 2/3] ipv6, route: remove BACKTRACK() macro

[David Miller]

From: Cong Wang <amwang@redhat.com>
Date: Tue, 11 Sep 2012 09:19:22 +0800

> On Mon, 2012-09-10 at 16:32 -0400, David Miller wrote:
>> From: Cong Wang <amwang@redhat.com>
>> Date: Mon, 10 Sep 2012 20:48:45 +0800
>> 
>> > It doesn't save any code, nor it helps readability.
>> > 
>> > Signed-off-by: Cong Wang <amwang@redhat.com>
>> 
>> I'm not applying this.
>> 
>> Having two copies of the same exact logic means we will accumulate
>> bugs in the future if someone fixes the problem only in one
>> copy.
> 
> Makes sense, but BACKTRACK() is not well written, as it jumps out of its
> definition. :(

Anyone who has worked with longest-matching-prefix TRIE traversal
functions will have a pretty good idea what is going on here.

Yes, I know this conflicts with cases like how we killed all of
the netlink macros with embedded gotos.

But this patch made things worse and added code duplication.  Fix
it without the code duplication side effect and it'll be fine.

Re: [PATCH 2/3] ipv6, route: remove BACKTRACK() macro

[Cong Wang]

On Mon, 2012-09-10 at 21:42 -0400, David Miller wrote:
> 
> Yes, I know this conflicts with cases like how we killed all of
> the netlink macros with embedded gotos.
> 
> But this patch made things worse and added code duplication.  Fix
> it without the code duplication side effect and it'll be fine.

Ok, will do.

Thanks, David!

[PATCH 3/3] fib6: remove some useless empty lines and comments

[Cong Wang]

There are too many empty lines in this source file,
remove the useless ones, together with some useless
comments.

Signed-off-by: Cong Wang <amwang@redhat.com>
---
 net/ipv6/ip6_fib.c |   78 +--------------------------------------------------
 1 files changed, 2 insertions(+), 76 deletions(-)

diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 13690d6..b4faf2d 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -88,7 +88,6 @@ static int fib6_walk_continue(struct fib6_walker_t *w);
  *	tested when modifications are made to the destination cache as a
  *	result of redirects, path MTU changes, etc.
  */
-
 static __u32 rt_sernum;
 
 static void fib6_gc_timer_cb(unsigned long arg);
@@ -123,10 +122,6 @@ static __inline__ u32 fib6_new_sernum(void)
  *	These assume a 32bit processor (although it will work on
  *	64bit processors)
  */
-
-/*
- *	test bit
- */
 #if defined(__LITTLE_ENDIAN)
 # define BITOP_BE32_SWIZZLE	(0x1F & ~7)
 #else
@@ -369,7 +364,6 @@ static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 
 	s_h = cb->args[0];
 	s_e = cb->args[1];
-
 	w = (void *)cb->args[2];
 	if (!w) {
 		/* New dump:
@@ -378,7 +372,6 @@ static int inet6_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
 		 */
 		cb->args[3] = (long)cb->done;
 		cb->done = fib6_dump_done;
-
 		/*
 		 * 2. allocate and initialize walker.
 		 */
@@ -426,7 +419,6 @@ out:
  *	by either creating and inserting or by returning an existing
  *	node.
  */
-
 static struct fib6_node * fib6_add_1(struct fib6_node *root, void *addr,
 				     int addrlen, int plen,
 				     int offset, int allow_create,
@@ -442,12 +434,9 @@ static struct fib6_node * fib6_add_1(struct fib6_node *root, void *addr,
 	RT6_TRACE("fib6_add_1\n");
 
 	/* insert node in tree */
-
 	fn = root;
-
 	do {
 		key = (struct rt6key *)((u8 *)fn->leaf + offset);
-
 		/*
 		 *	Prefix match
 		 */
@@ -466,23 +455,17 @@ static struct fib6_node * fib6_add_1(struct fib6_node *root, void *addr,
 		/*
 		 *	Exact match ?
 		 */
-
 		if (plen == fn->fn_bit) {
 			/* clean up an intermediate node */
 			if (!(fn->fn_flags & RTN_RTINFO)) {
 				rt6_release(fn->leaf);
 				fn->leaf = NULL;
 			}
-
 			fn->fn_sernum = sernum;
 
 			return fn;
 		}
 
-		/*
-		 *	We have more bits to go
-		 */
-
 		/* Try to walk down on tree. */
 		fn->fn_sernum = sernum;
 		dir = addr_bit_set(addr, fn->fn_bit);
@@ -512,11 +495,10 @@ static struct fib6_node * fib6_add_1(struct fib6_node *root, void *addr,
 	 */
 
 	ln = node_alloc();
-
 	if (!ln)
 		return NULL;
-	ln->fn_bit = plen;
 
+	ln->fn_bit = plen;
 	ln->parent = pn;
 	ln->fn_sernum = sernum;
 
@@ -527,7 +509,6 @@ static struct fib6_node * fib6_add_1(struct fib6_node *root, void *addr,
 
 	return ln;
 
-
 insert_above:
 	/*
 	 * split since we don't have a common prefix anymore or
@@ -536,17 +517,13 @@ insert_above:
 	 * this new node will point to the one we need to create
 	 * and the current
 	 */
-
 	pn = fn->parent;
-
 	/* find 1st bit in difference between the 2 addrs.
 
 	   See comment in __ipv6_addr_diff: bit may be an invalid value,
 	   but if it is >= plen, the value is ignored in any case.
 	 */
-
 	bit = __ipv6_addr_diff(addr, &key->addr, addrlen);
-
 	/*
 	 *		(intermediate)[in]
 	 *	          /	   \
@@ -571,15 +548,11 @@ insert_above:
 		 * the branches would not match less specific routes
 		 * in the other branch
 		 */
-
 		in->fn_bit = bit;
-
 		in->parent = pn;
 		in->leaf = fn->leaf;
 		atomic_inc(&in->leaf->rt6i_ref);
-
 		in->fn_sernum = sernum;
-
 		/* update parent pointer */
 		if (dir)
 			pn->right = in;
@@ -587,10 +560,8 @@ insert_above:
 			pn->left  = in;
 
 		ln->fn_bit = plen;
-
 		ln->parent = in;
 		fn->parent = in;
-
 		ln->fn_sernum = sernum;
 
 		if (addr_bit_set(addr, bit)) {
@@ -607,16 +578,12 @@ insert_above:
 		 *	          /	   \
 		 *	     (old node)[fn] NULL
 		 */
-
 		ln = node_alloc();
-
 		if (!ln)
 			return NULL;
 
 		ln->fn_bit = plen;
-
 		ln->parent = pn;
-
 		ln->fn_sernum = sernum;
 
 		if (dir)
@@ -637,7 +604,6 @@ insert_above:
 /*
  *	Insert routing information in a node.
  */
-
 static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 			    struct nl_info *info)
 {
@@ -650,12 +616,10 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 	int found = 0;
 
 	ins = &fn->leaf;
-
 	for (iter = fn->leaf; iter; iter = iter->dst.rt6_next) {
 		/*
 		 *	Search for duplicates
 		 */
-
 		if (iter->rt6i_metric == rt->rt6i_metric) {
 			/*
 			 *	Same priority level
@@ -667,7 +631,6 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 				found++;
 				break;
 			}
-
 			if (iter->dst.dev == rt->dst.dev &&
 			    iter->rt6i_idev == rt->rt6i_idev &&
 			    ipv6_addr_equal(&iter->rt6i_gateway,
@@ -681,24 +644,20 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
 				return -EEXIST;
 			}
 		}
-
 		if (iter->rt6i_metric > rt->rt6i_metric)
 			break;
-
 		ins = &iter->dst.rt6_next;
 	}
 
 	/* Reset round-robin state, if necessary */
 	if (ins == &fn->leaf)
 		fn->rr_ptr = NULL;
-
 	/*
 	 *	insert node
 	 */
 	if (!replace) {
 		if (!add)
 			pr_warn("NLM_F_CREATE should be set when creating new route\n");
-
 add:
 		rt->dst.rt6_next = iter;
 		*ins = rt;
@@ -706,12 +665,10 @@ add:
 		atomic_inc(&rt->rt6i_ref);
 		inet6_rt_notify(RTM_NEWROUTE, rt, info);
 		info->nl_net->ipv6.rt6_stats->fib_rt_entries++;
-
 		if (!(fn->fn_flags & RTN_RTINFO)) {
 			info->nl_net->ipv6.rt6_stats->fib_route_nodes++;
 			fn->fn_flags |= RTN_RTINFO;
 		}
-
 	} else {
 		if (!found) {
 			if (add)
@@ -818,7 +775,6 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info)
 					sizeof(struct in6_addr), rt->rt6i_src.plen,
 					offsetof(struct rt6_info, rt6i_src),
 					allow_create, replace_required);
-
 			if (!sn) {
 				/* If it is failed, discard just allocated
 				   root, and then (in st_failure) stale node
@@ -827,7 +783,6 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info)
 				node_free(sfn);
 				goto st_failure;
 			}
-
 			/* Now link new subtree to main tree */
 			sfn->parent = fn;
 			fn->subtree = sfn;
@@ -836,7 +791,6 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info)
 					sizeof(struct in6_addr), rt->rt6i_src.plen,
 					offsetof(struct rt6_info, rt6i_src),
 					allow_create, replace_required);
-
 			if (IS_ERR(sn)) {
 				err = PTR_ERR(sn);
 				sn = NULL;
@@ -852,7 +806,6 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt, struct nl_info *info)
 		fn = sn;
 	}
 #endif
-
 	err = fib6_add_rt2node(fn, rt, info);
 	if (!err) {
 		fib6_start_gc(info->nl_net, rt);
@@ -916,20 +869,15 @@ static struct fib6_node * fib6_lookup_1(struct fib6_node *root,
 
 	if (unlikely(args->offset == 0))
 		return NULL;
-
 	/*
 	 *	Descend on a tree
 	 */
-
 	fn = root;
-
 	for (;;) {
 		struct fib6_node *next;
 
 		dir = addr_bit_set(args->addr, fn->fn_bit);
-
 		next = dir ? fn->right : fn->left;
-
 		if (next) {
 			fn = next;
 			continue;
@@ -943,7 +891,6 @@ static struct fib6_node * fib6_lookup_1(struct fib6_node *root,
 
 			key = (struct rt6key *) ((u8 *) fn->leaf +
 						 args->offset);
-
 			if (ipv6_prefix_equal(&key->addr, args->addr, key->plen)) {
 #ifdef CONFIG_IPV6_SUBTREES
 				if (fn->subtree)
@@ -953,10 +900,8 @@ static struct fib6_node * fib6_lookup_1(struct fib6_node *root,
 					return fn;
 			}
 		}
-
 		if (fn->fn_flags & RTN_ROOT)
 			break;
-
 		fn = fn->parent;
 	}
 
@@ -994,8 +939,6 @@ struct fib6_node * fib6_lookup(struct fib6_node *root, const struct in6_addr *da
  *	Get node with specified destination prefix (and source prefix,
  *	if subtrees are used)
  */
-
-
 static struct fib6_node * fib6_locate_1(struct fib6_node *root,
 					const struct in6_addr *addr,
 					int plen, int offset)
@@ -1004,17 +947,14 @@ static struct fib6_node * fib6_locate_1(struct fib6_node *root,
 
 	for (fn = root; fn ; ) {
 		struct rt6key *key = (struct rt6key *)((u8 *)fn->leaf + offset);
-
 		/*
 		 *	Prefix match
 		 */
 		if (plen < fn->fn_bit ||
 		    !ipv6_prefix_equal(&key->addr, addr, fn->fn_bit))
 			return NULL;
-
 		if (plen == fn->fn_bit)
 			return fn;
-
 		/*
 		 *	We have more bits to go
 		 */
@@ -1034,7 +974,6 @@ struct fib6_node * fib6_locate(struct fib6_node *root,
 
 	fn = fib6_locate_1(root, daddr, dst_len,
 			   offsetof(struct rt6_info, rt6i_dst));
-
 #ifdef CONFIG_IPV6_SUBTREES
 	if (src_len) {
 		WARN_ON(saddr == NULL);
@@ -1043,19 +982,12 @@ struct fib6_node * fib6_locate(struct fib6_node *root,
 					   offsetof(struct rt6_info, rt6i_src));
 	}
 #endif
-
 	if (fn && fn->fn_flags & RTN_RTINFO)
 		return fn;
 
 	return NULL;
 }
 
-
-/*
- *	Deletion
- *
- */
-
 static struct rt6_info *fib6_find_prefix(struct net *net, struct fib6_node *fn)
 {
 	if (fn->fn_flags & RTN_ROOT)
@@ -1076,7 +1008,6 @@ static struct rt6_info *fib6_find_prefix(struct net *net, struct fib6_node *fn)
  *	Called to trim the tree of intermediate nodes when possible. "fn"
  *	is the node we want to try and remove.
  */
-
 static struct fib6_node *fib6_repair_tree(struct net *net,
 					   struct fib6_node *fn)
 {
@@ -1270,11 +1201,9 @@ int fib6_del(struct rt6_info *rt, struct nl_info *info)
 #endif
 		fib6_prune_clones(info->nl_net, pn, rt);
 	}
-
 	/*
 	 *	Walk the leaf entries looking for ourself
 	 */
-
 	for (rtp = &fn->leaf; *rtp; rtp = &(*rtp)->dst.rt6_next) {
 		if (*rtp == rt) {
 			fib6_del_route(fn, rtp, info);
@@ -1307,7 +1236,6 @@ int fib6_del(struct rt6_info *rt, struct nl_info *info)
  *	>0  -> walk is incomplete (i.e. suspended)
  *	<0  -> walk is terminated by an error.
  */
-
 static int fib6_walk_continue(struct fib6_walker_t *w)
 {
 	struct fib6_node *fn, *pn;
@@ -1444,7 +1372,6 @@ static int fib6_clean_node(struct fib6_walker_t *w)
  *	prune==1 -> only immediate children of node (certainly,
  *	ignoring pure split nodes) will be scanned.
  */
-
 static void fib6_clean_tree(struct net *net, struct fib6_node *root,
 			    int (*func)(struct rt6_info *, void *arg),
 			    int prune, void *arg)
@@ -1483,6 +1410,7 @@ void fib6_clean_all_ro(struct net *net, int (*func)(struct rt6_info *, void *arg
 	}
 	rcu_read_unlock();
 }
+
 void fib6_clean_all(struct net *net, int (*func)(struct rt6_info *, void *arg),
 		    int prune, void *arg)
 {
@@ -1523,7 +1451,6 @@ static void fib6_prune_clones(struct net *net, struct fib6_node *fn,
 /*
  *	Garbage collection
  */
-
 static struct fib6_gc_args
 {
 	int			timeout;
@@ -1541,7 +1468,6 @@ static int fib6_age(struct rt6_info *rt, void *arg)
 	 *	Also age clones. Note, that clones are aged out
 	 *	only if they are not in use now.
 	 */
-
 	if (rt->rt6i_flags & RTF_EXPIRES && rt->dst.expires) {
 		if (time_after(now, rt->dst.expires)) {
 			RT6_TRACE("expiring %p\n", rt);
-- 
1.7.7.6

Re: [PATCH 3/3] fib6: remove some useless empty lines and comments

[David Miller]

From: Cong Wang <amwang@redhat.com>
Date: Mon, 10 Sep 2012 20:48:46 +0800

> There are too many empty lines in this source file,
> remove the useless ones, together with some useless
> comments.
> 
> Signed-off-by: Cong Wang <amwang@redhat.com>

This patch is of zero value, please find something more useful to work
on and contribute, I'm not applying this.

Re: [PATCH 3/3] fib6: remove some useless empty lines and comments

[Cong Wang]

On Mon, 2012-09-10 at 16:33 -0400, David Miller wrote:
> From: Cong Wang <amwang@redhat.com>
> Date: Mon, 10 Sep 2012 20:48:46 +0800
> 
> > There are too many empty lines in this source file,
> > remove the useless ones, together with some useless
> > comments.
> > 
> > Signed-off-by: Cong Wang <amwang@redhat.com>
> 
> This patch is of zero value, please find something more useful to work
> on and contribute, I'm not applying this.

Maybe useful for people like me who don't have a large screen. :)

Anyway, feel free to ignore it.

Re: [PATCH 1/3] ipv6: remove some useless RCU read lock

[David Miller]

From: Cong Wang <amwang@redhat.com>
Date: Mon, 10 Sep 2012 20:48:44 +0800

> After this commit:
> 	commit 97cac0821af4474ec4ba3a9e7a36b98ed9b6db88
> 	Author: David S. Miller <davem@davemloft.net>
> 	Date:   Mon Jul 2 22:43:47 2012 -0700
> 
> 	    ipv6: Store route neighbour in rt6_info struct.
> 
> we no longer use RCU to protect route neighbour.
> 
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Cong Wang <amwang@redhat.com>

Applied to net-next, thanks.