[quagga-dev 14702] Re: FreeBSD Bug: Zebra not deleting routes

Martin Winter mwinter at opensourcerouting.org
Fri Feb 19 02:19:54 GMT 2016


Timo,

On 17 Feb 2016, at 23:01, Timo Teras wrote:

> On Wed, 17 Feb 2016 20:11:07 -0800
> "Martin Winter" <mwinter at 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

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: freebsd_timo_fixed.patch
URL: <http://lists.quagga.net/pipermail/quagga-dev/attachments/20160218/230ce62b/attachment-0001.ksh>


More information about the Quagga-dev mailing list