From 003c1ba05ab9f3e7e87bd20aa7ced12cadb7298a Mon Sep 17 00:00:00 2001 From: vivek Date: Sun, 15 Nov 2015 07:17:47 -0800 Subject: [PATCH] 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 Reviewed-by: Daniel Walton 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 --- bgpd/bgp_route.c | 16 ++++++++++++++-- bgpd/bgp_zebra.c | 29 +++++++++++++++++++++-------- 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