bgpd: Add a better breadcrumb for when bgp is missconfiged

Currently During bgp open collision resolution if both
the router-id's are the same, we correctly follow
the RFC and close the connection.  The problem is of course
that there is no notification of the error in configuration
to the end user other than a subtle open debug message.

Explicitly call out the miss-configuration as an error message
as that this miss-config took several hours of debugging to notice.

Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>

diff --git a/bgpd/bgp_errors.c b/bgpd/bgp_errors.c
index d9aba87e35ceda8d0f37eeb4f5738a6437f8ccc7..8a33ce67894497691720b1b94616cf9841338f14 100644 (file)
--- a/bgpd/bgp_errors.c
+++ b/bgpd/bgp_errors.c
@@ -456,6 +456,12 @@ static struct log_ref ferr_bgp_err[] = {
                .description = "As part of BGP startup, the peer and ourselves can start connections to each other at the same time. During this process BGP received additional configuration, but it was only applied to one of the two nascent connections. Depending on the result of collision detection and resolution this configuration might be lost.  To remedy this, after performing collision detection and resolution the peer session has been reset in order to apply the new configuration.",
                .suggestion = "Gather data and open a Issue so that this developmental escape can be fixed, the peer should have been reset",
        },
+       {
+               .code = EC_BGP_ROUTER_ID_SAME,
+               .title = "BGP has detected a duplicate router id during collision resolution",
+               .description = "As part of normal collision detection for opening a connection to a peer, BGP has detected that the remote peer's router-id is the same as ours",
+               .suggestion = "Change one of the two router-id's",
+       },
        {
                .code = END_FERR,
        }

diff --git a/bgpd/bgp_errors.h b/bgpd/bgp_errors.h
index 35c5cc3998eabdb30e57fc69ec13f2f568ec4e3a..49c58ae6b0f93f275ec5cf09e2c61e6c2e0675f0 100644 (file)
--- a/bgpd/bgp_errors.h
+++ b/bgpd/bgp_errors.h
@@ -98,6 +98,7 @@ enum bgp_log_refs {
        EC_BGP_CAPABILITY_UNKNOWN,
        EC_BGP_INVALID_NEXTHOP_LENGTH,
        EC_BGP_DOPPELGANGER_CONFIG,
+       EC_BGP_ROUTER_ID_SAME,
 };
 
 extern void bgp_error_init(void);

diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index c72f7226e2f397c8a79526ee7f20afc629bdd6d0..9d030378c74cfd10a1e6186185e185301ceb25f4 100644 (file)
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -1007,6 +1007,11 @@ static int bgp_collision_detect(struct peer *new, struct in_addr remote_id)
                                        return -1;
                                }
                        else {
+                               if (ntohl(peer->local_id.s_addr) ==
+                                   ntohl(remote_id.s_addr))
+                                       flog_err(EC_BGP_ROUTER_ID_SAME, "Peer's router-id %s is the same as ours",
+                                                inet_ntoa(remote_id));
+
                                /* 3. Otherwise, the local system closes newly
                                   created
                                   BGP connection (the one associated with the