On Mon, Feb 26, 2018 at 03:55:14PM -0700, David Ahern wrote:

On 2/26/18 3:28 PM, Wei Wang wrote:

quoted

quoted

@@ -213,11 +234,6 @@ static inline void rt6_set_expires(struct rt6_info *rt, unsigned long expires)

 static inline void rt6_update_expires(struct rt6_info *rt0, int timeout)
 {
-       struct rt6_info *rt;
-
-       for (rt = rt0; rt && !(rt->rt6i_flags & RTF_EXPIRES); rt = rt->from);
-       if (rt && rt != rt0)
-               rt0->dst.expires = rt->dst.expires;

I was wondering if we need to retain the above logic. It makes sure
dst.expires gets synced to its "parent" route. But  it might be hard
because after your change, we can no longer use rt->from to refer to
the "parent".

As I understand it, the FIB entries are cloned into pcpu, uncached and
exception routes. We should never have an rt6_info that ever points back
more than 1 level -- ie., the dst rt6_info points to a from representing
the original FIB entry.

Agree on at most 1 level.

After my change 'from' will still point to the FIB entry as a fib6_info
which has its own expires.

When I looked this code I was really confused. At best, the for loop
above sets rt0->dst.expires to some value based on the 'from' but then
the very next line calls dst_set_expires with the passed in timeout value.

My understanding is, the rt0 first inherits the expires from its rt0->from.

The following dst_set_expires() set a new timeout if the new timeout
is earlier than the existing expires.  I think it is essentially
taking a min.

One question, would avoid taking the min cause the rt0 somehow
have a longer expires than its parent (or f6i after this series)?

quoted

quoted

        dst_set_expires(&rt0->dst, timeout);
        rt0->rt6i_flags |= RTF_EXPIRES;
 }