Re: CVS commit: src/sys/netinet

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

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.

Thanks,
  ozaki-r