Revert "bgpd: Handle Addpath capability using dynamic capabilities"

This reverts commit 05cf9d03b345393b8d63ffe9345c42debd8362b6.

TL;DR; Handling BGP AddPath capability is not trivial (possible) dynamically.

When the sender is AddPath-capable and sends NLRIs encoded with AddPath ID,
and at the same time the receiver sends AddPath capability "disable-addpath-rx"
(flag update) via dynamic capabilities, both peers are out of sync about the
AddPath state. The receiver thinks already he's not AddPath-capable anymore,
hence it tries to parse NLRIs as non-AddPath, while they are actually encoded
as AddPath.

AddPath capability itself does not provide (in RFC) any mechanism on backward
compatible way to handle NLRIs if they come mixed (AddPath + non-AddPath).

This explains why we have failures in our CI periodically.

Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>

1 files changed, 5 insertions, 28 deletions

diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c
index 8e27da54d8..8f6203f34c 100644
--- a/bgpd/bgp_vty.c
+++ b/bgpd/bgp_vty.c
@@ -9006,21 +9006,12 @@ DEFUN(neighbor_disable_addpath_rx,
 	struct peer *peer;
 	afi_t afi = bgp_node_afi(vty);
 	safi_t safi = bgp_node_safi(vty);
-	int ret;
-	int action;
 
 	peer = peer_and_group_lookup_vty(vty, peer_str);
 	if (!peer)
 		return CMD_WARNING_CONFIG_FAILED;
 
-	action = bgp_addpath_capability_action(peer->addpath_type[afi][safi], 0);
-
-	ret = peer_af_flag_set_vty(vty, peer_str, afi, safi,
-				   PEER_FLAG_DISABLE_ADDPATH_RX);
-
-	bgp_capability_send(peer, afi, safi, CAPABILITY_CODE_ADDPATH, action);
-
-	return ret;
+	return peer_af_flag_set_vty(vty, peer_str, afi, safi, PEER_FLAG_DISABLE_ADDPATH_RX);
 }
 
 DEFUN(no_neighbor_disable_addpath_rx,
@@ -9035,21 +9026,12 @@ DEFUN(no_neighbor_disable_addpath_rx,
 	struct peer *peer;
 	afi_t afi = bgp_node_afi(vty);
 	safi_t safi = bgp_node_safi(vty);
-	int ret;
-	int action;
 
 	peer = peer_and_group_lookup_vty(vty, peer_str);
 	if (!peer)
 		return CMD_WARNING_CONFIG_FAILED;
 
-	action = bgp_addpath_capability_action(peer->addpath_type[afi][safi], 0);
-
-	ret = peer_af_flag_unset_vty(vty, peer_str, afi, safi,
-				     PEER_FLAG_DISABLE_ADDPATH_RX);
-
-	bgp_capability_send(peer, afi, safi, CAPABILITY_CODE_ADDPATH, action);
-
-	return ret;
+	return peer_af_flag_unset_vty(vty, peer_str, afi, safi, PEER_FLAG_DISABLE_ADDPATH_RX);
 }
 
 DEFUN (neighbor_addpath_tx_all_paths,
@@ -9061,15 +9043,12 @@ DEFUN (neighbor_addpath_tx_all_paths,
 {
 	int idx_peer = 1;
 	struct peer *peer;
-	afi_t afi = bgp_node_afi(vty);
-	safi_t safi = bgp_node_safi(vty);
 
 	peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg);
 	if (!peer)
 		return CMD_WARNING_CONFIG_FAILED;
 
-	bgp_addpath_set_peer_type(peer, afi, safi, BGP_ADDPATH_ALL, 0);
-
+	bgp_addpath_set_peer_type(peer, bgp_node_afi(vty), bgp_node_safi(vty), BGP_ADDPATH_ALL, 0);
 	return CMD_SUCCESS;
 }
 
@@ -9089,20 +9068,18 @@ DEFUN (no_neighbor_addpath_tx_all_paths,
 {
 	int idx_peer = 2;
 	struct peer *peer;
-	afi_t afi = bgp_node_afi(vty);
-	safi_t safi = bgp_node_safi(vty);
 
 	peer = peer_and_group_lookup_vty(vty, argv[idx_peer]->arg);
 	if (!peer)
 		return CMD_WARNING_CONFIG_FAILED;
 
-	if (peer->addpath_type[afi][safi] != BGP_ADDPATH_ALL) {
+	if (peer->addpath_type[bgp_node_afi(vty)][bgp_node_safi(vty)] != BGP_ADDPATH_ALL) {
 		vty_out(vty,
 			"%% Peer not currently configured to transmit all paths.");
 		return CMD_WARNING_CONFIG_FAILED;
 	}
 
-	bgp_addpath_set_peer_type(peer, afi, safi, BGP_ADDPATH_NONE, 0);
+	bgp_addpath_set_peer_type(peer, bgp_node_afi(vty), bgp_node_safi(vty), BGP_ADDPATH_NONE, 0);
 
 	return CMD_SUCCESS;
 }