diff options
| author | Don Slice <dslice@cumulusnetworks.com> | 2019-09-24 05:02:02 -0700 |
|---|---|---|
| committer | Don Slice <dslice@cumulusnetworks.com> | 2019-09-25 13:54:39 -0700 |
| commit | 1c875ddb576d94f08e6786bf1ec02d1809ce107c (patch) | |
| tree | 8959af449c1a02a10a5deea2730d80e63ab91470 | |
| parent | e030e863b673e59a6daa75b1a7be8f1987a8e8bd (diff) | |
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>
| -rw-r--r-- | bgpd/bgp_updgrp_packet.c | 47 |
1 files changed, 29 insertions, 18 deletions
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)) { |
