From: Don Slice <dslice@cumulusnetworks.com>
Date: Tue, 24 Sep 2019 12:02:02 +0000 (-0700)
Subject: bgpd: stop sending nexthop set by "route-map in" to eBGP peers
X-Git-Tag: frr-7.2~9^2
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=refs%2Fpull%2F5057%2Fhead;p=mirror%2Ffrr.git

bgpd: stop sending nexthop set by "route-map in" to eBGP peers

Problem reported that when a "neighbor x.x.x.x route-map FOO in"
set a next-hop value, that modified next-hop value was also sent
to eBGP peers.  This is incorrect since bgp is expected to set
next-hop to self when sending to eBGP peers unless third party
next-hop on a shared segment is true.  This fix modifies the
behavior to stop sending the modified next-hop to eBGP peers
if the route-map was applied inbound on another peer.

Ticket: CM-26025
Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
---

diff --git a/bgpd/bgp_updgrp_packet.c b/bgpd/bgp_updgrp_packet.c
index 688abae0e4..9329c8d892 100644
--- a/bgpd/bgp_updgrp_packet.c
+++ b/bgpd/bgp_updgrp_packet.c
@@ -391,6 +391,7 @@ struct stream *bpacket_reformat_for_peer(struct bpacket *pkt,
 	struct peer *peer;
 	char buf[BUFSIZ];
 	char buf2[BUFSIZ];
+	struct bgp_filter *filter;
 
 	s = stream_dup(pkt->buffer);
 	peer = PAF_PEER(paf);
@@ -401,6 +402,8 @@ struct stream *bpacket_reformat_for_peer(struct bpacket *pkt,
 		afi_t nhafi;
 		int route_map_sets_nh;
 		nhlen = stream_getc_from(s, vec->offset);
+		filter = &peer->filter[paf->afi][paf->safi];
+
 		if (peer_cap_enhe(peer, paf->afi, paf->safi))
 			nhafi = AFI_IP6;
 		else
@@ -439,25 +442,25 @@ struct stream *bpacket_reformat_for_peer(struct bpacket *pkt,
 			mod_v4nh = &v4nh;
 
 			/*
-			 * If route-map has set the nexthop, that is always
-			 * used; if it is
-			 * specified as peer-address, the peering address is
-			 * picked up.
-			 * Otherwise, if NH is unavailable from attribute, the
-			 * peering addr
-			 * is picked up; the "NH unavailable" case also covers
-			 * next-hop-self
-			 * and some other scenarios -- see
-			 * subgroup_announce_check(). In
-			 * all other cases, use the nexthop carried in the
-			 * attribute unless
-			 * it is EBGP non-multiaccess and there is no
-			 * next-hop-unchanged setting.
+			 * If route-map has set the nexthop, that is normally
+			 * used; if it is specified as peer-address, the peering
+			 * address is picked up. Otherwise, if NH is unavailable
+			 * from attribute, the peering addr is picked up; the
+			 * "NH unavailable" case also covers next-hop-self and
+			 * some other scenarios - see subgroup_announce_check().
+			 * In all other cases, use the nexthop carried in the
+			 * attribute unless it is EBGP non-multiaccess and there
+			 * is no next-hop-unchanged setting or the peer is EBGP
+			 * and the route-map that changed the next-hop value
+			 * was applied inbound rather than outbound. Updates to
+			 * an EBGP peer should only modify the next-hop if it
+			 * was set in an outbound route-map to that peer.
 			 * Note: It is assumed route-map cannot set the nexthop
-			 * to an
-			 * invalid value.
+			 * to an invalid value.
 			 */
-			if (route_map_sets_nh) {
+			if (route_map_sets_nh
+			    && ((peer->sort != BGP_PEER_EBGP)
+				|| ROUTE_MAP_OUT(filter))) {
 				if (CHECK_FLAG(
 					    vec->flags,
 					    BPKT_ATTRVEC_FLAGS_RMAP_NH_PEER_ADDRESS)) {
@@ -543,7 +546,15 @@ struct stream *bpacket_reformat_for_peer(struct bpacket *pkt,
 
 			stream_get_from(&v6nhglobal, s, offset_nhglobal,
 					IPV6_MAX_BYTELEN);
-			if (route_map_sets_nh) {
+
+			/*
+			 * Updates to an EBGP peer should only modify the
+			 * next-hop if it was set in an outbound route-map
+			 * to that peer.
+			 */
+			if (route_map_sets_nh
+			    && ((peer->sort != BGP_PEER_EBGP)
+				|| ROUTE_MAP_OUT(filter))) {
 				if (CHECK_FLAG(
 					    vec->flags,
 					    BPKT_ATTRVEC_FLAGS_RMAP_NH_PEER_ADDRESS)) {