Merge pull request #15257 from opensourcerouting/fix/reinstall_aggreg…

…ate_route_if_rmap

bgpd: Reinstall aggregated routes if using route-maps and it was changed

bgpd/bgp_route.c

@@ -7369,8 +7369,9 @@ static void bgp_aggregate_install(
 		 * If the aggregate information has not changed
 		 * no need to re-install it again.
 		 */
-		if (pi && bgp_aggregate_info_same(pi, origin, aspath, community,
-						  ecommunity, lcommunity)) {
+		if (pi && (!aggregate->rmap.changed &&
+			   bgp_aggregate_info_same(pi, origin, aspath, community,
+						   ecommunity, lcommunity))) {
 			bgp_dest_unlock_node(dest);
 
 			if (aspath)
@@ -8377,6 +8378,7 @@ static int bgp_aggregate_set(struct vty *vty, const char *prefix_str, afi_t afi,
 		aggregate->rmap.name =
 			XSTRDUP(MTYPE_ROUTE_MAP_NAME, rmap);
 		aggregate->rmap.map = route_map_lookup_by_name(rmap);
+		aggregate->rmap.changed = true;
 		route_map_counter_increment(aggregate->rmap.map);
 	}
 

bgpd/bgp_route.h

@@ -431,6 +431,7 @@ struct bgp_aggregate {
 	struct {
 		char *name;
 		struct route_map *map;
+		bool changed;
 	} rmap;
 
 	/* Suppress-count. */

bgpd/bgp_routemap.c

@@ -4596,6 +4596,7 @@ static void bgp_route_map_process_update(struct bgp *bgp, const char *rmap_name,
 					route_map_counter_increment(map);
 
 				aggregate->rmap.map = map;
+				aggregate->rmap.changed = true;
 
 				matched = true;
 			}

tests/topotests/bgp_aggregate_address_route_map/test_bgp_aggregate-address_route-map.py

@@ -74,7 +74,8 @@ def test_bgp_maximum_prefix_invalid():
     if tgen.routers_have_failure():
         pytest.skip(tgen.errors)
 
-    router = tgen.gears["r2"]
+    r1 = tgen.gears["r1"]
+    r2 = tgen.gears["r2"]
 
     def _bgp_converge(router):
         output = json.loads(router.vtysh_cmd("show ip bgp neighbor 192.168.255.1 json"))
@@ -86,22 +87,30 @@ def _bgp_converge(router):
         }
         return topotest.json_cmp(output, expected)
 
-    def _bgp_aggregate_address_has_metric(router):
+    def _bgp_aggregate_address_has_metric(router, metric):
         output = json.loads(router.vtysh_cmd("show ip bgp 172.16.255.0/24 json"))
-        expected = {"paths": [{"metric": 123}]}
+        expected = {"paths": [{"metric": metric}]}
         return topotest.json_cmp(output, expected)
 
-    test_func = functools.partial(_bgp_converge, router)
-    success, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5)
-
-    assert result is None, 'Failed to see bgp convergence in "{}"'.format(router)
-
-    test_func = functools.partial(_bgp_aggregate_address_has_metric, router)
-    success, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5)
-
-    assert (
-        result is None
-    ), 'Failed to see applied metric for aggregated prefix in "{}"'.format(router)
+    test_func = functools.partial(_bgp_converge, r2)
+    _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5)
+    assert result is None, "Failed to see bgp convergence in r2"
+
+    test_func = functools.partial(_bgp_aggregate_address_has_metric, r2, 123)
+    _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5)
+    assert result is None, "Failed to see applied metric for aggregated prefix in r2"
+
+    r1.vtysh_cmd(
+        """
+    configure terminal
+     route-map aggr-rmap permit 10
+      set metric 666
+    """
+    )
+
+    test_func = functools.partial(_bgp_aggregate_address_has_metric, r2, 666)
+    _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5)
+    assert result is None, "Failed to see applied metric for aggregated prefix in r2"
 
 
 if __name__ == "__main__":