On 2016/10/02 19:37, Martin Pieuchot wrote:
> I'm trying to figure out where the regression in IPv6/NDP is and here's
> what I found so far.
>
> When the route expiration time got converted from time_second to
> time_uptime we forgot to do the same for values inside RAs. I'm not
> sure what's the real impact of this, but it is clearly wrong. Diff
> below should fix that.
>
> ok?
I haven't been able to trigger the problem again even without the
diff, but looking in ntpd log there have been a few time offsets
recently on that machine so it sounds like a good candidate.
No problems seen with the diff yet, and makes sense and reads well
to me, so OK sthen@.
> Index: netinet6/in6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6.c,v
> retrieving revision 1.192
> diff -u -p -r1.192 in6.c
> --- netinet6/in6.c 4 Sep 2016 10:32:01 -0000 1.192
> +++ netinet6/in6.c 2 Oct 2016 17:18:24 -0000
> @@ -353,7 +353,7 @@ in6_ioctl(u_long cmd, caddr_t data, stru
> case SIOCGIFALIFETIME_IN6:
> ifr->ifr_ifru.ifru_lifetime = ia6->ia6_lifetime;
> if (ia6->ia6_lifetime.ia6t_vltime != ND6_INFINITE_LIFETIME) {
> - time_t maxexpire;
> + time_t expire, maxexpire;
> struct in6_addrlifetime *retlt =
> &ifr->ifr_ifru.ifru_lifetime;
>
> @@ -365,8 +365,13 @@ in6_ioctl(u_long cmd, caddr_t data, stru
> (time_t)~(1ULL << ((sizeof(maxexpire) * 8) - 1));
> if (ia6->ia6_lifetime.ia6t_vltime <
> maxexpire - ia6->ia6_updatetime) {
> - retlt->ia6t_expire = ia6->ia6_updatetime +
> + expire = ia6->ia6_updatetime +
> ia6->ia6_lifetime.ia6t_vltime;
> + if (expire != 0) {
> + expire -= time_uptime;
> + expire += time_second;
> + }
> + retlt->ia6t_expire = expire;
> } else
> retlt->ia6t_expire = maxexpire;
> }
> @@ -604,7 +609,7 @@ in6_update_ifa(struct ifnet *ifp, struct
> ia6->ia_ifa.ifa_addr = sin6tosa(&ia6->ia_addr);
> ia6->ia_addr.sin6_family = AF_INET6;
> ia6->ia_addr.sin6_len = sizeof(ia6->ia_addr);
> - ia6->ia6_createtime = ia6->ia6_updatetime = time_second;
> + ia6->ia6_createtime = ia6->ia6_updatetime = time_uptime;
> if ((ifp->if_flags & (IFF_POINTOPOINT | IFF_LOOPBACK)) != 0) {
> /*
> * XXX: some functions expect that ifa_dstaddr is not
> @@ -667,12 +672,12 @@ in6_update_ifa(struct ifnet *ifp, struct
> ia6->ia6_lifetime = ifra->ifra_lifetime;
> if (ia6->ia6_lifetime.ia6t_vltime != ND6_INFINITE_LIFETIME) {
> ia6->ia6_lifetime.ia6t_expire =
> - time_second + ia6->ia6_lifetime.ia6t_vltime;
> + time_uptime + ia6->ia6_lifetime.ia6t_vltime;
> } else
> ia6->ia6_lifetime.ia6t_expire = 0;
> if (ia6->ia6_lifetime.ia6t_pltime != ND6_INFINITE_LIFETIME) {
> ia6->ia6_lifetime.ia6t_preferred =
> - time_second + ia6->ia6_lifetime.ia6t_pltime;
> + time_uptime + ia6->ia6_lifetime.ia6t_pltime;
> } else
> ia6->ia6_lifetime.ia6t_preferred = 0;
>
> Index: netinet6/in6.h
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/in6.h,v
> retrieving revision 1.90
> diff -u -p -r1.90 in6.h
> --- netinet6/in6.h 27 Jun 2016 16:33:48 -0000 1.90
> +++ netinet6/in6.h 2 Oct 2016 17:18:24 -0000
> @@ -280,11 +280,11 @@ struct route_in6 {
>
> #define IFA6_IS_DEPRECATED(a) \
> ((a)->ia6_lifetime.ia6t_pltime != ND6_INFINITE_LIFETIME && \
> - (u_int32_t)((time_second - (a)->ia6_updatetime)) > \
> + (u_int32_t)((time_uptime - (a)->ia6_updatetime)) > \
> (a)->ia6_lifetime.ia6t_pltime)
> #define IFA6_IS_INVALID(a) \
> ((a)->ia6_lifetime.ia6t_vltime != ND6_INFINITE_LIFETIME && \
> - (u_int32_t)((time_second - (a)->ia6_updatetime)) > \
> + (u_int32_t)((time_uptime - (a)->ia6_updatetime)) > \
> (a)->ia6_lifetime.ia6t_vltime)
>
> #endif /* _KERNEL */
> Index: netinet6/ip6_forward.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_forward.c,v
> retrieving revision 1.92
> diff -u -p -r1.92 ip6_forward.c
> --- netinet6/ip6_forward.c 24 Aug 2016 09:41:12 -0000 1.92
> +++ netinet6/ip6_forward.c 2 Oct 2016 17:18:24 -0000
> @@ -104,8 +104,8 @@ ip6_forward(struct mbuf *m, struct rtent
> IN6_IS_ADDR_MULTICAST(&ip6->ip6_dst) ||
> IN6_IS_ADDR_UNSPECIFIED(&ip6->ip6_src)) {
> ip6stat.ip6s_cantforward++;
> - if (ip6_log_time + ip6_log_interval < time_second) {
> - ip6_log_time = time_second;
> + if (ip6_log_time + ip6_log_interval < time_uptime) {
> + ip6_log_time = time_uptime;
> inet_ntop(AF_INET6, &ip6->ip6_src, src6, sizeof(src6));
> inet_ntop(AF_INET6, &ip6->ip6_dst, dst6, sizeof(dst6));
> log(LOG_DEBUG,
> @@ -193,8 +193,8 @@ reroute:
> ip6stat.ip6s_cantforward++;
> ip6stat.ip6s_badscope++;
>
> - if (ip6_log_time + ip6_log_interval < time_second) {
> - ip6_log_time = time_second;
> + if (ip6_log_time + ip6_log_interval < time_uptime) {
> + ip6_log_time = time_uptime;
> inet_ntop(AF_INET6, &ip6->ip6_src, src6, sizeof(src6));
> inet_ntop(AF_INET6, &ip6->ip6_dst, dst6, sizeof(dst6));
> log(LOG_DEBUG,
> Index: netinet6/ip6_id.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_id.c,v
> retrieving revision 1.11
> diff -u -p -r1.11 ip6_id.c
> --- netinet6/ip6_id.c 4 Dec 2015 12:10:26 -0000 1.11
> +++ netinet6/ip6_id.c 2 Oct 2016 17:18:24 -0000
> @@ -197,7 +197,7 @@ ip6id_initid(struct randomtab *p)
> p->ru_g = ip6id_pmod(p->ru_gen, j, p->ru_n);
> p->ru_counter = 0;
>
> - p->ru_reseed = time_second + p->ru_out;
> + p->ru_reseed = time_uptime + p->ru_out;
> p->ru_msb = p->ru_msb ? 0 : (1U << (p->ru_bits - 1));
> }
>
> @@ -206,7 +206,7 @@ ip6id_randomid(struct randomtab *p)
> {
> int i, n;
>
> - if (p->ru_counter >= p->ru_max || time_second > p->ru_reseed)
> + if (p->ru_counter >= p->ru_max || time_uptime > p->ru_reseed)
> ip6id_initid(p);
>
> /* Skip a random number of ids */
> Index: netinet6/ip6_mroute.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/ip6_mroute.c,v
> retrieving revision 1.104
> diff -u -p -r1.104 ip6_mroute.c
> --- netinet6/ip6_mroute.c 23 Aug 2016 11:01:16 -0000 1.104
> +++ netinet6/ip6_mroute.c 2 Oct 2016 17:18:24 -0000
> @@ -980,10 +980,10 @@ ip6_mforward(struct ip6_hdr *ip6, struct
> */
> if (IN6_IS_ADDR_UNSPECIFIED(&ip6->ip6_src)) {
> ip6stat.ip6s_cantforward++;
> - if (ip6_log_time + ip6_log_interval < time_second) {
> + if (ip6_log_time + ip6_log_interval < time_uptime) {
> char src[INET6_ADDRSTRLEN], dst[INET6_ADDRSTRLEN];
>
> - ip6_log_time = time_second;
> + ip6_log_time = time_uptime;
>
> inet_ntop(AF_INET6, &ip6->ip6_src, src, sizeof(src));
> inet_ntop(AF_INET6, &ip6->ip6_dst, dst, sizeof(dst));
> Index: netinet6/nd6.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/nd6.c,v
> retrieving revision 1.192
> diff -u -p -r1.192 nd6.c
> --- netinet6/nd6.c 15 Sep 2016 02:00:18 -0000 1.192
> +++ netinet6/nd6.c 2 Oct 2016 17:18:24 -0000
> @@ -447,7 +447,7 @@ nd6_timer_work(void *null)
>
> /* expire default router list */
> TAILQ_FOREACH_SAFE(dr, &nd_defrouter, dr_entry, ndr)
> - if (dr->expire && dr->expire < time_second)
> + if (dr->expire && dr->expire < time_uptime)
> defrtrlist_del(dr);
>
> /*
> @@ -479,7 +479,7 @@ nd6_timer_work(void *null)
> * prefix is not necessary.
> */
> if (pr->ndpr_vltime != ND6_INFINITE_LIFETIME &&
> - time_second - pr->ndpr_lastupdate > pr->ndpr_vltime) {
> + time_uptime - pr->ndpr_lastupdate > pr->ndpr_vltime) {
> /*
> * address expiration and prefix expiration are
> * separate. NEVER perform in6_purgeaddr here.
> @@ -764,9 +764,9 @@ nd6_free(struct rtentry *rt, int gc)
> * XXX: the check for ln_state would be redundant,
> * but we intentionally keep it just in case.
> */
> - if (dr->expire > time_second) {
> + if (dr->expire > time_uptime) {
> nd6_llinfo_settimer(ln,
> - dr->expire - time_second);
> + dr->expire - time_uptime);
> } else
> nd6_llinfo_settimer(ln, nd6_gctimer);
> splx(s);
> @@ -1686,6 +1686,7 @@ fill_drlist(void *oldp, size_t *oldlenp,
> int error = 0, s;
> struct in6_defrouter *d = NULL, *de = NULL;
> struct nd_defrouter *dr;
> + time_t expire;
> size_t l;
>
> s = splsoftnet();
> @@ -1704,7 +1705,12 @@ fill_drlist(void *oldp, size_t *oldlenp,
> in6_recoverscope(&d->rtaddr, &dr->rtaddr);
> d->flags = dr->flags;
> d->rtlifetime = dr->rtlifetime;
> - d->expire = dr->expire;
> + expire = dr->expire;
> + if (expire != 0) {
> + expire -= time_uptime;
> + expire += time_second;
> + }
> + d->expire = expire;
> d->if_index = dr->ifp->if_index;
> }
>
> Index: netinet6/nd6_rtr.c
> ===================================================================
> RCS file: /cvs/src/sys/netinet6/nd6_rtr.c,v
> retrieving revision 1.147
> diff -u -p -r1.147 nd6_rtr.c
> --- netinet6/nd6_rtr.c 30 Sep 2016 06:27:21 -0000 1.147
> +++ netinet6/nd6_rtr.c 2 Oct 2016 17:18:24 -0000
> @@ -428,7 +428,7 @@ nd6_ra_input(struct mbuf *m, int off, in
> dr0.rtaddr = saddr6;
> dr0.flags = nd_ra->nd_ra_flags_reserved;
> dr0.rtlifetime = ntohs(nd_ra->nd_ra_router_lifetime);
> - dr0.expire = time_second + dr0.rtlifetime;
> + dr0.expire = time_uptime + dr0.rtlifetime;
> dr0.ifp = ifp;
> /* unspecified or not? (RFC 2461 6.3.4) */
> if (advreachable) {
> @@ -518,7 +518,7 @@ nd6_ra_input(struct mbuf *m, int off, in
> pr.ndpr_plen = pi->nd_opt_pi_prefix_len;
> pr.ndpr_vltime = ntohl(pi->nd_opt_pi_valid_time);
> pr.ndpr_pltime = ntohl(pi->nd_opt_pi_preferred_time);
> - pr.ndpr_lastupdate = time_second;
> + pr.ndpr_lastupdate = time_uptime;
>
> if (in6_init_prefix_ltimes(&pr))
> continue; /* prefix lifetime init failed */
> @@ -1350,7 +1350,7 @@ prelist_update(struct nd_prefix *new, st
>
> if (lt6_tmp.ia6t_vltime == ND6_INFINITE_LIFETIME)
> storedlifetime = ND6_INFINITE_LIFETIME;
> - else if (time_second - ia6->ia6_updatetime >
> + else if (time_uptime - ia6->ia6_updatetime >
> lt6_tmp.ia6t_vltime) {
> /*
> * The case of "invalid" address. We should usually
> @@ -1359,7 +1359,7 @@ prelist_update(struct nd_prefix *new, st
> storedlifetime = 0;
> } else
> storedlifetime = lt6_tmp.ia6t_vltime -
> - (time_second - ia6->ia6_updatetime);
> + (time_uptime - ia6->ia6_updatetime);
> if (TWOHOUR < new->ndpr_vltime ||
> storedlifetime < new->ndpr_vltime) {
> lt6_tmp.ia6t_vltime = new->ndpr_vltime;
> @@ -1390,7 +1390,7 @@ prelist_update(struct nd_prefix *new, st
> in6_init_address_ltimes(pr, <6_tmp);
>
> ia6->ia6_lifetime = lt6_tmp;
> - ia6->ia6_updatetime = time_second;
> + ia6->ia6_updatetime = time_uptime;
> }
>
> if ((!autoconf || ((ifp->if_xflags & IFXF_INET6_NOPRIVACY) == 0 &&
> @@ -1978,11 +1978,11 @@ in6_init_prefix_ltimes(struct nd_prefix
> if (ndpr->ndpr_pltime == ND6_INFINITE_LIFETIME)
> ndpr->ndpr_preferred = 0;
> else
> - ndpr->ndpr_preferred = time_second + ndpr->ndpr_pltime;
> + ndpr->ndpr_preferred = time_uptime + ndpr->ndpr_pltime;
> if (ndpr->ndpr_vltime == ND6_INFINITE_LIFETIME)
> ndpr->ndpr_expire = 0;
> else
> - ndpr->ndpr_expire = time_second + ndpr->ndpr_vltime;
> + ndpr->ndpr_expire = time_uptime + ndpr->ndpr_vltime;
>
> return 0;
> }
> @@ -1996,7 +1996,7 @@ in6_init_address_ltimes(struct nd_prefix
> if (lt6->ia6t_vltime == ND6_INFINITE_LIFETIME)
> lt6->ia6t_expire = 0;
> else {
> - lt6->ia6t_expire = time_second;
> + lt6->ia6t_expire = time_uptime;
> lt6->ia6t_expire += lt6->ia6t_vltime;
> }
>
> @@ -2004,7 +2004,7 @@ in6_init_address_ltimes(struct nd_prefix
> if (lt6->ia6t_pltime == ND6_INFINITE_LIFETIME)
> lt6->ia6t_preferred = 0;
> else {
> - lt6->ia6t_preferred = time_second;
> + lt6->ia6t_preferred = time_uptime;
> lt6->ia6t_preferred += lt6->ia6t_pltime;
> }
> }
>
