Vincent/Nicholas - Can we get some help tracking down and fixing this issue?
thanks! donald On Mon, Feb 22, 2016 at 10:26 PM, Martin Winter < [email protected]> 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. >> > > Not yet. > I’m dealing with (most likely) several issues which seem overlap. So it > takes a bit > more time. > I still tend to think that Timo’s fix is good - but only addressing part > of the problems. > I’ll prefer to keep it on hold at this time until at least one more > FreeBSD bug is fixed. > > > One of the git bisect on BGP errors pointed to commit > > commit 04a3aabf58d95d01c4c8168eeff43cf9d9892eee > Author: Nicolas Dichtel <[email protected]> > Date: Thu Sep 3 10:47:43 2015 +0200 > > vrf: add a runtime check before playing with netns > > This patch adds a runtime check to determine if netns are > available. Some > systems like OpenWRT have the system call setns() but don't > have the kernel > option CONFIG_NET_NS enabled. > > Reported-by: Christian Franke <[email protected]> > Signed-off-by: Nicolas Dichtel <[email protected]> > Tested-by: Christian Franke <[email protected]> > > > Doing a git bisect points to this commit breaking at least 3 BGP tests. > I’m still trying to manually verify and understand how this breaks it, so > give me > more time. > > (Feel free to guess on how this commit could break FreeBSD… seems to be > related > to similar not correctly installing forwarding entries as well) > > - Martin > > > > On Thu, Feb 18, 2016 at 10:30 PM, Martin Winter < >> [email protected]> 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 < >>>> [email protected]> wrote: >>>> >>>> Timo, >>>>> >>>>> On 17 Feb 2016, at 23:01, Timo Teras wrote: >>>>> >>>>> On Wed, 17 Feb 2016 20:11:07 -0800 >>>>> >>>>>> "Martin Winter" <[email protected]> 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 >>>>> [email protected] >>>>> https://lists.quagga.net/mailman/listinfo/quagga-dev >>>>> >>>>> >>>
_______________________________________________ Quagga-dev mailing list [email protected] https://lists.quagga.net/mailman/listinfo/quagga-dev
