Merge pull request #17184 from FRRouting/mergify/bp/dev/10.2/pr-17168

bgpd: fix AIGP calculation in route advertisement (backport #17168)

bgpd/bgp_attr.c

@@ -480,12 +480,11 @@ static bool bgp_attr_aigp_get_tlv_metric(uint8_t *pnt, int length,
 	return false;
 }
 
-static void stream_put_bgp_aigp_tlv_metric(struct stream *s,
-					   struct bgp_path_info *bpi)
+static void stream_put_bgp_aigp_tlv_metric(struct stream *s, uint64_t aigp)
 {
 	stream_putc(s, BGP_AIGP_TLV_METRIC);
 	stream_putw(s, BGP_AIGP_TLV_METRIC_LEN);
-	stream_putq(s, bgp_aigp_metric_total(bpi));
+	stream_putq(s, aigp);
 }
 
 static bool bgp_attr_aigp_valid(uint8_t *pnt, int length)
@@ -4549,14 +4548,11 @@ static void bgp_packet_ecommunity_attribute(struct stream *s, struct peer *peer,
 }
 
 /* Make attribute packet. */
-bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
-				struct stream *s, struct attr *attr,
-				struct bpacket_attr_vec_arr *vecarr,
-				struct prefix *p, afi_t afi, safi_t safi,
-				struct peer *from, struct prefix_rd *prd,
-				mpls_label_t *label, uint8_t num_labels,
-				bool addpath_capable, uint32_t addpath_tx_id,
-				struct bgp_path_info *bpi)
+bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer, struct stream *s,
+				struct attr *attr, struct bpacket_attr_vec_arr *vecarr,
+				struct prefix *p, afi_t afi, safi_t safi, struct peer *from,
+				struct prefix_rd *prd, mpls_label_t *label, uint8_t num_labels,
+				bool addpath_capable, uint32_t addpath_tx_id)
 {
 	size_t cp;
 	size_t aspath_sizep;
@@ -5035,10 +5031,7 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
 	}
 
 	/* AIGP */
-	if (bpi && CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) &&
-	    (CHECK_FLAG(peer->flags, PEER_FLAG_AIGP) ||
-	     peer->sub_sort == BGP_PEER_EBGP_OAD ||
-	     peer->sort != BGP_PEER_EBGP)) {
+	if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) && AIGP_TRANSMIT_ALLOWED(peer)) {
 		/* At the moment only AIGP Metric TLV exists for AIGP
 		 * attribute. If more comes in, do not forget to update
 		 * attr_len variable to include new ones.
@@ -5048,7 +5041,7 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer,
 		stream_putc(s, BGP_ATTR_FLAG_OPTIONAL);
 		stream_putc(s, BGP_ATTR_AIGP);
 		stream_putc(s, attr_len);
-		stream_put_bgp_aigp_tlv_metric(s, bpi);
+		stream_put_bgp_aigp_tlv_metric(s, attr->aigp_metric);
 	}
 
 	/* Unknown transit attribute. */
@@ -5317,7 +5310,7 @@ void bgp_dump_routes_attr(struct stream *s, struct bgp_path_info *bpi,
 		stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS);
 		stream_putc(s, BGP_ATTR_AIGP);
 		stream_putc(s, attr_len);
-		stream_put_bgp_aigp_tlv_metric(s, bpi);
+		stream_put_bgp_aigp_tlv_metric(s, attr->aigp_metric);
 	}
 
 	/* Return total size of attribute. */

bgpd/bgp_attr.h

@@ -387,12 +387,12 @@ extern struct attr *bgp_attr_aggregate_intern(
 	struct community *community, struct ecommunity *ecommunity,
 	struct lcommunity *lcommunity, struct bgp_aggregate *aggregate,
 	uint8_t atomic_aggregate, const struct prefix *p);
-extern bgp_size_t bgp_packet_attribute(
-	struct bgp *bgp, struct peer *peer, struct stream *s, struct attr *attr,
-	struct bpacket_attr_vec_arr *vecarr, struct prefix *p, afi_t afi,
-	safi_t safi, struct peer *from, struct prefix_rd *prd,
-	mpls_label_t *label, uint8_t num_labels, bool addpath_capable,
-	uint32_t addpath_tx_id, struct bgp_path_info *bpi);
+extern bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer, struct stream *s,
+				       struct attr *attr, struct bpacket_attr_vec_arr *vecarr,
+				       struct prefix *p, afi_t afi, safi_t safi, struct peer *from,
+				       struct prefix_rd *prd, mpls_label_t *label,
+				       uint8_t num_labels, bool addpath_capable,
+				       uint32_t addpath_tx_id);
 extern void bgp_dump_routes_attr(struct stream *s, struct bgp_path_info *bpi,
 				 const struct prefix *p);
 extern bool attrhash_cmp(const void *arg1, const void *arg2);
@@ -585,6 +585,10 @@ static inline void bgp_attr_set_transit(struct attr *attr,
 	attr->transit = transit;
 }
 
