From 8b93b03839681a327900b26c2ee184420cccb7f3 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Fri, 19 May 2017 10:51:00 -0300 Subject: [PATCH] bgpd: fix sending of invalid nexthops on the wire The bpacket_reformat_for_peer() function rewrites the nexthop of outgoing route updates on a per-peer basis in order to handle route-maps ("set ip next-hop") and locally-originated routes missing a nexthop. In the latter case, RFC 4271 says the following: "When announcing a locally-originated route to an internal peer, the BGP speaker SHOULD use the interface address of the router through which the announced network is reachable for the speaker as the NEXT_HOP". We were doing this for regular IPv4/IPv6 routes, but not for VPN/EVPN/ENCAP routes, which were being announced with invalid nexthops (0.0.0.0 or ::). This patch fixes this problem. Signed-off-by: Renato Westphal --- bgpd/bgp_attr.c | 3 +-- bgpd/bgp_updgrp_packet.c | 56 +++++++++++++++++++++++++++++++++------- 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 5fe8ebb67f..4d5f7cab5d 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -2742,6 +2742,7 @@ bgp_packet_mpattr_start (struct stream *s, afi_t afi, safi_t safi, afi_t nh_afi, else if (nh_afi == AFI_MAX) nh_afi = BGP_NEXTHOP_AFI_FROM_NHLEN(attr->extra->mp_nexthop_len); /* Nexthop */ + bpacket_attr_vec_arr_set_vec (vecarr, BGP_ATTR_VEC_NH, s, attr); switch (nh_afi) { case AFI_IP: @@ -2754,7 +2755,6 @@ bgp_packet_mpattr_start (struct stream *s, afi_t afi, safi_t safi, afi_t nh_afi, stream_put_ipv4 (s, attr->nexthop.s_addr); break; case SAFI_MPLS_VPN: - bpacket_attr_vec_arr_set_vec (vecarr, BGP_ATTR_VEC_NH, s, attr); stream_putc (s, 12); stream_putl (s, 0); /* RD = 0, per RFC */ stream_putl (s, 0); @@ -2777,7 +2777,6 @@ bgp_packet_mpattr_start (struct stream *s, afi_t afi, safi_t safi, afi_t nh_afi, struct attr_extra *attre = attr->extra; assert (attr->extra); - bpacket_attr_vec_arr_set_vec (vecarr, BGP_ATTR_VEC_NH, s, attr); stream_putc (s, attre->mp_nexthop_len); stream_put (s, &attre->mp_nexthop_global, IPV6_MAX_BYTELEN); if (attre->mp_nexthop_len == BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL) diff --git a/bgpd/bgp_updgrp_packet.c b/bgpd/bgp_updgrp_packet.c index 8839de391e..5872ca6f3f 100644 --- a/bgpd/bgp_updgrp_packet.c +++ b/bgpd/bgp_updgrp_packet.c @@ -427,21 +427,33 @@ bpacket_reformat_for_peer (struct bpacket *pkt, struct peer_af *paf) nhafi = BGP_NEXTHOP_AFI_FROM_NHLEN(nhlen); if (peer_cap_enhe(peer)) nhafi = AFI_IP6; - if (paf->safi == SAFI_MPLS_VPN && /* if VPN && not global */ - nhlen != BGP_ATTR_NHLEN_VPNV6_GLOBAL_AND_LL) - nhafi = AFI_MAX; /* no change allowed */ } if (nhafi == AFI_IP) { struct in_addr v4nh, *mod_v4nh; int nh_modified = 0; + size_t offset_nh = vec->offset + 1; route_map_sets_nh = (CHECK_FLAG (vec->flags, BPKT_ATTRVEC_FLAGS_RMAP_IPV4_NH_CHANGED) || CHECK_FLAG (vec->flags, BPKT_ATTRVEC_FLAGS_RMAP_NH_PEER_ADDRESS)); - stream_get_from (&v4nh, s, vec->offset + 1, 4); + switch (nhlen) + { + case BGP_ATTR_NHLEN_IPV4: + break; + case BGP_ATTR_NHLEN_VPNV4: + offset_nh += 8; + break; + default: + /* TODO: handle IPv6 nexthops */ + zlog_warn ("%s: %s: invalid MP nexthop length (AFI IP): %u", + __func__, peer->host, nhlen); + return NULL; + } + + stream_get_from (&v4nh, s, offset_nh, IPV4_MAX_BYTELEN); mod_v4nh = &v4nh; /* @@ -482,7 +494,7 @@ bpacket_reformat_for_peer (struct bpacket *pkt, struct peer_af *paf) } if (nh_modified) /* allow for VPN RD */ - stream_put_in_addr_at (s, vec->offset + 1 + nhlen - 4, mod_v4nh); + stream_put_in_addr_at (s, offset_nh, mod_v4nh); if (bgp_debug_update(peer, NULL, NULL, 0)) zlog_debug ("u%" PRIu64 ":s%" PRIu64 " %s send UPDATE w/ nexthop %s%s", @@ -495,6 +507,8 @@ bpacket_reformat_for_peer (struct bpacket *pkt, struct peer_af *paf) struct in6_addr v6nhglobal, *mod_v6nhg; struct in6_addr v6nhlocal, *mod_v6nhl; int gnh_modified, lnh_modified; + size_t offset_nhglobal = vec->offset + 1; + size_t offset_nhlocal = vec->offset + 1; gnh_modified = lnh_modified = 0; mod_v6nhg = &v6nhglobal; @@ -509,7 +523,28 @@ bpacket_reformat_for_peer (struct bpacket *pkt, struct peer_af *paf) * additional work being to handle 1 or 2 nexthops. Also, 3rd * party nexthop is not propagated for EBGP right now. */ - stream_get_from (&v6nhglobal, s, vec->offset + 1, 16); + switch (nhlen) + { + case BGP_ATTR_NHLEN_IPV6_GLOBAL: + break; + case BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL: + offset_nhlocal += IPV6_MAX_BYTELEN; + break; + case BGP_ATTR_NHLEN_VPNV6_GLOBAL: + offset_nhglobal += 8; + break; + case BGP_ATTR_NHLEN_VPNV6_GLOBAL_AND_LL: + offset_nhglobal += 8; + offset_nhlocal += 8 * 2 + IPV6_MAX_BYTELEN; + break; + default: + /* TODO: handle IPv4 nexthops */ + zlog_warn ("%s: %s: invalid MP nexthop length (AFI IP6): %u", + __func__, peer->host, nhlen); + return NULL; + } + + stream_get_from (&v6nhglobal, s, offset_nhglobal, IPV6_MAX_BYTELEN); if (route_map_sets_nh) { if (CHECK_FLAG(vec->flags, @@ -536,9 +571,10 @@ bpacket_reformat_for_peer (struct bpacket *pkt, struct peer_af *paf) } - if (nhlen == 32 || nhlen == 48) /* 48 == VPN */ + if (nhlen == BGP_ATTR_NHLEN_IPV6_GLOBAL_AND_LL || + nhlen == BGP_ATTR_NHLEN_VPNV6_GLOBAL_AND_LL) { - stream_get_from (&v6nhlocal, s, vec->offset + 1 + (nhlen-IPV6_MAX_BYTELEN), IPV6_MAX_BYTELEN); + stream_get_from (&v6nhlocal, s, offset_nhlocal, IPV6_MAX_BYTELEN); if (IN6_IS_ADDR_UNSPECIFIED (&v6nhlocal)) { mod_v6nhl = &peer->nexthop.v6_local; @@ -547,9 +583,9 @@ bpacket_reformat_for_peer (struct bpacket *pkt, struct peer_af *paf) } if (gnh_modified) - stream_put_in6_addr_at (s, vec->offset + 1, mod_v6nhg); + stream_put_in6_addr_at (s, offset_nhglobal, mod_v6nhg); if (lnh_modified) - stream_put_in6_addr_at (s, vec->offset + 1 + (nhlen-IPV6_MAX_BYTELEN), mod_v6nhl); + stream_put_in6_addr_at (s, offset_nhlocal, mod_v6nhl); if (bgp_debug_update(peer, NULL, NULL, 0)) {