From e7682ccd1b4929a7b2690b2c2f49ec9d4cf18875 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 1 Sep 2021 20:50:31 -0400 Subject: [PATCH 1/4] bgpd: Do not randomly generate a vrf id for -Z When FRR added the -Z parameter the bgp daemon was setting a vrf identifier based upon a number starting at 1. This caused issues when we upgraded the code to the outgoing sockets to use vrf_bind always. FRR should never just randomly select a vrf identifier. Let's just use VRF_DEFAULT when we are in a -Z environment. It's a safe bet. Fixes: #9519 Signed-off-by: Donald Sharp --- bgpd/bgpd.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 9004dff3b8..7236b9fe4b 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3409,8 +3409,21 @@ int bgp_get(struct bgp **bgp_val, as_t *as, const char *name, return ret; bgp = bgp_create(as, name, inst_type); - if (bgp_option_check(BGP_OPT_NO_ZEBRA) && name) - bgp->vrf_id = vrf_generate_id(); + + /* + * view instances will never work inside of a vrf + * as such they must always be in the VRF_DEFAULT + * Also we must set this to something useful because + * of the vrf socket code needing an actual useful + * default value to send to the underlying OS. + * + * This code is currently ignoring vrf based + * code using the -Z option( and that is probably + * best addressed elsewhere in the code ) + */ + if (inst_type == BGP_INSTANCE_TYPE_VIEW) + bgp->vrf_id = VRF_DEFAULT; + bgp_router_id_set(bgp, &bgp->router_id_zebra, true); bgp_address_init(bgp); bgp_tip_hash_init(bgp); From 7224dcd86a9e8bee0430801f16c137721b2fb8dc Mon Sep 17 00:00:00 2001 From: Igor Ryzhov Date: Thu, 2 Sep 2021 15:29:18 +0300 Subject: [PATCH 2/4] bgpd: fix bgp_get_bound_name to handle views better The vrf socket code needs a interface/vrf name to be passed in, in order for it to properly bind to the correct vrf. In the case where bgp is using a view based instance the bgp_get_bound_name should handle views better and not return anything to be bound to. Fixes #9519. Signed-off-by: Igor Ryzhov --- bgpd/bgp_network.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index 3c061ef1e0..7c9aa44c80 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -611,8 +611,6 @@ static int bgp_accept(struct thread *thread) /* BGP socket bind. */ static char *bgp_get_bound_name(struct peer *peer) { - char *name = NULL; - if (!peer) return NULL; @@ -628,14 +626,16 @@ static char *bgp_get_bound_name(struct peer *peer) * takes precedence over VRF. For IPv4 peering, explicit interface or * VRF are the situations to bind. */ - if (peer->su.sa.sa_family == AF_INET6) - name = (peer->conf_if ? peer->conf_if - : (peer->ifname ? peer->ifname - : peer->bgp->name)); - else - name = peer->ifname ? peer->ifname : peer->bgp->name; + if (peer->su.sa.sa_family == AF_INET6 && peer->conf_if) + return peer->conf_if; - return name; + if (peer->ifname) + return peer->ifname; + + if (peer->bgp->inst_type == BGP_INSTANCE_TYPE_VIEW) + return NULL; + + return peer->bgp->name; } static int bgp_update_address(struct interface *ifp, const union sockunion *dst, From 0e099318c8469319de72a0367c261fd6b30c1e98 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 1 Sep 2021 20:57:49 -0400 Subject: [PATCH 3/4] lib: Remove unused function vrf_generate_id Signed-off-by: Donald Sharp --- lib/vrf.c | 7 ------- lib/vrf.h | 1 - 2 files changed, 8 deletions(-) diff --git a/lib/vrf.c b/lib/vrf.c index 79313d66d9..f9307d3039 100644 --- a/lib/vrf.c +++ b/lib/vrf.c @@ -1068,13 +1068,6 @@ int vrf_sockunion_socket(const union sockunion *su, vrf_id_t vrf_id, return ret; } -vrf_id_t vrf_generate_id(void) -{ - static int vrf_id_local; - - return ++vrf_id_local; -} - /* ------- Northbound callbacks ------- */ /* diff --git a/lib/vrf.h b/lib/vrf.h index f8b0148eb8..9949ec4112 100644 --- a/lib/vrf.h +++ b/lib/vrf.h @@ -323,7 +323,6 @@ extern int vrf_netns_handler_create(struct vty *vty, struct vrf *vrf, extern void vrf_disable(struct vrf *vrf); extern int vrf_enable(struct vrf *vrf); extern void vrf_delete(struct vrf *vrf); -extern vrf_id_t vrf_generate_id(void); extern const struct frr_yang_module_info frr_vrf_info; From 888e727c9ec32da0579d93709fb2d7ead166cbcc Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 2 Sep 2021 08:53:19 -0400 Subject: [PATCH 4/4] bgpd: Add some debug events for when things go wrong As it stands there are cases where FRR is silently handling error events and not giving any log output to say what is going wrong. This should be fixed. Signed-off-by: Donald Sharp --- bgpd/bgp_network.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index 7c9aa44c80..3005eba271 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -706,7 +706,8 @@ int bgp_connect(struct peer *peer) ifindex_t ifindex = 0; if (peer->conf_if && BGP_PEER_SU_UNSPEC(peer)) { - zlog_debug("Peer address not learnt: Returning from connect"); + if (bgp_debug_neighbor_events(peer)) + zlog_debug("Peer address not learnt: Returning from connect"); return 0; } frr_with_privs(&bgpd_privs) { @@ -714,8 +715,13 @@ int bgp_connect(struct peer *peer) peer->fd = vrf_sockunion_socket(&peer->su, peer->bgp->vrf_id, bgp_get_bound_name(peer)); } - if (peer->fd < 0) + if (peer->fd < 0) { + if (bgp_debug_neighbor_events(peer)) + zlog_debug("%s: Failure to create socket for connection to %s, error received: %s(%d)", + __func__, peer->host, safe_strerror(errno), + errno); return -1; + } set_nonblocking(peer->fd); @@ -725,8 +731,13 @@ int bgp_connect(struct peer *peer) bgp_socket_set_buffer_size(peer->fd); - if (bgp_set_socket_ttl(peer, peer->fd) < 0) + if (bgp_set_socket_ttl(peer, peer->fd) < 0) { + if (bgp_debug_neighbor_events(peer)) + zlog_debug("%s: Failure to set socket ttl for connection to %s, error received: %s(%d)", + __func__, peer->host, safe_strerror(errno), + errno); return -1; + } sockopt_reuseaddr(peer->fd); sockopt_reuseport(peer->fd);