+#define AIGP_TRANSMIT_ALLOWED(peer)                                                                \
+	(CHECK_FLAG((peer)->flags, PEER_FLAG_AIGP) || ((peer)->sub_sort == BGP_PEER_EBGP_OAD) ||   \
+	 ((peer)->sort != BGP_PEER_EBGP))
+
 static inline uint64_t bgp_attr_get_aigp_metric(const struct attr *attr)
 {
 	return attr->aigp_metric;

bgpd/bgp_bmp.c

@@ -934,9 +934,8 @@ static struct stream *bmp_update(const struct prefix *p, struct prefix_rd *prd,
 	stream_putw(s, 0);
 
 	/* 5: Encode all the attributes, except MP_REACH_NLRI attr. */
-	total_attr_len =
-		bgp_packet_attribute(NULL, peer, s, attr, &vecarr, NULL, afi,
-				     safi, peer, NULL, NULL, 0, 0, 0, NULL);
+	total_attr_len = bgp_packet_attribute(NULL, peer, s, attr, &vecarr, NULL, afi, safi, peer,
+					      NULL, NULL, 0, 0, 0);
 
 	/* space check? */
 

bgpd/bgp_route.c

@@ -2820,6 +2820,21 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
 					false));
 	}
 
+	/*
+	 * Adjust AIGP for propagation when the nexthop is set to ourselves,
+	 * e.g., using "set ip nexthop peer-address" or when advertising to
+	 * EBGP. Note in route reflection the nexthop is usually unmodified
+	 * and the AIGP should not be adjusted in that case.
+	 */
+	if (CHECK_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) && AIGP_TRANSMIT_ALLOWED(peer)) {
+		if (nh_reset ||
+		    CHECK_FLAG(attr->rmap_change_flags, BATTR_RMAP_NEXTHOP_PEER_ADDRESS)) {
+			uint64_t aigp = bgp_aigp_metric_total(pi);
+
+			bgp_attr_set_aigp_metric(attr, aigp);
+		}
+	}
+
 	return true;
 }
 

bgpd/bgp_updgrp_packet.c

@@ -738,9 +738,9 @@ struct bpacket *subgroup_update_packet(struct update_subgroup *subgrp)
 
 			/* 5: Encode all the attributes, except MP_REACH_NLRI
 			 * attr. */
-			total_attr_len = bgp_packet_attribute(
-				NULL, peer, s, adv->baa->attr, &vecarr, NULL,
-				afi, safi, from, NULL, NULL, 0, 0, 0, path);
+			total_attr_len = bgp_packet_attribute(NULL, peer, s, adv->baa->attr,
+							      &vecarr, NULL, afi, safi, from, NULL,
+							      NULL, 0, 0, 0);
 
 			space_remaining =
 				STREAM_CONCAT_REMAIN(s, snlri, STREAM_SIZE(s))
