Message ID | 20190618182543.65477-4-tracywwnj@gmail.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | ipv6: avoid taking refcnt on dst during route lookup | expand |
From: Wei Wang <tracywwnj@gmail.com> Date: Tue, 18 Jun 2019 11:25:41 -0700 > @@ -237,13 +240,16 @@ static int __fib6_rule_action(struct fib_rule *rule, struct flowi *flp, > goto out; > } > again: > - ip6_rt_put(rt); > + if (!(flags & RT6_LOOKUP_F_DST_NOREF) || > + !list_empty(&rt->rt6i_uncached)) > + ip6_rt_put(rt); This conditional release logic, with the special treatment of uncache items when using DST_NOREF, seems error prone. Maybe you can put this logic into a helper like ip6_rt_put_any() and do the list empty test etc. there? ip6_rt_put_any(struct rt6_info *rt, int flags); What do you think?
On Wed, Jun 19, 2019 at 9:07 AM David Miller <davem@davemloft.net> wrote: > > From: Wei Wang <tracywwnj@gmail.com> > Date: Tue, 18 Jun 2019 11:25:41 -0700 > > > @@ -237,13 +240,16 @@ static int __fib6_rule_action(struct fib_rule *rule, struct flowi *flp, > > goto out; > > } > > again: > > - ip6_rt_put(rt); > > + if (!(flags & RT6_LOOKUP_F_DST_NOREF) || > > + !list_empty(&rt->rt6i_uncached)) > > + ip6_rt_put(rt); > > This conditional release logic, with the special treatment of uncache items > when using DST_NOREF, seems error prone. > > Maybe you can put this logic into a helper like ip6_rt_put_any() and do the > list empty test etc. there? > > ip6_rt_put_any(struct rt6_info *rt, int flags); > > What do you think? Thanks David. Sure. Will update.
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c index bcfae13409b5..9bbcf550cceb 100644 --- a/net/ipv6/fib6_rules.c +++ b/net/ipv6/fib6_rules.c @@ -113,14 +113,17 @@ 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); + if (!(flags & RT6_LOOKUP_F_DST_NOREF)) + ip6_rt_put(rt); rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, skb, flags); if (rt->dst.error != -EAGAIN) return &rt->dst; - ip6_rt_put(rt); + if (!(flags & RT6_LOOKUP_F_DST_NOREF)) + ip6_rt_put(rt); } - 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 +240,16 @@ static int __fib6_rule_action(struct fib_rule *rule, struct flowi *flp, goto out; } again: - ip6_rt_put(rt); + if (!(flags & RT6_LOOKUP_F_DST_NOREF) || + !list_empty(&rt->rt6i_uncached)) + ip6_rt_put(rt); 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;