| Message ID | 1361343610.19353.177.camel@edumazet-glaptop |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On 2013/02/20 15:00, Eric Dumazet wrote: > On Wed, 2013-02-20 at 14:33 +0800, Gao feng wrote: > >> How can we? >> one usage of rt6_update_expires and rt6_set_expires >> is changing rt6->dst.from to rt6->dst.expires, we should release the >> already holded reference of rt6->dst.from. >> > > Just don't union the two fields, as Neil did. > > Setting the dst.expires value should not change dst.from at all. > Get it,it looks good to me.we can make sure dst->from doesn't being operated at same time. Thanks! -- 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
Eric Dumazet wrote: > On Wed, 2013-02-20 at 14:33 +0800, Gao feng wrote: > >> How can we? >> one usage of rt6_update_expires and rt6_set_expires >> is changing rt6->dst.from to rt6->dst.expires, we should release the >> already holded reference of rt6->dst.from. >> > > Just don't union the two fields, as Neil did. > > Setting the dst.expires value should not change dst.from at all. > > Something like the (untested, because its too late here) patch ? > > include/net/dst.h | 11 +++++------ > include/net/ip6_fib.h | 21 +++------------------ > net/ipv6/route.c | 3 +-- > 3 files changed, 9 insertions(+), 26 deletions(-) > > diff --git a/include/net/dst.h b/include/net/dst.h > index 3da47e0..3f31a48 100644 > --- a/include/net/dst.h > +++ b/include/net/dst.h > @@ -36,13 +36,12 @@ struct dst_entry { > struct net_device *dev; > struct dst_ops *ops; > unsigned long _metrics; > - union { > - unsigned long expires; > - /* point to where the dst_entry copied from */ > - struct dst_entry *from; > - }; > + unsigned long expires; > + > + /* point to where the dst_entry copied from */ > + struct dst_entry *from; > + > struct dst_entry *path; > - void *__pad0; > #ifdef CONFIG_XFRM > struct xfrm_state *xfrm; > #else Initialize "from" in dst_alloc(). > @@ -199,13 +185,12 @@ static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from) > { > struct dst_entry *new = (struct dst_entry *) from; > > - if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) { > + if (rt->dst.from) { > if (new == rt->dst.from) > return; > dst_release(rt->dst.from); > } > > - rt->rt6i_flags &= ~RTF_EXPIRES; > rt->dst.from = new; > dst_hold(new); > } rt6_set_from can work only for fresh entries. In fact, I have very similar patch. --yoshfuji -- 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
diff --git a/include/net/dst.h b/include/net/dst.h index 3da47e0..3f31a48 100644 --- a/include/net/dst.h +++ b/include/net/dst.h @@ -36,13 +36,12 @@ struct dst_entry { struct net_device *dev; struct dst_ops *ops; unsigned long _metrics; - union { - unsigned long expires; - /* point to where the dst_entry copied from */ - struct dst_entry *from; - }; + unsigned long expires; + + /* point to where the dst_entry copied from */ + struct dst_entry *from; + struct dst_entry *path; - void *__pad0; #ifdef CONFIG_XFRM struct xfrm_state *xfrm; #else diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h index 6919a50..731b35c 100644 --- a/include/net/ip6_fib.h +++ b/include/net/ip6_fib.h @@ -164,33 +164,19 @@ static inline struct inet6_dev *ip6_dst_idev(struct dst_entry *dst) static inline void rt6_clean_expires(struct rt6_info *rt) { - if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) - dst_release(rt->dst.from); - rt->rt6i_flags &= ~RTF_EXPIRES; - rt->dst.from = NULL; + rt->dst.expires = 0; } static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires) { - if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) - dst_release(rt->dst.from); - rt->rt6i_flags |= RTF_EXPIRES; rt->dst.expires = expires; } static inline void rt6_update_expires(struct rt6_info *rt, int timeout) { - if (!(rt->rt6i_flags & RTF_EXPIRES)) { - if (rt->dst.from) - dst_release(rt->dst.from); - /* dst_set_expires relies on expires == 0 - * if it has not been set previously. - */ - rt->dst.expires = 0; - } - + rt->dst.expires = 0; dst_set_expires(&rt->dst, timeout); rt->rt6i_flags |= RTF_EXPIRES; } @@ -199,13 +185,12 @@ static inline void rt6_set_from(struct rt6_info *rt, struct rt6_info *from) { struct dst_entry *new = (struct dst_entry *) from; - if (!(rt->rt6i_flags & RTF_EXPIRES) && rt->dst.from) { + if (rt->dst.from) { if (new == rt->dst.from) return; dst_release(rt->dst.from); } - rt->rt6i_flags &= ~RTF_EXPIRES; rt->dst.from = new; dst_hold(new); } diff --git a/net/ipv6/route.c b/net/ipv6/route.c index 515bb51..6874b6e 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -296,8 +296,7 @@ static void ip6_dst_destroy(struct dst_entry *dst) in6_dev_put(idev); } - if (!(rt->rt6i_flags & RTF_EXPIRES) && dst->from) - dst_release(dst->from); + dst_release(dst->from); if (rt6_has_peer(rt)) { struct inet_peer *peer = rt6_peer_ptr(rt);