Acked-by: Donald Sharp <[email protected]>

Cumulus has this exact patch pretty-much in two patches:

https://github.com/CumulusNetworks/quagga/commit/76145957957916bbb4d0ca33018084b5203528a5
https://github.com/CumulusNetworks/quagga/commit/d17ec8df733bdc9a9294c82d6e53c0daaffd0c59

The first happens to be exposed on take-X, but the second has not been
updated there yet.

donald

On Wed, Nov 18, 2015 at 11:00 AM, Paul Jakma <[email protected]> wrote:

> From: Paul Jakma <[email protected]>
>
> *  J Yu <[email protected]> noted a problem with bgpd of routes not
>    having their nexthop updated correctly.
>
>    Martin Winter <[email protected]> pinned this down to the
>    case where a BGP route is updated from one with a valid nexthop to an
>    invalid next-hop, using a test tool. Once the problem occurs, the
> incorrect
>    route may remain, even after further UPDATEs, so long as the nexthop in
> the
>    zebra RIB does not match the BGP route's nexthop.
>
>    Jacqueline Yu then pinned the issue down further to being due to bgpd
>    sending the DELETE for the route to zebra with the new nexthop after a
>    BGP UPDATE updates an existing route, but then is found to be invalid,
>    and zebra not finding the route as it requires a match on all
> attributes.
>
> * bgp_zebra.c: (bgp_zebra_withdraw) When deleting a prefix, we want it
> gone.
>   Do not send additional matching attributes like the nexthop, which can
>   only cause incorrect non-matches.
> ---
>  bgpd/bgp_zebra.c | 46 +++++-----------------------------------------
>  1 file changed, 5 insertions(+), 41 deletions(-)
>
> diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
> index 4b97353..83b0902 100644
> --- a/bgpd/bgp_zebra.c
> +++ b/bgpd/bgp_zebra.c
> @@ -880,18 +880,14 @@ bgp_zebra_withdraw (struct prefix *p, struct
> bgp_info *info, safi_t safi)
>    if (p->family == AF_INET)
>      {
>        struct zapi_ipv4 api;
> -      struct in_addr *nexthop;
>
>        api.vrf_id = VRF_DEFAULT;
>        api.flags = flags;
> -      nexthop = &info->attr->nexthop;
>
>        api.type = ZEBRA_ROUTE_BGP;
>        api.message = 0;
>        api.safi = safi;
> -      SET_FLAG (api.message, ZAPI_MESSAGE_NEXTHOP);
> -      api.nexthop_num = 1;
> -      api.nexthop = &nexthop;
> +      api.nexthop_num = 0;
>        api.ifindex_num = 0;
>        SET_FLAG (api.message, ZAPI_MESSAGE_METRIC);
>        api.metric = info->attr->med;
> @@ -899,10 +895,9 @@ bgp_zebra_withdraw (struct prefix *p, struct bgp_info
> *info, safi_t safi)
>        if (BGP_DEBUG(zebra, ZEBRA))
>         {
>           char buf[2][INET_ADDRSTRLEN];
> -         zlog_debug("Zebra send: IPv4 route delete %s/%d nexthop %s
> metric %u",
> +         zlog_debug("Zebra send: IPv4 route delete %s/%d metric %u",
>                      inet_ntop(AF_INET, &p->u.prefix4, buf[0],
> sizeof(buf[0])),
>                      p->prefixlen,
> -                    inet_ntop(AF_INET, nexthop, buf[1], sizeof(buf[1])),
>                      api.metric);
>         }
>
> @@ -914,54 +909,23 @@ bgp_zebra_withdraw (struct prefix *p, struct
> bgp_info *info, safi_t safi)
>    if (p->family == AF_INET6)
>      {
>        struct zapi_ipv6 api;
> -      unsigned int ifindex;
> -      struct in6_addr *nexthop;
> -
> -      assert (info->attr->extra);
>
> -      ifindex = 0;
> -      nexthop = NULL;
> -
> -      /* Only global address nexthop exists. */
> -      if (info->attr->extra->mp_nexthop_len == 16)
> -       nexthop = &info->attr->extra->mp_nexthop_global;
> -
> -      /* If both global and link-local address present. */
> -      if (info->attr->extra->mp_nexthop_len == 32)
> -       {
> -         nexthop = &info->attr->extra->mp_nexthop_local;
> -         if (info->peer->nexthop.ifp)
> -           ifindex = info->peer->nexthop.ifp->ifindex;
> -       }
> -
> -      if (nexthop == NULL)
> -       return;
> -
> -      if (IN6_IS_ADDR_LINKLOCAL (nexthop) && ! ifindex)
> -       if (info->peer->ifname)
> -         ifindex = ifname2ifindex (info->peer->ifname);
> -
>        api.vrf_id = VRF_DEFAULT;
>        api.flags = flags;
>        api.type = ZEBRA_ROUTE_BGP;
>        api.message = 0;
>        api.safi = safi;
> -      SET_FLAG (api.message, ZAPI_MESSAGE_NEXTHOP);
> -      api.nexthop_num = 1;
> -      api.nexthop = &nexthop;
> -      SET_FLAG (api.message, ZAPI_MESSAGE_IFINDEX);
> -      api.ifindex_num = 1;
> -      api.ifindex = &ifindex;
> +      api.nexthop_num = 0;
> +      api.ifindex_num = 0;
>        SET_FLAG (api.message, ZAPI_MESSAGE_METRIC);
>        api.metric = info->attr->med;
>
>        if (BGP_DEBUG(zebra, ZEBRA))
>         {
>           char buf[2][INET6_ADDRSTRLEN];
> -         zlog_debug("Zebra send: IPv6 route delete %s/%d nexthop %s
> metric %u",
> +         zlog_debug("Zebra send: IPv6 route delete %s/%d metric %u",
>                      inet_ntop(AF_INET6, &p->u.prefix6, buf[0],
> sizeof(buf[0])),
>                      p->prefixlen,
> -                    inet_ntop(AF_INET6, nexthop, buf[1], sizeof(buf[1])),
>                      api.metric);
>         }
>
> --
> 2.1.0
>
>
> _______________________________________________
> 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

Reply via email to