Patchwork [net,2/3] ipv6: fill rt6i_gateway with nexthop address

login
register
mail settings
Submitter Julian Anastasov
Date Oct. 20, 2013, 12:43 p.m.
Message ID <1382272985-1528-3-git-send-email-ja@ssi.bg>
Download mbox | patch
Permalink /patch/284968/
State Accepted
Delegated to: David Miller
Headers show

Comments

Julian Anastasov - Oct. 20, 2013, 12:43 p.m.
Make sure rt6i_gateway contains nexthop information in
all routes returned from lookup or when routes are directly
attached to skb for generated ICMP packets.

The effect of this patch should be a faster version of
rt6_nexthop() and the consideration of local addresses as
nexthop.

Signed-off-by: Julian Anastasov <ja@ssi.bg>
---
 include/net/ip6_route.h | 6 ++----
 net/ipv6/ip6_output.c   | 4 ++--
 net/ipv6/route.c        | 8 ++++++--
 3 files changed, 10 insertions(+), 8 deletions(-)
Hannes Frederic Sowa - Oct. 20, 2013, 6:33 p.m.
On Sun, Oct 20, 2013 at 03:43:04PM +0300, Julian Anastasov wrote:
> Make sure rt6i_gateway contains nexthop information in
> all routes returned from lookup or when routes are directly
> attached to skb for generated ICMP packets.
> 
> The effect of this patch should be a faster version of
> rt6_nexthop() and the consideration of local addresses as
> nexthop.
> 
> Signed-off-by: Julian Anastasov <ja@ssi.bg>
> ---
>  include/net/ip6_route.h | 6 ++----
>  net/ipv6/ip6_output.c   | 4 ++--
>  net/ipv6/route.c        | 8 ++++++--
>  3 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> index 481404a..2b786b7 100644
> --- a/include/net/ip6_route.h
> +++ b/include/net/ip6_route.h
> @@ -194,11 +194,9 @@ static inline int ip6_skb_dst_mtu(struct sk_buff *skb)
>  	       skb_dst(skb)->dev->mtu : dst_mtu(skb_dst(skb));
>  }
>  
> -static inline struct in6_addr *rt6_nexthop(struct rt6_info *rt, struct in6_addr *dest)
> +static inline struct in6_addr *rt6_nexthop(struct rt6_info *rt)
>  {
> -	if (rt->rt6i_flags & RTF_GATEWAY || !ipv6_addr_any(&rt->rt6i_gateway))
> -		return &rt->rt6i_gateway;
> -	return dest;
> +	return &rt->rt6i_gateway;
>  }
>  

Hm, looks good. Will test is soon.

One small remark:

Maybe we can kill rt6_nexthop completly and use rt6i_gateway directly so a
reader does not have to lookup this function.

Thanks,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hannes Frederic Sowa - Oct. 21, 2013, 5:01 a.m.
On Sun, Oct 20, 2013 at 03:43:04PM +0300, Julian Anastasov wrote:
> Make sure rt6i_gateway contains nexthop information in
> all routes returned from lookup or when routes are directly
> attached to skb for generated ICMP packets.
> 
> The effect of this patch should be a faster version of
> rt6_nexthop() and the consideration of local addresses as
> nexthop.
> 
> Signed-off-by: Julian Anastasov <ja@ssi.bg>

The patch is fine. I don't mind if we leave it as is or remove rt6_nexthop,
so:

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

Thanks,

  Hannes

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Julian Anastasov - Oct. 21, 2013, 7:31 a.m.
Hello,

On Mon, 21 Oct 2013, Hannes Frederic Sowa wrote:

> On Sun, Oct 20, 2013 at 03:43:04PM +0300, Julian Anastasov wrote:
> > Make sure rt6i_gateway contains nexthop information in
> > all routes returned from lookup or when routes are directly
> > attached to skb for generated ICMP packets.
> > 
> > The effect of this patch should be a faster version of
> > rt6_nexthop() and the consideration of local addresses as
> > nexthop.
> > 
> > Signed-off-by: Julian Anastasov <ja@ssi.bg>
> 
> The patch is fine. I don't mind if we leave it as is or remove rt6_nexthop,
> so:
> 
> Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

	Thanks for the review! I don't mind too about
removing rt6_nexthop. For me it is 51% against 49% to keep it
as it denotes the places that use nexthop and not gateway.
May be more opinions will help to decide because I don't know
if there are any plans to use similar techniques as done for IPv4.

Regards

