On Mon, Aug 07, 2017 at 03:51:00PM +0200, Alexander Bluhm wrote:
> On Mon, Aug 07, 2017 at 08:07:33AM +0000, Florian Obser wrote:
> > index cafdd9fe36f..7796af6191c 100644
> > --- sys/netinet6/in6.c
> > +++ sys/netinet6/in6.c
> > @@ -686,6 +686,10 @@ in6_update_ifa(struct ifnet *ifp, struct in6_aliasreq
> > *ifra,
> > */
> > ia6->ia6_flags = ifra->ifra_flags;
> >
> > + KERNEL_LOCK();
> > + nd6_expire_timer_update(ia6);
> > + KERNEL_UNLOCK();
> > +
>
> Can we ever reach this code without holding the kernel lock?
sppp_update_ip6_addr() seems to hold NET_LOCK but not the kernel lock
>
> > + if (!timeout_pending(&nd6_expire_timeout) || nd6_expire_time >
> > + expire_time) {
> > + expire_time++; /* fire one second after expiry */
>
> Why do we need this and why do we do it after the
> "nd6_expire_time > expire_time" check?
expire_time contains the uptime seconds when pltime or vltime reach 0.
IFA6_IS_INVALID and IFA6_IS_DEPRECATED check for '>', not for '>='.
That means without the ++ the timer would fire exactly at the second
when pltime or vltime reach zero. But the address is not yet invalid
or deprecated. We would then schedule a timeout in 0 seconds. Which is
according to the man page actually 1 second and then the address is
invalid or deprecated.
The ++ ensures that the timeout fires after the address is considered
deprecated / invalid.
>
> > /* check address lifetime */
> > if (IFA6_IS_INVALID(ia6)) {
> > in6_purgeaddr(&ia6->ia_ifa);
> > - } else if (IFA6_IS_DEPRECATED(ia6)) {
> > - ia6->ia6_flags |= IN6_IFF_DEPRECATED;
> > } else {
> > - /*
> > - * A new RA might have made a deprecated address
> > - * preferred.
> > - */
> > - ia6->ia6_flags &= ~IN6_IFF_DEPRECATED;
> > + if (IFA6_IS_DEPRECATED(ia6))
> > + ia6->ia6_flags |= IN6_IFF_DEPRECATED;
> > + nd6_expire_timer_update(ia6);
>
> Why is "ia6_flags &= ~IN6_IFF_DEPRECATED" removed?
previously the RA processing in the kernel fiddled with the lifetimes
but left the flags alone. Then the timer would fire every second and
fix up the flags.
With this diff the timer fires when an address becomes deprecated or
invalid. It does not when an address becomes preferred again. So it makes no
sense to clear the flag.
> And where is the IN6_IFF_DEPRECATED flag cleared now?
In in6_update_ifa() which is called from the ioctl, the only path where
we are setting new pltimes / vltimes.
/*
* configure address flags.
*/
ia6->ia6_flags = ifra->ifra_flags;
ifra->ifra_flags never has IN6_IFF_DEPRECATED set
>
> bluhm
>
--
I'm not entirely sure you are real.
