From b17fd55d831a9c02010b04bd74b475726936d9b8 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 29 Jan 2025 23:03:06 +0200 Subject: bgpd: Do not start BGP session if BGP identifier is not set If we have IPv6-only network and no IPv4 addresses at all, then by default 0.0.0.0 is created which is treated as malformed according to RFC 6286. Signed-off-by: Donatas Abraitis --- bgpd/bgp_fsm.c | 1 + bgpd/bgp_network.c | 17 ++++++++++++++++- bgpd/bgpd.h | 1 + 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index 7605501754..b07afaf384 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -603,6 +603,7 @@ const char *const peer_down_str[] = { "Socket Error", "Admin. shutdown (RTT)", "Suppress Fib Turned On or Off", + "Router ID is missing", }; static void bgp_graceful_restart_timer_off(struct peer_connection *connection, diff --git a/bgpd/bgp_network.c b/bgpd/bgp_network.c index e09dbc22af..a4be725937 100644 --- a/bgpd/bgp_network.c +++ b/bgpd/bgp_network.c @@ -568,7 +568,7 @@ static void bgp_accept(struct event *thread) /* Do not try to reconnect if the peer reached maximum * prefixes, restart timer is still running or the peer - * is shutdown. + * is shutdown, or BGP identifier is not set (0.0.0.0). */ if (BGP_PEER_START_SUPPRESSED(peer1)) { if (bgp_debug_neighbor_events(peer1)) { @@ -585,6 +585,14 @@ static void bgp_accept(struct event *thread) return; } + if (peer1->bgp->router_id.s_addr == INADDR_ANY) { + zlog_warn("[Event] Incoming BGP connection rejected from %s due missing BGP identifier, set it with `bgp router-id`", + peer1->host); + peer1->last_reset = PEER_DOWN_ROUTER_ID_ZERO; + close(bgp_sock); + return; + } + if (bgp_debug_neighbor_events(peer1)) zlog_debug("[Event] connection from %s fd %d, active peer status %d fd %d", inet_sutop(&su, buf), bgp_sock, connection1->status, @@ -770,6 +778,13 @@ int bgp_connect(struct peer_connection *connection) assert(!CHECK_FLAG(connection->thread_flags, PEER_THREAD_READS_ON)); ifindex_t ifindex = 0; + if (peer->bgp->router_id.s_addr == INADDR_ANY) { + peer->last_reset = PEER_DOWN_ROUTER_ID_ZERO; + zlog_warn("%s: BGP identifier is missing for peer %s, set it with `bgp router-id`", + __func__, peer->host); + return connect_error; + } + if (peer->conf_if && BGP_CONNECTION_SU_UNSPEC(connection)) { if (bgp_debug_neighbor_events(peer)) zlog_debug("Peer address not learnt: Returning from connect"); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 61907140d9..a565a323b5 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1801,6 +1801,7 @@ struct peer { #define PEER_DOWN_SOCKET_ERROR 34U /* Some socket error happened */ #define PEER_DOWN_RTT_SHUTDOWN 35U /* Automatically shutdown due to RTT */ #define PEER_DOWN_SUPPRESS_FIB_PENDING 36U /* Suppress fib pending changed */ +#define PEER_DOWN_ROUTER_ID_ZERO 37U /* router-id is 0.0.0.0 */ /* * Remember to update peer_down_str in bgp_fsm.c when you add * a new value to the last_reset reason -- cgit v1.2.3 From 9075eaa07f523a753a061c7d94c401c6ba089d14 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Mon, 3 Feb 2025 14:49:53 +0100 Subject: bgpd: fix add label support to EVPN AD routes When peering with an EVPN device from other vendor, FRR acting as route reflector is not able to read nor transmit the label value. Actually, EVPN AD routes completely ignore the label value in the code, whereas in some functionalities like evpn-vpws, it is authorised to carry and propagate label value. Fix this by handling the label value. Signed-off-by: Philippe Guibert --- bgpd/bgp_evpn_mh.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_evpn_mh.c b/bgpd/bgp_evpn_mh.c index aa28b6f3d8..375687fe0f 100644 --- a/bgpd/bgp_evpn_mh.c +++ b/bgpd/bgp_evpn_mh.c @@ -1202,6 +1202,7 @@ int bgp_evpn_type1_route_process(struct peer *peer, afi_t afi, safi_t safi, mpls_label_t label; struct in_addr vtep_ip; struct prefix_evpn p; + uint8_t num_labels = 0; if (psize != BGP_EVPN_TYPE1_PSIZE) { flog_err(EC_BGP_EVPN_ROUTE_INVALID, @@ -1226,6 +1227,7 @@ int bgp_evpn_type1_route_process(struct peer *peer, afi_t afi, safi_t safi, pfx += EVPN_ETH_TAG_BYTES; memcpy(&label, pfx, BGP_LABEL_BYTES); + num_labels++; /* EAD route prefix doesn't include the nexthop in the global * table @@ -1234,13 +1236,11 @@ int bgp_evpn_type1_route_process(struct peer *peer, afi_t afi, safi_t safi, build_evpn_type1_prefix(&p, eth_tag, &esi, vtep_ip); /* Process the route. */ if (attr) { - bgp_update(peer, (struct prefix *)&p, addpath_id, attr, afi, - safi, ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, NULL, - 0, 0, NULL); + bgp_update(peer, (struct prefix *)&p, addpath_id, attr, afi, safi, ZEBRA_ROUTE_BGP, + BGP_ROUTE_NORMAL, &prd, &label, num_labels, 0, NULL); } else { - bgp_withdraw(peer, (struct prefix *)&p, addpath_id, afi, safi, - ZEBRA_ROUTE_BGP, BGP_ROUTE_NORMAL, &prd, NULL, 0, - NULL); + bgp_withdraw(peer, (struct prefix *)&p, addpath_id, afi, safi, ZEBRA_ROUTE_BGP, + BGP_ROUTE_NORMAL, &prd, &label, num_labels, NULL); } return 0; } -- cgit v1.2.3 From daa68852a2a78acf103e8ae1127953b2870c6772 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 5 Feb 2025 14:13:01 +0200 Subject: Revert "bgpd: fix duplicate BGP instance created with unified config" This reverts commit aba588dd09aa098a88ba1355798c0e784e91ebc8. --- bgpd/bgpd.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 7c1be226b1..a984c5af87 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3623,13 +3623,13 @@ struct bgp *bgp_lookup(as_t as, const char *name) } /* Lookup BGP structure by view name. */ -struct bgp *bgp_lookup_by_name_filter(const char *name, bool filter_auto) +struct bgp *bgp_lookup_by_name(const char *name) { struct bgp *bgp; struct listnode *node, *nnode; for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) { - if (filter_auto && CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO)) + if (CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO)) continue; if ((bgp->name == NULL && name == NULL) || (bgp->name && name && strcmp(bgp->name, name) == 0)) @@ -3638,11 +3638,6 @@ struct bgp *bgp_lookup_by_name_filter(const char *name, bool filter_auto) return NULL; } -struct bgp *bgp_lookup_by_name(const char *name) -{ - return bgp_lookup_by_name_filter(name, true); -} - /* Lookup BGP instance based on VRF id. */ /* Note: Only to be used for incoming messages from Zebra. */ struct bgp *bgp_lookup_by_vrf_id(vrf_id_t vrf_id) @@ -3735,7 +3730,7 @@ int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as, const char *name, /* Multiple instance check. */ if (name) - bgp = bgp_lookup_by_name_filter(name, false); + bgp = bgp_lookup_by_name(name); else bgp = bgp_get_default(); -- cgit v1.2.3 From 3abd84ef5be1ef56b66f0e7617f8afab6da6c5cc Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Wed, 5 Feb 2025 14:15:51 +0200 Subject: bgpd: fix duplicate BGP instance created with unified config When running the bgp_evpn_rt5 setup with unified config, memory leak about a non deleted BGP instance happens. > root@ubuntu2204hwe:~/frr/tests/topotests/bgp_evpn_rt5# cat /tmp/topotests/bgp_evpn_rt5.test_bgp_evpn/r1.asan.bgpd.1164105 > > ================================================================= > ==1164105==ERROR: LeakSanitizer: detected memory leaks > > Indirect leak of 12496 byte(s) in 1 object(s) allocated from: > #0 0x7f358eeb4a57 in __interceptor_calloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:154 > #1 0x7f358e877233 in qcalloc lib/memory.c:106 > #2 0x55d06c95680a in bgp_create bgpd/bgpd.c:3405 > #3 0x55d06c95a7b3 in bgp_get bgpd/bgpd.c:3805 > #4 0x55d06c87a9b5 in bgp_get_vty bgpd/bgp_vty.c:603 > #5 0x55d06c68dc71 in bgp_evpn_local_l3vni_add bgpd/bgp_evpn.c:7032 > #6 0x55d06c92989b in bgp_zebra_process_local_l3vni bgpd/bgp_zebra.c:3204 > #7 0x7f358e9e3feb in zclient_read lib/zclient.c:4626 > #8 0x7f358e98082d in event_call lib/event.c:1996 > #9 0x7f358e848931 in frr_run lib/libfrr.c:1232 > #10 0x55d06c60eae1 in main bgpd/bgp_main.c:557 > #11 0x7f358e229d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58 Actually, a BGP VRF Instance is created in auto mode when creating the global BGP instance for the L3 VNI. And again, an other BGP VRF instance is created. Fix this by ensuring that a non existing BGP instance is not present. If it is present, and with auto mode or in hidden mode, then override the AS value. Fixes: f153b9a9b636 ("bgpd: Ignore auto created VRF BGP instances") Signed-off-by: Philippe Guibert Signed-off-by: Donatas Abraitis --- bgpd/bgpd.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index a984c5af87..17a34dbacb 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3623,13 +3623,13 @@ struct bgp *bgp_lookup(as_t as, const char *name) } /* Lookup BGP structure by view name. */ -struct bgp *bgp_lookup_by_name(const char *name) +struct bgp *bgp_lookup_by_name_filter(const char *name, bool filter_auto) { struct bgp *bgp; struct listnode *node, *nnode; for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) { - if (CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO)) + if (filter_auto && CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO)) continue; if ((bgp->name == NULL && name == NULL) || (bgp->name && name && strcmp(bgp->name, name) == 0)) @@ -3638,6 +3638,11 @@ struct bgp *bgp_lookup_by_name(const char *name) return NULL; } +struct bgp *bgp_lookup_by_name(const char *name) +{ + return bgp_lookup_by_name_filter(name, true); +} + /* Lookup BGP instance based on VRF id. */ /* Note: Only to be used for incoming messages from Zebra. */ struct bgp *bgp_lookup_by_vrf_id(vrf_id_t vrf_id) -- cgit v1.2.3