BGP: Set the peer's state to idle when it is Connect or Active in bgp_collision_detect

[zmw12306]

In function bgp_collision_detect:
When other->status ==Connect || Active, is it better to set the state to Idle?

By stopping the outgoing connection attempts, the BGP process conserves resources, reduces unnecessary traffic, and avoids potential further complications or errors that might arise from having two simultaneous connections trying to reach the same state.

frr/bgpd/bgp_packet.c

Lines 1604 to 1698 in 26373fb

	static int bgp_collision_detect(struct peer_connection *connection,
	struct peer *new, struct in_addr remote_id)
	{
	struct peer *peer;
	struct peer_connection *other;
	/*
	* Upon receipt of an OPEN message, the local system must examine
	* all of its connections that are in the OpenConfirm state. A BGP
	* speaker may also examine connections in an OpenSent state if it
	* knows the BGP Identifier of the peer by means outside of the
	* protocol. If among these connections there is a connection to a
	* remote BGP speaker whose BGP Identifier equals the one in the
	* OPEN message, then the local system performs the following
	* collision resolution procedure:
	*/
	peer = new->doppelganger;
	if (peer == NULL)
	return 0;
	other = peer->connection;
	/*
	* Do not accept the new connection in Established or Clearing
	* states. Note that a peer GR is handled by closing the existing
	* connection upon receipt of new one.
	*/
	if (peer_established(other) ||
	other->status == Clearing) {
	bgp_notify_send(connection, BGP_NOTIFY_CEASE,
	BGP_NOTIFY_CEASE_COLLISION_RESOLUTION);
	return -1;
	}
	if ((other->status != OpenConfirm) &&
	(other->status != OpenSent))
	return 0;
	/*
	* 1. The BGP Identifier of the local system is
	* compared to the BGP Identifier of the remote
	* system (as specified in the OPEN message).
	*
	* If the BGP Identifiers of the peers
	* involved in the connection collision
	* are identical, then the connection
	* initiated by the BGP speaker with the
	* larger AS number is preserved.
	*/
	if (ntohl(peer->local_id.s_addr) < ntohl(remote_id.s_addr)
	|| (ntohl(peer->local_id.s_addr) == ntohl(remote_id.s_addr)
	&& peer->local_as < peer->as))
	if (!CHECK_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER)) {
	/*
	* 2. If the value of the local BGP
	* Identifier is less than the remote one,
	* the local system closes BGP connection
	* that already exists (the one that is
	* already in the OpenConfirm state),
	* and accepts BGP connection initiated by
	* the remote system.
	*/
	bgp_notify_send(other, BGP_NOTIFY_CEASE,
	BGP_NOTIFY_CEASE_COLLISION_RESOLUTION);
	return 1;
	} else {
	bgp_notify_send(connection, BGP_NOTIFY_CEASE,
	BGP_NOTIFY_CEASE_COLLISION_RESOLUTION);
	return -1;
	}
	else {
	if (ntohl(peer->local_id.s_addr) == ntohl(remote_id.s_addr)
	&& peer->local_as == peer->as)
	flog_err(EC_BGP_ROUTER_ID_SAME,
	"Peer's router-id %pI4 is the same as ours",
	&remote_id);
	/*
	* 3. Otherwise, the local system closes newly
	* created BGP connection (the one associated with the
	* newly received OPEN message), and continues to use
	* the existing one (the one that is already in the
	* OpenConfirm state).
	*/
	if (CHECK_FLAG(peer->sflags, PEER_STATUS_ACCEPT_PEER)) {
	bgp_notify_send(other, BGP_NOTIFY_CEASE,
	BGP_NOTIFY_CEASE_COLLISION_RESOLUTION);
	return 1;
	} else {
	bgp_notify_send(connection, BGP_NOTIFY_CEASE,
	BGP_NOTIFY_CEASE_COLLISION_RESOLUTION);
	return -1;
	}
	}
	}

[ton31337]

I'm not sure I understood what the problem you are referring to. Could you elaborate?

When other->status ==Connect || Active, is it better to set the state to Idle?

Did you mean return -1 here that would call BGP_Stop and move to Idle state or what actually?

	if ((other->status != OpenConfirm) &&
	    (other->status != OpenSent))
		return 0;