Martin -
Thanks for this work.
I'll be applying the patch here in a few minutes.
donald
On Wed, Feb 24, 2016 at 6:02 AM, Martin Winter <
mwin...@opensourcerouting.org> wrote:
> On 22 Feb 2016, at 6:40, Donald Sharp wrote:
>
>> Martin -
>>
>> Any word on this? I'd like to push this patch if it fixes your issues.
>>
>
> Ok, finally more answers.
> It seems something went wrong when I retested with Timo’s fix. (Might be
> related to the way I retested as I was unable to do a clean rebuild and
> applied patch [probably wrong] on top of the last commit. Rebase made it
> impossible to pull a new copy as I usually do for each run)
>
> Anyway, re-running the tests and Timo’s patch (same fixed version as
> mailed earlier) fixes at least all BGP IPv4 bug difference between
> Linux and FreeBSD.
>
> So I suggest to go ahead an apply that patch on top of proposed/6 branch
>
> Full results (including all other protocols) of the rerun will be available
> in approx another 12..18hrs.
>
> - Martin
>
>
>
> On Thu, Feb 18, 2016 at 10:30 PM, Martin Winter <
>> mwin...@opensourcerouting.org> wrote:
>>
>> On 18 Feb 2016, at 18:35, Donald Sharp wrote:
>>>
>>> Martin -
>>>>
>>>> rt_socket.c is not compiled on linux so no need for verification. Once
>>>>
>>> you
>>>
>>>> have a run please let me know and I'll push your updated patch.
>>>>
>>>
>>> It’s running now. Will know soon. It definitely fixes some of the errors
>>> and I hope it actually fixes all of the FreeBSD errors.
>>>
>>> Will know in approx one more day.
>>>
>>> (BTW: Testing on a git commit which does no longer exist because of
>>> rebase,
>>> but I needed the same to compare. Lucky that I still had the VMs with the
>>> old git checkout running…)
>>>
>>> - Martin
>>>
>>>
>>>> donald
>>>>
>>>> On Thu, Feb 18, 2016 at 9:19 PM, Martin Winter <
>>>> mwin...@opensourcerouting.org> wrote:
>>>>
>>>> Timo,
>>>>>
>>>>> On 17 Feb 2016, at 23:01, Timo Teras wrote:
>>>>>
>>>>> On Wed, 17 Feb 2016 20:11:07 -0800
>>>>>
>>>>>> "Martin Winter" <mwin...@opensourcerouting.org> wrote:
>>>>>>
>>>>>> This is based on Quagga proposed 6 branch of Feb 10 (git f48d5b9957 -
>>>>>>
>>>>>>> which does no longer exist, [no] thanks to rebase on a public
>>>>>>> git) on FreeBSD 10.2.
>>>>>>>
>>>>>>> Zebra seems to fail delete any routes in FreeBSD RIB. It fails with
>>>>>>> updates (better routes to different interface) and
>>>>>>> it fails to remove them when quagga is killed.
>>>>>>>
>>>>>>>
>>>>>> Thanks for the testing. Sounds like this is breakage caused by my
>>>>>> atomic FIB patch which was pretty untested on *BSD.
>>>>>>
>>>>>> Looks like the code talking to kernel does not handle RTM_CHANGE
>>>>>> correctly. As first test, perhaps just removing RTM_CHANGE usage might
>>>>>> fix the issues and revert to how it used to work.
>>>>>>
>>>>>> Using RTM_CHANGE on kernels where it works is better, but it's left an
>>>>>> exercise for developer who has access and will to fix it on *BSD.
>>>>>>
>>>>>>
>>>>> Thanks for the patch. Seems like you never tested it (failed to
>>>>>
>>>> compile),
>>>
>>>> but was close enough to guess what you probably meant. See inline on
>>>>>
>>>> patch
>>>
>>>>
>>>>> diff --git a/zebra/rt_socket.c b/zebra/rt_socket.c
>>>>>
>>>>>> index 4d0a7db..9012280 100644
>>>>>> --- a/zebra/rt_socket.c
>>>>>> +++ b/zebra/rt_socket.c
>>>>>> @@ -68,7 +68,7 @@ sin_masklen (struct in_addr mask)
>>>>>>
>>>>>> /* Interface between zebra message and rtm message. */
>>>>>> static int
>>>>>> -kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib, int
>>>>>>
>>>>> family)
>>>
>>>> +kernel_rtm_ipv4 (int cmd, struct prefix *p, struct rib *rib)
>>>>>>
>>>>>> {
>>>>>> struct sockaddr_in *mask = NULL;
>>>>>> @@ -245,7 +245,7 @@ sin6_masklen (struct in6_addr mask)
>>>>>>
>>>>>> /* Interface between zebra message and rtm message. */
>>>>>> static int
>>>>>> -kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib, int
>>>>>>
>>>>> family)
>>>
>>>> +kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib)
>>>>>> {
>>>>>> struct sockaddr_in6 *mask;
>>>>>> struct sockaddr_in6 sin_dest, sin_mask, sin_gate;
>>>>>> @@ -363,33 +363,32 @@ kernel_rtm_ipv6 (int cmd, struct prefix *p,
>>>>>>
>>>>> struct
>>>
>>>> rib *rib, int family)
>>>>>>
>>>>>> #endif
>>>>>>
>>>>>> +static int
>>>>>> +kernel_rtm_ipv6 (int cmd, struct prefix *p, struct rib *rib)
>>>>>>
>>>>>>
>>>>> I assume this should be
>>>>> kernel_rtm - not kernel_rtm_ipv6
>>>>> (otherwise you have a duplicate for kernel_rtm_ipv6() and a loop
>>>>> in case of AF_INET6)
>>>>>
>>>>>
>>>>> +{
>>>>>
>>>>>> + switch (PREFIX_FAMILY(p))
>>>>>> + {
>>>>>> + case AF_INET:
>>>>>> + return kernel_rtm_ipv4 (cmd, p, rib);
>>>>>> + case AF_INET6:
>>>>>> + return kernel_rtm_ipv6 (cmd, p, rib);
>>>>>> + }
>>>>>> + return 0;
>>>>>> +}
>>>>>> +
>>>>>> int
>>>>>> kernel_route_rib (struct prefix *p, struct rib *old, struct rib *new)
>>>>>> {
>>>>>> - struct rib *rib;
>>>>>> - int route = 0, cmd;
>>>>>> -
>>>>>> - if (!old && new)
>>>>>> - cmd = RTM_ADD;
>>>>>> - else if (old && !new)
>>>>>> - cmd = RTM_DELETE;
>>>>>> - else
>>>>>> - cmd = RTM_CHANGE;
>>>>>> -
>>>>>> - rib = new ? new : old;
>>>>>> + int route = 0;
>>>>>>
>>>>>> if (zserv_privs.change(ZPRIVS_RAISE))
>>>>>> zlog (NULL, LOG_ERR, "Can't raise privileges");
>>>>>>
>>>>>> - switch (PREFIX_FAMILY(p))
>>>>>> - {
>>>>>> - case AF_INET:
>>>>>> - route = kernel_rtm_ipv4 (cmd, p, rib, AF_INET);
>>>>>> - break;
>>>>>> - case AF_INET6:
>>>>>> - route = kernel_rtm_ipv6 (cmd, p, rib, AF_INET6);
>>>>>> - break;
>>>>>> - }
>>>>>> + if (old)
>>>>>> + route |= kernel_rtm (RTM_DELETE, p, rib);
>>>>>>
>>>>>>
>>>>> Changed to
>>>>> route |= kernel_rtm (RTM_DELETE, p, old);
>>>>>
>>>>> +
>>>>>
>>>>>> + if (new)
>>>>>> + route |= kernel_rtm (RTM_ADD, p, rib);
>>>>>>
>>>>>>
>>>>> and changed to
>>>>> route |= kernel_rtm (RTM_ADD, p, new);
>>>>>
>>>>> (You removed the declaration of “rib” above - so rib is undefined)
>>>>>
>>>>>
>>>>> if (zserv_privs.change(ZPRIVS_LOWER))
>>>>>> zlog (NULL, LOG_ERR, "Can't lower privileges");
>>>>>>
>>>>>>
>>>>> Attached is a updated patch
>>>>>
>>>>> With the changes it fixes the specific issue I’ve mentioned. I have not
>>>>> verified the
>>>>> patch on Linux yet. Will do a full run with this patch to see how many
>>>>>
>>>> of
>>>
>>>> my approx
>>>>> 20..30 failing FreeBSD testcases it fixes (I assume many to all…)
>>>>>
>>>>> - Martin
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Quagga-dev mailing list
>>>>> Quagga-dev@lists.quagga.net
>>>>> https://lists.quagga.net/mailman/listinfo/quagga-dev
>>>>>
>>>>>
>>>
_______________________________________________
Quagga-dev mailing list
Quagga-dev@lists.quagga.net
https://lists.quagga.net/mailman/listinfo/quagga-dev
