Hi Martin,

the tests were successful.
Your fix works.

Thanks for your efforts!

Florian


On 11/27/14 09:55, Florian Riehm wrote:
> Hi Martin,
> 
> thank you for clarification and thank you for your patch.
> 
> Your patch looks reasonably to me. I forgot RTAX_IFP and RTAX_IFA in my patch.
> After a first trial, the fix works for me.
> Today I will start nightly regression tests with it.
> You will get the result tomorrow.
> 
> Regards
> 
> Florian
> 
> On 11/26/14 12:58, Martin Pieuchot wrote:
>> Hello Florian,
>>
>> On 26/11/14(Wed) 06:56, Florian Riehm wrote:
>>> since OpenBSD 5.6 route change messages can change the interface of a route
>>> (rt_ifa) even if a message doesn't seem to require it because of a changed
>>> gateway or stuff like that.
>>> I would like to ask if it's a regression or if the new behavior is intended.
>>
>> Since the behavior is different and it's not documented, it is a 
>> regression, thanks for reporting it.  I've just committed some new
>> tests in order to prevent this from happening again.
>>
>>> Example: (only for testing - it doesn't represent my network topology)
>>> ifconfig em0 inet6 fd88::1/64
>>> ifconfig em1 inet6 fd99::1/64
>>> route add -inet6 fd88::666 fd99::1
>>> route get fd88::666
>>>     interface: em1 (as expected)
>>> route change fd88::666 -mtu 1500
>>> route get fd88::666
>>>     interface: em0 (broken - trying to ping the target results in "No route 
>>> to
>>> host")
>>>
>>> In the example I can workaround the problem with adding a gateway while 
>>> changing
>>> the mtu:
>>> route change fd88::666 fd99::1 -mtu 1500
>>>
>>> A comment in route_output (rtsock.c) says
>>> /*
>>> * new gateway could require new ifaddr, ifp;
>>> * flags may also be different; ifp may be specified
>>> * by ll sockaddr when protocol address is ambiguous
>>> */
>>> but their is no check for a 'new gateway'.
>>
>> You're right.  What's happening here is that we always call rt_geifa() in
>> the first place.  This is a nasty function that tries to find an ifaddr
>> to attach a route.  But if the gateway is not new or you didn't specify
>> any of the "-ifp" or "-ifa" argument we should not look for a different
>> ifaddr.
>>
>> Here's a slightly different diff, could you tell me if it fixes the
>> regression in your case?
>>
>> Index: net/rtsock.c
>> ===================================================================
>> RCS file: /home/ncvs/src/sys/net/rtsock.c,v
>> retrieving revision 1.152
>> diff -u -p -r1.152 rtsock.c
>> --- net/rtsock.c     12 Aug 2014 13:52:08 -0000      1.152
>> +++ net/rtsock.c     26 Nov 2014 11:55:25 -0000
>> @@ -740,13 +740,6 @@ report:
>>                      break;
>>  
>>              case RTM_CHANGE:
>> -                    /*
>> -                     * new gateway could require new ifaddr, ifp;
>> -                     * flags may also be different; ifp may be specified
>> -                     * by ll sockaddr when protocol address is ambiguous
>> -                     */
>> -                    if ((error = rt_getifa(&info, tableid)) != 0)
>> -                            goto flush;
>>                      newgate = 0;
>>                      if (info.rti_info[RTAX_GATEWAY] != NULL)
>>                              if (rt->rt_gateway == NULL ||
>> @@ -761,7 +754,17 @@ report:
>>                              error = EDQUOT;
>>                              goto flush;
>>                      }
>> -                    ifa = info.rti_ifa;
>> +                    /*
>> +                     * new gateway could require new ifaddr, ifp;
>> +                     * flags may also be different; ifp may be specified
>> +                     * by ll sockaddr when protocol address is ambiguous
>> +                     */
>> +                    if (newgate || info.rti_info[RTAX_IFP] != NULL ||
>> +                        info.rti_info[RTAX_IFA] != NULL) {
>> +                            if ((error = rt_getifa(&info, tableid)) != 0)
>> +                                    goto flush;
>> +                            ifa = info.rti_ifa;
>> +                    }
>>                      if (ifa) {
>>                              if (rt->rt_ifa != ifa) {
>>                                      if (rt->rt_ifa->ifa_rtrequest)
>>
> 

Reply via email to