diff mbox series

[v2,net-next,3/5] ipv6: honor RT6_LOOKUP_F_DST_NOREF in rule lookup logic

Message ID 20190619223158.35829-4-tracywwnj@gmail.com
State Changes Requested
Delegated to: David Miller
Headers show
Series ipv6: avoid taking refcnt on dst during route lookup | expand

Commit Message

Wei Wang June 19, 2019, 10:31 p.m. UTC 
From: Wei Wang <weiwan@google.com>

This patch specifically converts the rule lookup logic to honor this
flag and not release refcnt when traversing each rule and calling
lookup() on each routing table.
Similar to previous patch, we also need some special handling of dst
entries in uncached list because there is always 1 refcnt taken for them
even if RT6_LOOKUP_F_DST_NOREF flag is set.

Signed-off-by: Wei Wang <weiwan@google.com>
---
 include/net/ip6_route.h | 10 ++++++++++
 net/ipv6/fib6_rules.c   | 12 +++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

Comments

David Ahern June 19, 2019, 11:12 p.m. UTC | #1 
On 6/19/19 4:31 PM, Wei Wang wrote:
> diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
> index bcfae13409b5..d22b6c140f23 100644
> --- a/net/ipv6/fib6_rules.c
> +++ b/net/ipv6/fib6_rules.c
> @@ -113,14 +113,15 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
>  		rt = lookup(net, net->ipv6.fib6_local_tbl, fl6, skb, flags);
>  		if (rt != net->ipv6.ip6_null_entry && rt->dst.error != -EAGAIN)
>  			return &rt->dst;
> -		ip6_rt_put(rt);
> +		ip6_rt_put_flags(rt, flags);
>  		rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, skb, flags);
>  		if (rt->dst.error != -EAGAIN)
>  			return &rt->dst;
> -		ip6_rt_put(rt);
> +		ip6_rt_put_flags(rt, flags);
>  	}
>  
> -	dst_hold(&net->ipv6.ip6_null_entry->dst);
> +	if (!(flags & RT6_LOOKUP_F_DST_NOREF))
> +		dst_hold(&net->ipv6.ip6_null_entry->dst);
>  	return &net->ipv6.ip6_null_entry->dst;
>  }
>  

What about the fib6_rule_lookup when CONFIG_IPV6_MULTIPLE_TABLES is
configured off? If caller passes in RT6_LOOKUP_F_DST_NOREF that version
is still taking a reference in the error path and does a put after the
lookup.
Wei Wang June 19, 2019, 11:25 p.m. UTC | #2 
On Wed, Jun 19, 2019 at 4:12 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 6/19/19 4:31 PM, Wei Wang wrote:
> > diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
> > index bcfae13409b5..d22b6c140f23 100644
> > --- a/net/ipv6/fib6_rules.c
> > +++ b/net/ipv6/fib6_rules.c
> > @@ -113,14 +113,15 @@ struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
> >               rt = lookup(net, net->ipv6.fib6_local_tbl, fl6, skb, flags);
> >               if (rt != net->ipv6.ip6_null_entry && rt->dst.error != -EAGAIN)
> >                       return &rt->dst;
> > -             ip6_rt_put(rt);
> > +             ip6_rt_put_flags(rt, flags);
> >               rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, skb, flags);
> >               if (rt->dst.error != -EAGAIN)
> >                       return &rt->dst;
> > -             ip6_rt_put(rt);
> > +             ip6_rt_put_flags(rt, flags);
> >       }
> >
> > -     dst_hold(&net->ipv6.ip6_null_entry->dst);
> > +     if (!(flags & RT6_LOOKUP_F_DST_NOREF))
> > +             dst_hold(&net->ipv6.ip6_null_entry->dst);
> >       return &net->ipv6.ip6_null_entry->dst;
> >  }
> >
>
> What about the fib6_rule_lookup when CONFIG_IPV6_MULTIPLE_TABLES is
> configured off? If caller passes in RT6_LOOKUP_F_DST_NOREF that version
> is still taking a reference in the error path and does a put after the
> lookup.

True. I missed that part. Thanks for catching it. Will update.
diff mbox series

Patch

diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 82bced2fc1e3..0709835c01ad 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -94,6 +94,16 @@  static inline struct dst_entry *ip6_route_output(struct net *net,
 	return ip6_route_output_flags(net, sk, fl6, 0);
 }
 
+/* Only conditionally release dst if flags indicates
+ * !RT6_LOOKUP_F_DST_NOREF or dst is in uncached_list.
+ */
+static inline void ip6_rt_put_flags(struct rt6_info *rt, int flags)
+{
+	if (!(flags & RT6_LOOKUP_F_DST_NOREF) ||
+	    !list_empty(&rt->rt6i_uncached))
+		ip6_rt_put(rt);
+}
+
 struct dst_entry *ip6_route_lookup(struct net *net, struct flowi6 *fl6,
 				   const struct sk_buff *skb, int flags);
 struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index bcfae13409b5..d22b6c140f23 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -113,14 +113,15 @@  struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
 		rt = lookup(net, net->ipv6.fib6_local_tbl, fl6, skb, flags);
 		if (rt != net->ipv6.ip6_null_entry && rt->dst.error != -EAGAIN)
 			return &rt->dst;
-		ip6_rt_put(rt);
+		ip6_rt_put_flags(rt, flags);
 		rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, skb, flags);
 		if (rt->dst.error != -EAGAIN)
 			return &rt->dst;
-		ip6_rt_put(rt);
+		ip6_rt_put_flags(rt, flags);
 	}
 
-	dst_hold(&net->ipv6.ip6_null_entry->dst);
+	if (!(flags & RT6_LOOKUP_F_DST_NOREF))
+		dst_hold(&net->ipv6.ip6_null_entry->dst);
 	return &net->ipv6.ip6_null_entry->dst;
 }
 
@@ -237,13 +238,14 @@  static int __fib6_rule_action(struct fib_rule *rule, struct flowi *flp,
 			goto out;
 	}
 again:
-	ip6_rt_put(rt);
+	ip6_rt_put_flags(rt, flags);
 	err = -EAGAIN;
 	rt = NULL;
 	goto out;
 
 discard_pkt:
-	dst_hold(&rt->dst);
+	if (!(flags & RT6_LOOKUP_F_DST_NOREF))
+		dst_hold(&rt->dst);
 out:
 	res->rt6 = rt;
 	return err;