Re: CVS commit: src/sys/netinet

[Roy Marples <roy%marples.name@localhost>]

On 06/04/2018 11:59, Ryota Ozaki wrote:
> On Fri, Apr 6, 2018 at 7:45 PM, Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
> > On Fri, Apr 6, 2018 at 7:04 PM, Roy Marples <roy%marples.name@localhost> wrote:
> > > On 06/04/2018 10:19, Ryota Ozaki wrote:
> > > > Module Name:    src
> > > > Committed By:   ozaki-r
> > > > Date:           Fri Apr  6 09:19:16 UTC 2018
> > > >
> > > > Modified Files:
> > > >          src/sys/netinet: in.c
> > > >
> > > > Log Message:
> > > > Simplify; clear then set flags to ia4_flags (NFCI)
> > >
> > >
> > > This change is not right.
> > > You are clearing the flags for an already existing address and now allows
> > > this:
> > >
> > > ifconfig bge0 1.2.3.4/16
> > > address becomes tentative
> > > ifconfig bge0 1.2.3.4/16
> > > address flags cleared, address can be used before DaD finishes.
> > >
> > > Can we match it to the inet6 path and at least remember if existing flags
> > > were TENTATIVE | DETACHED? That is only done there because we can update
> > > flags from userland, we have no mechanism for this in inet hence the slight
> > > difference.
> > >
> > > I would also be happier with always setting DETACHED on link down, but only
> > > setting TENTATIVE if ip_dad_count > 0.
> > > Would that help solve your issue with GARP? What is GARP anyway?
> >
> > Not enough. GARP wasn't sent because of the check in arpannounce:
> >
> >          if (ia->ia4_flags & (IN_IFF_NOTREADY | IN_IFF_DETACHED)) {
> >                  ARPLOG(LOG_DEBUG, "%s not ready\n", ARPLOGADDR(ip));
> >                  return;
> >          }
> >          arprequest(ifp, ip, ip, enaddr);
> >
> > So my change was to not set any flags if !(ip_dad_count > 0).
> > Do you have another idea to avoid the situation?
>
> Oh, I misread. Did you suggest a change like this?
>
>          if (ifp->if_link_state == LINK_STATE_DOWN) {
>                  ia->ia4_flags |= IN_IFF_DETACHED;
>                  ia->ia4_flags &= ~IN_IFF_TENTATIVE;
> -       } else if (hostIsNew && if_do_dad(ifp))
> +       } else if (hostIsNew && if_do_dad(ifp) && ip_dad_count > 0)
>                  ia->ia4_flags |= IN_IFF_TRYTENTATIVE;
>
> If so, it solves the issue.

If that is against the code prior to your commits, then yes.
Please make a similar change in sys/netinet6/in6.c so it's consistent 
there as well.

Roy