@@ -1149,12 +1149,9 @@ void subgroup_default_update_packet(struct update_subgroup *subgrp,
 	/* Make place for total attribute length.  */
 	pos = stream_get_endp(s);
 	stream_putw(s, 0);
-	total_attr_len =
-		bgp_packet_attribute(NULL, peer, s, attr, &vecarr, &p, afi,
-				     safi, from, NULL, &label, num_labels,
-				     addpath_capable,
-				     BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE,
-				     NULL);
+	total_attr_len = bgp_packet_attribute(NULL, peer, s, attr, &vecarr, &p, afi, safi, from,
+					      NULL, &label, num_labels, addpath_capable,
+					      BGP_ADDPATH_TX_ID_FOR_DEFAULT_ORIGINATE);
 
 	/* Set Total Path Attribute Length. */
 	stream_putw_at(s, pos, total_attr_len);

tests/topotests/bgp_aigp_rr/r1/bgpd.conf

@@ -0,0 +1,27 @@
+router bgp 65001
+ no bgp ebgp-requires-policy
+ no bgp network import-check
+ bgp route-reflector allow-outbound-policy
+ neighbor 10.0.0.2 remote-as internal
+ neighbor 10.0.0.2 update-source lo
+ neighbor 10.0.0.2 timers 1 3
+ neighbor 10.0.0.2 timers connect 1
+ neighbor 10.0.0.2 route-reflector-client
+ neighbor 10.0.0.3 remote-as internal
+ neighbor 10.0.0.3 update-source lo
+ neighbor 10.0.0.3 timers 1 3
+ neighbor 10.0.0.3 timers connect 1
+ neighbor 10.0.0.3 route-reflector-client
+ neighbor 10.0.0.4 remote-as internal
+ neighbor 10.0.0.4 update-source lo
+ neighbor 10.0.0.4 timers 1 3
+ neighbor 10.0.0.4 timers connect 1
+ neighbor 10.0.0.4 route-reflector-client
+ address-family ipv4
+  neighbor 10.0.0.4 route-map set-nexthop out
+ exit-address-family
+!
+route-map set-nexthop permit 10
+ set ip next-hop peer-address
+exit
+!

tests/topotests/bgp_aigp_rr/r1/ospfd.conf

@@ -0,0 +1,23 @@
+!
+interface lo
+ ip ospf passive
+!
+interface r1-eth0
+ ip ospf dead-interval 4
+ ip ospf hello-interval 1
+ ip ospf cost 10
+!
+interface r1-eth1
+ ip ospf dead-interval 4
+ ip ospf hello-interval 1
+ ip ospf cost 10
+!
+interface r1-eth2
+ ip ospf dead-interval 4
+ ip ospf hello-interval 1
+ ip ospf cost 10
+!
+router ospf
+ router-id 10.0.0.1
+ network 0.0.0.0/0 area 0
+!

tests/topotests/bgp_aigp_rr/r1/zebra.conf

@@ -0,0 +1,13 @@
+!
+interface lo
+ ip address 10.0.0.1/32
+!
+interface r1-eth0
+ ip address 192.168.12.1/24
+!
+interface r1-eth1
+ ip address 192.168.13.1/24
+!
+interface r1-eth2
+ ip address 192.168.14.1/24
+!

tests/topotests/bgp_aigp_rr/r2/bgpd.conf

@@ -0,0 +1,18 @@
+router bgp 65001
+ no bgp ebgp-requires-policy
+ no bgp network import-check
+ neighbor 10.0.0.1 remote-as internal
+ neighbor 10.0.0.1 update-source lo
+ neighbor 10.0.0.1 timers 1 3
+ neighbor 10.0.0.1 timers connect 1
+ address-family ipv4
+  redistribute connected route-map connected-to-bgp
+  neighbor 10.0.0.1 next-hop-self
+ exit-address-family
+!
+ip prefix-list p22 seq 5 permit 10.0.2.2/32
+!
+route-map connected-to-bgp permit 10
+ match ip address prefix-list p22
+ set aigp 2
+!

tests/topotests/bgp_aigp_rr/r2/ospfd.conf

@@ -0,0 +1,18 @@
+!
+interface lo
+ ip ospf passive
+!
+interface r2-eth0
+ ip ospf dead-interval 4
+ ip ospf hello-interval 1
+ ip ospf cost 10
+!
+interface r2-eth1
+ ip ospf dead-interval 4
+ ip ospf hello-interval 1
+ ip ospf cost 10
+!
+router ospf
+ router-id 10.0.0.2
+ network 0.0.0.0/0 area 0
+!

tests/topotests/bgp_aigp_rr/r2/zebra.conf

@@ -0,0 +1,11 @@
+!
+interface lo
+ ip address 10.0.0.2/32
+ ip address 10.0.2.2/32
+!
+interface r2-eth0
+ ip address 192.168.12.2/24
+!
+interface r2-eth1
+ ip address 192.168.23.2/24
+!

tests/topotests/bgp_aigp_rr/r3/bgpd.conf

@@ -0,0 +1,11 @@
+router bgp 65001
+ no bgp ebgp-requires-policy
+ no bgp network import-check
+ neighbor 10.0.0.1 remote-as internal
+ neighbor 10.0.0.1 update-source lo
+ neighbor 10.0.0.1 timers 1 3
+ neighbor 10.0.0.1 timers connect 1
+ address-family ipv4
+  neighbor 10.0.0.1 next-hop-self
+ exit-address-family
+!

tests/topotests/bgp_aigp_rr/r3/ospfd.conf

@@ -0,0 +1,18 @@
+!
+interface lo
+ ip ospf passive
+!
+interface r3-eth0
+ ip ospf dead-interval 4
+ ip ospf hello-interval 1
+ ip ospf cost 10
+!
+interface r3-eth1
+ ip ospf dead-interval 4
+ ip ospf hello-interval 1
+ ip ospf cost 10
+!
+router ospf
+ router-id 10.0.0.3
+ network 0.0.0.0/0 area 0
+!

tests/topotests/bgp_aigp_rr/r3/zebra.conf

@@ -0,0 +1,10 @@
+!
+interface lo
+ ip address 10.0.0.3/32
+!
+interface r3-eth0
+ ip address 192.168.13.3/24
+!
+interface r3-eth1
+ ip address 192.168.23.3/24
+!

tests/topotests/bgp_aigp_rr/r4/bgpd.conf

@@ -0,0 +1,11 @@
+router bgp 65001
+ no bgp ebgp-requires-policy
+ no bgp network import-check
+ neighbor 10.0.0.1 remote-as internal
+ neighbor 10.0.0.1 update-source lo
+ neighbor 10.0.0.1 timers 1 3
+ neighbor 10.0.0.1 timers connect 1
+ address-family ipv4
+  neighbor 10.0.0.1 next-hop-self
+ exit-address-family
+!

tests/topotests/bgp_aigp_rr/r4/ospfd.conf

@@ -0,0 +1,13 @@
+!
+interface lo
+ ip ospf passive
+!
+interface r4-eth0
+ ip ospf dead-interval 4
+ ip ospf hello-interval 1
+ ip ospf cost 10
+!
+router ospf
+ router-id 10.0.0.4
+ network 0.0.0.0/0 area 0
+!

tests/topotests/bgp_aigp_rr/r4/zebra.conf

@@ -0,0 +1,7 @@
+!
+interface lo
+ ip address 10.0.0.4/32
+!
+interface r4-eth0
+ ip address 192.168.14.4/24
+!