On Fri, Apr 6, 2018 at 8:02 PM Roy Marples <r...@marples.name> wrote:
> On 06/04/2018 11:59, Ryota Ozaki wrote: > > On Fri, Apr 6, 2018 at 7:45 PM, Ryota Ozaki <ozak...@netbsd.org> wrote: > >> On Fri, Apr 6, 2018 at 7:04 PM, Roy Marples <r...@marples.name> 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