Merge pull request FRRouting#17147 from FRRouting/mergify/bp/stable/1…

…0.1/pr-17091

bgpd: fix several issues in sourcing AIGP attribute (backport FRRouting#17091)

bgpd/bgp_attr.h

@@ -595,9 +595,7 @@ static inline uint64_t bgp_attr_get_aigp_metric(const struct attr *attr)
 static inline void bgp_attr_set_aigp_metric(struct attr *attr, uint64_t aigp)
 {
 	attr->aigp_metric = aigp;
-
-	if (aigp)
-		SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP));
+	SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP));
 }
 
 static inline uint64_t bgp_aigp_metric_total(struct bgp_path_info *bpi)

bgpd/bgp_route.c

@@ -6698,9 +6698,6 @@ void bgp_static_update(struct bgp *bgp, const struct prefix *p,
 	if (afi == AFI_IP)
 		attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4;
 
-	if (bgp_static->igpmetric)
-		bgp_attr_set_aigp_metric(&attr, bgp_static->igpmetric);
-
 	if (bgp_static->atomic)
 		attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_ATOMIC_AGGREGATE);
 
@@ -8955,9 +8952,6 @@ void bgp_redistribute_add(struct bgp *bgp, struct prefix *p,
 	attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC);
 	attr.tag = tag;
 
-	if (metric)
-		bgp_attr_set_aigp_metric(&attr, metric);
-
 	afi = family2afi(p->family);
 
 	red = bgp_redist_lookup(bgp, afi, type, instance);
@@ -8967,10 +8961,8 @@ void bgp_redistribute_add(struct bgp *bgp, struct prefix *p,
 		/* Copy attribute for modification. */
 		attr_new = attr;
 
-		if (red->redist_metric_flag) {
+		if (red->redist_metric_flag)
 			attr_new.med = red->redist_metric;
-			bgp_attr_set_aigp_metric(&attr_new, red->redist_metric);
-		}
 
 		/* Apply route-map. */
 		if (red->rmap.name) {

bgpd/bgp_routemap.c

@@ -3451,19 +3451,15 @@ route_set_aigp_metric(void *rule, const struct prefix *pfx, void *object)
 {
 	const char *aigp_metric = rule;
 	struct bgp_path_info *path = object;
-	uint32_t aigp = 0;
+	uint32_t aigp;
 
-	if (strmatch(aigp_metric, "igp-metric")) {
-		if (!path->nexthop)
-			return RMAP_NOMATCH;
-
-		bgp_attr_set_aigp_metric(path->attr, path->nexthop->metric);
-	} else {
+	/* Note: the metric is stored as MED for a locally redistributed. */
+	if (strmatch(aigp_metric, "igp-metric"))
+		aigp = path->nexthop ? path->nexthop->metric : path->attr->med;
+	else
 		aigp = atoi(aigp_metric);
-		bgp_attr_set_aigp_metric(path->attr, aigp);
-	}
 
-	path->attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_AIGP);
+	bgp_attr_set_aigp_metric(path->attr, aigp);
 
 	return RMAP_OKAY;
 }

tests/topotests/bgp_aigp/r4/bgpd.conf

@@ -13,9 +13,16 @@ router bgp 65001
  neighbor 10.0.0.6 update-source lo
  address-family ipv4
   redistribute connected route-map connected-to-bgp
+  redistribute ospf route-map ospf-to-bgp
   neighbor 192.168.24.2 route-map set-nexthop out
  exit-address-family
 !
+ip prefix-list p66 seq 5 permit 10.0.6.6/32
+!
+route-map ospf-to-bgp permit 10
+ match ip address prefix-list p66
+ set aigp igp-metric
+!
 ! Two OSPF domains should be isolated - otherwise the connected routes
 ! on r4 would be advertised to r3 (via r4 -> r6 -> r5 -> r3), and can
 ! mess up bgp bestpath calculation (igp metrics for the BGP nexthops).

tests/topotests/bgp_aigp/r6/zebra.conf

@@ -1,6 +1,7 @@
 !
 interface lo
  ip address 10.0.0.6/32
+ ip address 10.0.6.6/32
 !
 interface r6-eth0
  ip address 192.168.46.6/24

tests/topotests/bgp_aigp/test_bgp_aigp.py

@@ -228,6 +228,11 @@ def _bgp_check_aigp_metric_bestpath():
     """
     )
 
+    # r4, 10.0.6.6/32 with aigp-metric 20
+    test_func = functools.partial(_bgp_check_aigp_metric, r4, "10.0.6.6/32", 20)
+    _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)
+    assert result is None, "aigp-metric for 10.0.6.6/32 is not 20"
+
     # r4, 10.0.0.71/32 with aigp-metric 71
     test_func = functools.partial(_bgp_check_aigp_metric, r4, "10.0.0.71/32", 71)
     _, result = topotest.run_and_expect(test_func, None, count=60, wait=1)