Re: Make sure to insert ifa/ia after initialization

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

On Wed, Jun 29, 2016 at 5:26 PM, Roy Marples <roy%marples.name@localhost> wrote:
> On 29/06/2016 04:36, Ryota Ozaki wrote:
>> On Wed, Jun 29, 2016 at 3:22 AM, Roy Marples <roy%marples.name@localhost> wrote:
>>> On 2016-06-28 02:03, Ryota Ozaki wrote:
>>>>
>>>> Hi,
>>>>
>>>> Basically we should insert an item to a collection
>>>> (say a list) after completed item's initialization to
>>>> avoid accessing an item that is initialized halfway.
>>>>
>>>> ifaddr and in{,6}_ifaddr aren't processed like so.
>>>> The patch below fixes the issue.
>>>>
>>>>   http://www.netbsd.org/~ozaki-r/insert_ifa_after_init.diff
>>>>
>>>> To this end, I need to tweak {arp,nd6}_rtrequest that
>>>> depend on that an ifaddr (in{,6}_ifaddr) is inserted
>>>> during its initialization; they explore interface's
>>>> address list to determine that rt_getkey(rt) of a given
>>>> rtentry is in the list (i.e., whether the route's
>>>> interface should be a loopback), which doesn't work
>>>> after the change. To make it work I use RTF_LOCAL flag
>>>> instead.
>>>>
>>>> Any comments or suggestions?
>>>
>>>
>>> +               if (rt->rt_flags & RTF_LOCAL) {
>>> +                       rt->rt_expire = 0;
>>> +                       if (useloopback) {
>>> +                               rt->rt_ifp = lo0ifp;
>>> +                               rt->rt_rmx.rmx_mtu = 0;
>>> +                       }
>>>                 }
>>> -               rt->rt_flags |= RTF_LOCAL;
>>> -               /*
>>>
>>> This has reverted r1.181 which had this comment:
>>> If, for whatever reason, a local interface route is removed and then
>>> re-added, mark it as a local route.
>>
>> Thank you for pointing it out! I think we can restore the behavior
>> by restoring the removed code that should work for the case because
>> the address already exists in the list. (Updated the patch.)
>
> Looks good now!

:)

>
>>>
>>> You've tested manually removing and re-adding the local route and noticing
>>> the flag is preserved?
>>
>> I'm trying, but I don't know how to re-add. Could you tell me how
>> to do that?
>>
>>   kvm# ifconfig vioif0 10.0.0.1/24
>>   kvm# netstat -nr -f inet |grep 10.0.0.1
>>   10.0.0.1           link#1             UHLl        -        -      -  lo0
>>   kvm# route delete 10.0.0.1
>>   delete host 10.0.0.1
>>   kvm# netstat -nr -f inet |grep 10.0.0.1
>>   kvm# route add -host 10.0.0.1 -iface vioif0
>>   route: vioif0: bad value
>>   kvm# route add -host 10.0.0.1 -iface lo0
>>   route: lo0: bad value
>
> route add -host 10.0.0.1 -link vioif0 -ifp lo0
>
> On my -current (a few days old and without your patch) it claims the
> route is added, but no route actually appears so it's hard for me to
> verify this, but I'm pretty sure the command is correct.

Thanks. It seems that an entry is created as an ARP cache. I guess
something broken due to changes of nexthop cache separation
for backcompat. I'll investigate the regression.

  ozaki-r