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 <donatas@opensourcerouting.org>

diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c
index 764b6b87162799bbdbb412c16a6f0e9fd83b5a4e..607e34e8d3f9b2860a3eee821d2c8e9087d8b26a 100644 (file)
--- 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 4bcb41b8dc23048b58634fdbdf744dba58432779..1f4f4d52a20caebc409faede4480888722e44b9d 100644 (file)
--- 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 ed689c8bac0878f99bb5e8cb3bc6d71028941918..db7580c283035850c5e2cab486fc83470d2192b3 100644 (file)
--- 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 84ff88be16f6e63b0856c607784f914d3fa08077..b953da57ce0bfd8fdff0f4d774cbe14537a100b5 100644 (file)
--- 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 6a56eb45987b215a8bc5755db8a0f21c412e73ff..b3cc86ed276b4b4180a98ebd072bf026bb8ae096 100644 (file)
--- 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 9b13270ca7048df3494de955e51d677bafe63a36..25f533631c90af044135de96fe333af1bf7d9e8c 100644 (file)
--- 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);