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 688abae0e41ae36033ee56f23311467885b0fc27..9329c8d8922af4978a693f44b3397318098cd42b 100644 (file)
--- 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)) {