--
Julian Anastasov <ja@ssi.bg>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Oct. 21, 2013, 10:42 p.m.
From: Julian Anastasov <ja@ssi.bg>
Date: Mon, 21 Oct 2013 10:31:06 +0300 (EEST)

> 	Thanks for the review! I don't mind too about
> removing rt6_nexthop. For me it is 51% against 49% to keep it
> as it denotes the places that use nexthop and not gateway.
> May be more opinions will help to decide because I don't know
> if there are any plans to use similar techniques as done for IPv4.

I have no strong opinion about removing rt6_nexthop.

If it is of low cost today and makes the code easier to
undersand and grep, just leave it alone.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 481404a..2b786b7 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -194,11 +194,9 @@  static inline int ip6_skb_dst_mtu(struct sk_buff *skb)
 	       skb_dst(skb)->dev->mtu : dst_mtu(skb_dst(skb));
 }
 
-static inline struct in6_addr *rt6_nexthop(struct rt6_info *rt, struct in6_addr *dest)
+static inline struct in6_addr *rt6_nexthop(struct rt6_info *rt)
 {
-	if (rt->rt6i_flags & RTF_GATEWAY || !ipv6_addr_any(&rt->rt6i_gateway))
-		return &rt->rt6i_gateway;
-	return dest;
+	return &rt->rt6i_gateway;
 }
 
 #endif
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index a54c45c..2b404df 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -105,7 +105,7 @@  static int ip6_finish_output2(struct sk_buff *skb)
 	}
 
 	rcu_read_lock_bh();
-	nexthop = rt6_nexthop((struct rt6_info *)dst, &ipv6_hdr(skb)->daddr);
+	nexthop = rt6_nexthop((struct rt6_info *)dst);
 	neigh = __ipv6_neigh_lookup_noref(dst->dev, nexthop);
 	if (unlikely(!neigh))
 		neigh = __neigh_create(&nd_tbl, nexthop, dst->dev, false);
@@ -874,7 +874,7 @@  static int ip6_dst_lookup_tail(struct sock *sk,
 	 */
 	rt = (struct rt6_info *) *dst;
 	rcu_read_lock_bh();
-	n = __ipv6_neigh_lookup_noref(rt->dst.dev, rt6_nexthop(rt, &fl6->daddr));
+	n = __ipv6_neigh_lookup_noref(rt->dst.dev, rt6_nexthop(rt));
 	err = n && !(n->nud_state & NUD_VALID) ? -EINVAL : 0;
 	rcu_read_unlock_bh();
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c979dd9..c1ee381 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -851,7 +851,6 @@  static struct rt6_info *rt6_alloc_cow(struct rt6_info *ort,
 			if (ort->rt6i_dst.plen != 128 &&
 			    ipv6_addr_equal(&ort->rt6i_dst.addr, daddr))
 				rt->rt6i_flags |= RTF_ANYCAST;
-			rt->rt6i_gateway = *daddr;
 		}
 
 		rt->rt6i_flags |= RTF_CACHE;
@@ -1338,6 +1337,7 @@  struct dst_entry *icmp6_dst_alloc(struct net_device *dev,
 	rt->dst.flags |= DST_HOST;
 	rt->dst.output  = ip6_output;
 	atomic_set(&rt->dst.__refcnt, 1);
+	rt->rt6i_gateway  = fl6->daddr;
 	rt->rt6i_dst.addr = fl6->daddr;
 	rt->rt6i_dst.plen = 128;
 	rt->rt6i_idev     = idev;
@@ -1873,7 +1873,10 @@  static struct rt6_info *ip6_rt_copy(struct rt6_info *ort,
 			in6_dev_hold(rt->rt6i_idev);
 		rt->dst.lastuse = jiffies;
 
-		rt->rt6i_gateway = ort->rt6i_gateway;
+		if (ort->rt6i_flags & RTF_GATEWAY)
+			rt->rt6i_gateway = ort->rt6i_gateway;
+		else
+			rt->rt6i_gateway = *dest;
 		rt->rt6i_flags = ort->rt6i_flags;
 		if ((ort->rt6i_flags & (RTF_DEFAULT | RTF_ADDRCONF)) ==
 		    (RTF_DEFAULT | RTF_ADDRCONF))
@@ -2160,6 +2163,7 @@  struct rt6_info *addrconf_dst_alloc(struct inet6_dev *idev,
 	else
 		rt->rt6i_flags |= RTF_LOCAL;
 
+	rt->rt6i_gateway  = *addr;
 	rt->rt6i_dst.addr = *addr;
 	rt->rt6i_dst.plen = 128;
 	rt->rt6i_table = fib6_get_table(net, RT6_TABLE_LOCAL);