From ace4b8fe61df2bd8e0819b1624efa90e24f6452c Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Mon, 17 Mar 2025 14:52:42 +0200 Subject: [PATCH] bgpd: Print the real reason why the peer is not accepted (incoming) If it's suppressed due to BFD down or unspecified connection, we never know the real reason and just say "no AF activated" which is misleading. Signed-off-by: Donatas Abraitis --- bgpd/bgp_fsm.c | 2 +- bgpd/bgp_network.c | 29 ++++++++++++++++++++++++----- bgpd/bgp_nexthop.c | 2 +- bgpd/bgp_zebra.c | 2 +- bgpd/bgpd.c | 34 ++++++++++++++++++---------------- bgpd/bgpd.h | 10 +++++++++- 6 files changed, 54 insertions(+), 25 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 764b6b8716..607e34e8d3 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -325,7 +325,7 @@ void bgp_timer_set(struct peer_connection *connection) /* First entry point of peer's finite state machine. In Idle status start timer is on unless peer is shutdown or peer is inactive. All other timer must be turned off */ - if (BGP_PEER_START_SUPPRESSED(peer) || !peer_active(connection) || + if (BGP_PEER_START_SUPPRESSED(peer) || peer_active(connection) != BGP_PEER_ACTIVE || peer->bgp->vrf_id == VRF_UNKNOWN) { EVENT_OFF(connection->t_start); } else { diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index 4bcb41b8dc..1f4f4d52a2 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -389,6 +389,23 @@ static void bgp_socket_set_buffer_size(const int fd) setsockopt_so_recvbuf(fd, bm->socket_buffer); } +static const char *bgp_peer_active2str(enum bgp_peer_active active) +{ + switch (active) { + case BGP_PEER_ACTIVE: + return "active"; + case BGP_PEER_CONNECTION_UNSPECIFIED: + return "unspecified connection"; + case BGP_PEER_BFD_DOWN: + return "BFD down"; + case BGP_PEER_AF_UNCONFIGURED: + return "no AF activated"; + } + + assert(!"We should never get here this is a dev escape"); + return "ERROR"; +} + /* Accept bgp connection. */ static void bgp_accept(struct event *thread) { @@ -400,6 +417,7 @@ static void bgp_accept(struct event *thread) struct peer_connection *connection, *incoming; char buf[SU_ADDRSTRLEN]; struct bgp *bgp = NULL; + enum bgp_peer_active active; sockunion_init(&su); @@ -508,7 +526,7 @@ static void bgp_accept(struct event *thread) bgp_fsm_change_status(incoming, Active); EVENT_OFF(incoming->t_start); - if (peer_active(incoming)) { + if (peer_active(incoming) == BGP_PEER_ACTIVE) { if (CHECK_FLAG(dynamic_peer->flags, PEER_FLAG_TIMER_DELAYOPEN)) BGP_EVENT_ADD(incoming, TCP_connection_open_w_delay); else @@ -559,10 +577,11 @@ static void bgp_accept(struct event *thread) } /* Check that at least one AF is activated for the peer. */ - if (!peer_active(connection)) { + active = peer_active(connection); + if (active != BGP_PEER_ACTIVE) { if (bgp_debug_neighbor_events(peer)) - zlog_debug("%s - incoming conn rejected - no AF activated for peer", - peer->host); + zlog_debug("%s - incoming conn rejected - %s", peer->host, + bgp_peer_active2str(active)); close(bgp_sock); return; } @@ -662,7 +681,7 @@ static void bgp_accept(struct event *thread) bgp_event_update(connection, TCP_connection_closed); } - if (peer_active(incoming)) { + if (peer_active(incoming) == BGP_PEER_ACTIVE) { if (CHECK_FLAG(doppelganger->flags, PEER_FLAG_TIMER_DELAYOPEN)) BGP_EVENT_ADD(incoming, TCP_connection_open_w_delay); else diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c index ed689c8bac..db7580c283 100644 --- a/bgpd/bgp_nexthop.c +++ b/bgpd/bgp_nexthop.c @@ -444,7 +444,7 @@ void bgp_connected_add(struct bgp *bgp, struct connected *ifc) !peer_established(peer->connection) && !CHECK_FLAG(peer->flags, PEER_FLAG_IFPEER_V6ONLY)) { connection = peer->connection; - if (peer_active(connection)) + if (peer_active(connection) == BGP_PEER_ACTIVE) BGP_EVENT_ADD(connection, BGP_Stop); BGP_EVENT_ADD(connection, BGP_Start); } diff --git a/bgpd/bgp_zebra.c b/bgpd/bgp_zebra.c index 84ff88be16..b953da57ce 100644 --- a/bgpd/bgp_zebra.c +++ b/bgpd/bgp_zebra.c @@ -137,7 +137,7 @@ static void bgp_start_interface_nbrs(struct bgp *bgp, struct interface *ifp) for (ALL_LIST_ELEMENTS(bgp->peer, node, nnode, peer)) { if (peer->conf_if && (strcmp(peer->conf_if, ifp->name) == 0) && !peer_established(peer->connection)) { - if (peer_active(peer->connection)) + if (peer_active(peer->connection) == BGP_PEER_ACTIVE) BGP_EVENT_ADD(peer->connection, BGP_Stop); BGP_EVENT_ADD(peer->connection, BGP_Start); } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 6a56eb4598..b3cc86ed27 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1960,7 +1960,7 @@ struct peer *peer_create(union sockunion *su, const char *conf_if, enum peer_asn_type as_type, struct peer_group *group, bool config_node, const char *as_str) { - int active; + enum bgp_peer_active active; struct peer *peer; char buf[SU_ADDRSTRLEN]; afi_t afi; @@ -2014,7 +2014,7 @@ struct peer *peer_create(union sockunion *su, const char *conf_if, } active = peer_active(peer->connection); - if (!active) { + if (active != BGP_PEER_ACTIVE) { if (peer->connection->su.sa.sa_family == AF_UNSPEC) peer->last_reset = PEER_DOWN_NBR_ADDR; else @@ -2046,7 +2046,7 @@ struct peer *peer_create(union sockunion *su, const char *conf_if, if (bgp->autoshutdown) peer_flag_set(peer, PEER_FLAG_SHUTDOWN); /* Set up peer's events and timers. */ - else if (!active && peer_active(peer->connection)) { + else if (active != BGP_PEER_ACTIVE && peer_active(peer->connection) == BGP_PEER_ACTIVE) { if (peer->last_reset == PEER_DOWN_NOAFI_ACTIVATED) peer->last_reset = 0; bgp_timer_set(peer->connection); @@ -2419,7 +2419,7 @@ static void peer_group2peer_config_copy_af(struct peer_group *group, static int peer_activate_af(struct peer *peer, afi_t afi, safi_t safi) { - int active; + enum bgp_peer_active active; struct peer *other; if (CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { @@ -2447,7 +2447,7 @@ static int peer_activate_af(struct peer *peer, afi_t afi, safi_t safi) if (peer->group) peer_group2peer_config_copy_af(peer->group, peer, afi, safi); - if (!active && peer_active(peer->connection)) { + if (active != BGP_PEER_ACTIVE && peer_active(peer->connection) == BGP_PEER_ACTIVE) { bgp_timer_set(peer->connection); } else { peer->last_reset = PEER_DOWN_AF_ACTIVATE; @@ -3391,7 +3391,7 @@ int peer_group_bind(struct bgp *bgp, union sockunion *su, struct peer *peer, } /* Set up peer's events and timers. */ - if (peer_active(peer->connection)) + if (peer_active(peer->connection) == BGP_PEER_ACTIVE) bgp_timer_set(peer->connection); } @@ -4709,16 +4709,16 @@ bool bgp_path_attribute_treat_as_withdraw(struct peer *peer, char *buf, } /* If peer is configured at least one address family return 1. */ -bool peer_active(struct peer_connection *connection) +enum bgp_peer_active peer_active(struct peer_connection *connection) { struct peer *peer = connection->peer; if (BGP_CONNECTION_SU_UNSPEC(connection)) - return false; + return BGP_PEER_CONNECTION_UNSPECIFIED; if (peer->bfd_config) { if (bfd_session_is_down(peer->bfd_config->session)) - return false; + return BGP_PEER_BFD_DOWN; } if (peer->afc[AFI_IP][SAFI_UNICAST] || peer->afc[AFI_IP][SAFI_MULTICAST] @@ -4732,8 +4732,9 @@ bool peer_active(struct peer_connection *connection) || peer->afc[AFI_IP6][SAFI_ENCAP] || peer->afc[AFI_IP6][SAFI_FLOWSPEC] || peer->afc[AFI_L2VPN][SAFI_EVPN]) - return true; - return false; + return BGP_PEER_ACTIVE; + + return BGP_PEER_AF_UNCONFIGURED; } /* If peer is negotiated at least one address family return 1. */ @@ -6417,7 +6418,7 @@ int peer_timers_connect_set(struct peer *peer, uint32_t connect) /* Skip peer-group mechanics for regular peers. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { if (!peer_established(peer->connection)) { - if (peer_active(peer->connection)) + if (peer_active(peer->connection) == BGP_PEER_ACTIVE) BGP_EVENT_ADD(peer->connection, BGP_Stop); BGP_EVENT_ADD(peer->connection, BGP_Start); } @@ -6438,7 +6439,7 @@ int peer_timers_connect_set(struct peer *peer, uint32_t connect) member->v_connect = connect; if (!peer_established(member->connection)) { - if (peer_active(member->connection)) + if (peer_active(member->connection) == BGP_PEER_ACTIVE) BGP_EVENT_ADD(member->connection, BGP_Stop); BGP_EVENT_ADD(member->connection, BGP_Start); } @@ -6471,7 +6472,7 @@ int peer_timers_connect_unset(struct peer *peer) /* Skip peer-group mechanics for regular peers. */ if (!CHECK_FLAG(peer->sflags, PEER_STATUS_GROUP)) { if (!peer_established(peer->connection)) { - if (peer_active(peer->connection)) + if (peer_active(peer->connection) == BGP_PEER_ACTIVE) BGP_EVENT_ADD(peer->connection, BGP_Stop); BGP_EVENT_ADD(peer->connection, BGP_Start); } @@ -6492,7 +6493,7 @@ int peer_timers_connect_unset(struct peer *peer) member->v_connect = peer->bgp->default_connect_retry; if (!peer_established(member->connection)) { - if (peer_active(member->connection)) + if (peer_active(member->connection) == BGP_PEER_ACTIVE) BGP_EVENT_ADD(member->connection, BGP_Stop); BGP_EVENT_ADD(member->connection, BGP_Start); } @@ -8771,7 +8772,8 @@ static int peer_unshut_after_cfg(struct bgp *bgp) peer->host); peer->shut_during_cfg = false; - if (peer_active(peer->connection) && peer->connection->status != Established) { + if (peer_active(peer->connection) == BGP_PEER_ACTIVE && + peer->connection->status != Established) { if (peer->connection->status != Idle) BGP_EVENT_ADD(peer->connection, BGP_Stop); BGP_EVENT_ADD(peer->connection, BGP_Start); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 9b13270ca7..25f533631c 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -2299,6 +2299,14 @@ enum bgp_martian_type { BGP_MARTIAN_SOO, /* bgp->evpn_info->macvrf_soo */ }; +/* Distinguish the reason why the peer is not active. */ +enum bgp_peer_active { + BGP_PEER_ACTIVE, + BGP_PEER_CONNECTION_UNSPECIFIED, + BGP_PEER_BFD_DOWN, + BGP_PEER_AF_UNCONFIGURED, +}; + extern const struct message bgp_martian_type_str[]; extern const char *bgp_martian_type2str(enum bgp_martian_type mt); @@ -2347,7 +2355,7 @@ extern struct peer *peer_unlock_with_caller(const char *, struct peer *); extern enum bgp_peer_sort peer_sort(struct peer *peer); extern enum bgp_peer_sort peer_sort_lookup(struct peer *peer); -extern bool peer_active(struct peer_connection *connection); +extern enum bgp_peer_active peer_active(struct peer_connection *connection); extern bool peer_active_nego(struct peer *); extern bool peer_afc_received(struct peer *peer); extern bool peer_afc_advertised(struct peer *peer);