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
