Re: Make sure to insert ifa/ia after initialization

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

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.

Roy