bgpd: make as-path-loop-detection conform to the framework

bgpd/bgp_route.c

@@ -2242,7 +2242,7 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi,
 	}
 
 	/* AS path loop check. */
-	if (peer->as_path_loop_detection &&
+	if (CHECK_FLAG(peer->flags, PEER_FLAG_AS_LOOP_DETECTION) &&
 	    aspath_loop_check(piattr->aspath, peer->as)) {
 		if (bgp_debug_update(NULL, p, subgrp->update_group, 0))
 			zlog_debug(

bgpd/bgp_updgrp.c

@@ -151,7 +151,6 @@ static void conf_copy(struct peer *dst, struct peer *src, afi_t afi,
 	dst->change_local_as = src->change_local_as;
 	dst->shared_network = src->shared_network;
 	dst->local_role = src->local_role;
-	dst->as_path_loop_detection = src->as_path_loop_detection;
 
 	if (src->soo[afi][safi]) {
 		ecommunity_free(&dst->soo[afi][safi]);
@@ -360,9 +359,12 @@ static unsigned int updgrp_hash_key_make(const void *p)
 	key = jhash_1word(peer->max_packet_size, key);
 	key = jhash_1word(peer->pmax_out[afi][safi], key);
 
-	if (peer->as_path_loop_detection)
-		key = jhash_2words(peer->as, peer->as_path_loop_detection, key);
 
+	if (CHECK_FLAG(peer->flags, PEER_FLAG_AS_LOOP_DETECTION))
+		key = jhash_2words(peer->as,
+				   CHECK_FLAG(peer->flags,
+					      PEER_FLAG_AS_LOOP_DETECTION),
+				   key);
 	if (peer->group)
 		key = jhash_1word(jhash(peer->group->name,
 					strlen(peer->group->name), SEED1),
@@ -464,7 +466,8 @@ static unsigned int updgrp_hash_key_make(const void *p)
 			   CHECK_FLAG(peer->af_cap[afi][safi],
 				      PEER_UPDGRP_AF_CAP_FLAGS),
 			   peer->v_routeadv, peer->change_local_as,
-			   peer->as_path_loop_detection);
+			   !!CHECK_FLAG(peer->flags,
+					PEER_FLAG_AS_LOOP_DETECTION));
 		zlog_debug("%pBP Update Group Hash: addpath paths-limit: (send %u, receive %u)",
 			   peer, peer->addpath_paths_limit[afi][safi].send,
 			   peer->addpath_paths_limit[afi][safi].receive);

bgpd/bgp_vty.c

@@ -9198,15 +9198,7 @@ DEFPY(
 	NEIGHBOR_ADDR_STR2
 	"Detect AS loops before sending to neighbor\n")
 {
-	struct peer *peer;
-
-	peer = peer_and_group_lookup_vty(vty, neighbor);
-	if (!peer)
-		return CMD_WARNING_CONFIG_FAILED;
-
-	peer->as_path_loop_detection = true;
-
-	return CMD_SUCCESS;
+	return peer_flag_set_vty(vty, neighbor, PEER_FLAG_AS_LOOP_DETECTION);
 }
 
 DEFPY (neighbor_addpath_paths_limit,
@@ -9275,15 +9267,7 @@ DEFPY(
 	NEIGHBOR_ADDR_STR2
 	"Detect AS loops before sending to neighbor\n")
 {
-	struct peer *peer;
-
-	peer = peer_and_group_lookup_vty(vty, neighbor);
-	if (!peer)
-		return CMD_WARNING_CONFIG_FAILED;
-
-	peer->as_path_loop_detection = false;
-
-	return CMD_SUCCESS;
+	return peer_flag_unset_vty(vty, neighbor, PEER_FLAG_AS_LOOP_DETECTION);
 }
 
 DEFPY(neighbor_path_attribute_discard,
@@ -18460,7 +18444,7 @@ static void bgp_config_write_peer_global(struct vty *vty, struct bgp *bgp,
 		vty_out(vty, " neighbor %s strict-capability-match\n", addr);
 
 	/* Sender side AS path loop detection. */
-	if (peer->as_path_loop_detection)
+	if (peergroup_flag_check(peer, PEER_FLAG_AS_LOOP_DETECTION))
 		vty_out(vty, " neighbor %s sender-as-path-loop-detection\n",
 			addr);
 

bgpd/bgpd.c

@@ -4587,6 +4587,7 @@ static const struct peer_flag_action peer_flag_action_list[] = {
 	{PEER_FLAG_GRACEFUL_SHUTDOWN, 0, peer_change_none},
 	{PEER_FLAG_CAPABILITY_SOFT_VERSION, 0, peer_change_none},
 	{PEER_FLAG_CAPABILITY_FQDN, 0, peer_change_none},
+	{PEER_FLAG_AS_LOOP_DETECTION, 0, peer_change_none},
 	{0, 0, 0}};
 
 static const struct peer_flag_action peer_af_flag_action_list[] = {

bgpd/bgpd.h

@@ -1470,6 +1470,7 @@ struct peer {
 #define PEER_FLAG_GRACEFUL_SHUTDOWN (1ULL << 35)
 #define PEER_FLAG_CAPABILITY_SOFT_VERSION (1ULL << 36)
 #define PEER_FLAG_CAPABILITY_FQDN (1ULL << 37)  /* fqdn capability */
+#define PEER_FLAG_AS_LOOP_DETECTION (1ULL << 38) /* as path loop detection */
 
 	/*
 	 *GR-Disabled mode means unset PEER_FLAG_GRACEFUL_RESTART
@@ -1823,9 +1824,6 @@ struct peer {
 	char *hostname;
 	char *domainname;
 
-	/* Sender side AS path loop detection. */
-	bool as_path_loop_detection;
-
 	/* Extended Message Support */
 	uint16_t max_packet_size;
 

tests/topotests/bgp_sender_as_path_loop_detection/test_bgp_sender-as-path-loop-detection.py

@@ -129,6 +129,83 @@ def _bgp_suppress_route_to_r3():
     assert result is None, "Routes should not be sent to r1 from r2"
 
 
+def test_remove_loop_detection_on_one_peer():
+    tgen = get_topogen()
+
+    if tgen.routers_have_failure():
+        pytest.skip(tgen.errors)
+
+    r2 = tgen.gears["r2"]
+
+    def _bgp_reset_route_to_r1():
+        output = json.loads(
+            r2.vtysh_cmd("show ip bgp neighbor 192.168.255.2 advertised-routes json")
+        )
+        expected = {"totalPrefixCounter": 3}
+        return topotest.json_cmp(output, expected)
+
+    r2.vtysh_cmd(
+        """
+        configure terminal
+         router bgp 65002
+          no neighbor 192.168.255.2 sender-as-path-loop-detection
+        """
+    )
+
+    r2.vtysh_cmd(
+        """
+        clear bgp *
+        """
+    )
+    test_func = functools.partial(_bgp_reset_route_to_r1)
+    _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5)
+    assert result is None, "Failed bgp to reset route"
+
+
+def test_loop_detection_on_peer_group():
+    tgen = get_topogen()
+
+    r2 = tgen.gears["r2"]
+
+    def _bgp_suppress_route_to_r1():
+        output = json.loads(
+            r2.vtysh_cmd("show ip bgp neighbor 192.168.255.2 advertised-routes json")
+        )
+        expected = {"totalPrefixCounter": 0}
+        return topotest.json_cmp(output, expected)
+
+    def _bgp_suppress_route_to_r3():
+        output = json.loads(
+            r2.vtysh_cmd("show ip bgp neighbor 192.168.254.2 advertised-routes json")
+        )
+        expected = {"totalPrefixCounter": 2}
+        return topotest.json_cmp(output, expected)
+
+    r2.vtysh_cmd(
+        """
+        configure terminal
+         router bgp 65002
+          neighbor loop_group peer-group
+          neighbor 192.168.255.2 peer-group loop_group
+          neighbor loop_group sender-as-path-loop-detection
+        """
+    )
+
+    r2.vtysh_cmd(
+        """
+        clear bgp *
+        """
+    )
+
+    test_func = functools.partial(_bgp_suppress_route_to_r3)
+    _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5)
+    assert result is None, "Route 172.16.255.253/32 should not be sent to r3 from r2"
+
+    test_func = functools.partial(_bgp_suppress_route_to_r1)
+    _, result = topotest.run_and_expect(test_func, None, count=30, wait=0.5)
+    assert result is None, "Routes should not be sent to r1 from r2"
+
+
 if __name__ == "__main__":
     args = ["-s"] + sys.argv[1:]
     sys.exit(pytest.main(args))