BGP: Fix the setting of link-local nexthops in some situations

This patch addresses three main issues:
a. Passing along the global IPv6 nexthop received from the EBGP peer to
IBGP peers but setting the link-local IPv6 nexthop to ourselves when
advertising EBGP-learnt routes to IBGP peers (in the absence of outbound
route-map or other overrides). The fix is to not send a link-local IPv6
nexthop in this case.

b. Passing along the link-local IPv6 nexthop received from one peer to
another peer which is (or may be) on a different subnet. This violates the
semantics of link-local IPv6 address. The fix is to set the nexthop to
ourselves in the situation where the nexthop normally has to be passed
but is a link-local IPv6 address.

c. Different behavior wrt nexthop advertisement for BGP unnumbered peering
if it is setup using link-local IPv6 address versus IPv4 /30 or /31. The
fix is to make the behavior consistent as long as the interface config is
the same in both cases.

Signed-off-by: Vivek Venkatraman <vivek@cumulusnetworks.com>
Reviewed-by: Daniel Walton <dwalton@cumulusnetworks.com>

Ticket: CM-7846, CM-8043
Reviewed By: CCR-3749
Testing Done: Manual testing, bgpsmoke (on 2.5-br)

Note: Imported from 2.5-br patch bgpd-fix-link-local-nexthop-setting.patch

2 files changed, 35 insertions, 10 deletions

diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index cf88b3ee8a..02b629e2e5 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -1336,7 +1336,7 @@ subgroup_announce_check (struct bgp_info *ri, struct update_subgroup *subgrp,
   if (peer->sort == BGP_PEER_EBGP
       && attr->flag & ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC))
     {
-      if (ri->peer != bgp->peer_self && ! transparent
+      if (from != bgp->peer_self && ! transparent
 	  && ! CHECK_FLAG (peer->af_flags[afi][safi], PEER_FLAG_MED_UNCHANGED))
 	attr->flag &= ~(ATTR_FLAG_BIT (BGP_ATTR_MULTI_EXIT_DISC));
     }
@@ -1360,6 +1360,7 @@ subgroup_announce_check (struct bgp_info *ri, struct update_subgroup *subgrp,
    * the peer (group) is configured to receive link-local nexthop unchanged
    * and it is available in the prefix OR we're not reflecting the route and
    * the peer (group) to whom we're going to announce is on a shared network
+   * and this is either a self-originated route or the peer is EBGP.
    */
   if (p->family == AF_INET6 || peer_cap_enhe(peer))
     {
@@ -1367,7 +1368,8 @@ subgroup_announce_check (struct bgp_info *ri, struct update_subgroup *subgrp,
       if ((CHECK_FLAG (peer->af_flags[afi][safi],
                        PEER_FLAG_NEXTHOP_LOCAL_UNCHANGED) &&
            IN6_IS_ADDR_LINKLOCAL (&attr->extra->mp_nexthop_local)) ||
-          (!reflect && peer->shared_network))
+          (!reflect && peer->shared_network &&
+           (from == bgp->peer_self || peer->sort == BGP_PEER_EBGP)))
         {
           attr->extra->mp_nexthop_len = BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL;
         }
@@ -1470,6 +1472,16 @@ subgroup_announce_check (struct bgp_info *ri, struct update_subgroup *subgrp,
           if (!paf)
             subgroup_announce_reset_nhop ((peer_cap_enhe(peer) ? AF_INET6 : p->family), attr);
         }
+      /* If IPv6/MP and nexthop does not have any override and happens to
+       * be a link-local address, reset it so that we don't pass along the
+       * source's link-local IPv6 address to recipients who may not be on
+       * the same interface.
+       */
+      if (p->family == AF_INET6 || peer_cap_enhe(peer))
+        {
+          if (IN6_IS_ADDR_LINKLOCAL (&attr->extra->mp_nexthop_global))
+            subgroup_announce_reset_nhop (AF_INET6, attr);
+        }
     }
 
   return 1;
diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c
index 410bc8564b..70d59a3e23 100644
--- a/bgpd/bgp_zebra.c
+++ b/bgpd/bgp_zebra.c
@@ -885,11 +885,24 @@ bgp_nexthop_set (union sockunion *local, union sockunion *remote,
       /* IPv6 nexthop*/
       ret = if_get_ipv6_global (ifp, &nexthop->v6_global);
 
-      /* There is no global nexthop. */
       if (!ret)
-	if_get_ipv6_local (ifp, &nexthop->v6_global);
+        {
+          /* There is no global nexthop. Use link-local address as both the
+           * global and link-local nexthop. In this scenario, the expectation
+           * for interop is that the network admin would use a route-map to
+           * specify the global IPv6 nexthop.
+           */
+          if_get_ipv6_local (ifp, &nexthop->v6_global);
+          memcpy (&nexthop->v6_local, &nexthop->v6_global,
+                  IPV6_MAX_BYTELEN);
+        }
       else
 	if_get_ipv6_local (ifp, &nexthop->v6_local);
+
+      if (if_lookup_by_ipv4 (&remote->sin.sin_addr))
+        peer->shared_network = 1;
+      else
+        peer->shared_network = 0;
 #endif /* HAVE_IPV6 */
     }
 
@@ -934,13 +947,13 @@ bgp_nexthop_set (union sockunion *local, union sockunion *remote,
           memcpy (&nexthop->v6_local, &local->sin6.sin6_addr,
                   IPV6_MAX_BYTELEN);
 	}
-    }
 
-  if (IN6_IS_ADDR_LINKLOCAL (&local->sin6.sin6_addr) ||
-      if_lookup_by_ipv6 (&remote->sin6.sin6_addr, remote->sin6.sin6_scope_id))
-    peer->shared_network = 1;
-  else
-    peer->shared_network = 0;
+      if (IN6_IS_ADDR_LINKLOCAL (&local->sin6.sin6_addr) ||
+          if_lookup_by_ipv6 (&remote->sin6.sin6_addr, remote->sin6.sin6_scope_id))
+        peer->shared_network = 1;
+      else
+        peer->shared_network = 0;
+    }
 
   /* KAME stack specific treatment.  */
 #ifdef KAME