Re: CVS commit: src/sys/netinet

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Fri, Apr 6, 2018 at 8:02 PM Roy Marples <roy%marples.name@localhost> wrote:

> 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.

Thanks. I fixed as you suggested.

   ozaki-r