Skip to content

Commit 9f7177a

Browse files
pguibert6WINDpguibert6WIND
committed
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 > FRRouting#4 0x55d06c87a9b5 in bgp_get_vty bgpd/bgp_vty.c:603 > FRRouting#5 0x55d06c68dc71 in bgp_evpn_local_l3vni_add bgpd/bgp_evpn.c:7032 > FRRouting#6 0x55d06c92989b in bgp_zebra_process_local_l3vni bgpd/bgp_zebra.c:3204 > FRRouting#7 0x7f358e9e3feb in zclient_read lib/zclient.c:4626 > FRRouting#8 0x7f358e98082d in event_call lib/event.c:1996 > FRRouting#9 0x7f358e848931 in frr_run lib/libfrr.c:1232 > FRRouting#10 0x55d06c60eae1 in main bgpd/bgp_main.c:557 > FRRouting#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: f153b9a ("bgpd: Ignore auto created VRF BGP instances") Signed-off-by: Philippe Guibert <[email protected]>
1 parent 6855bf2 commit 9f7177a

File tree

3 files changed

+27
-23
lines changed

3 files changed

+27
-23
lines changed

‎bgpd/bgp_vty.c

+10-8
Original file line numberDiff line numberDiff line change
@@ -1499,13 +1499,12 @@ DEFUN_NOSH (router_bgp,
14991499
int idx_asn = 2;
15001500
int idx_view_vrf = 3;
15011501
int idx_vrf = 4;
1502-
int is_new_bgp = 0;
15031502
int idx_asnotation = 3;
15041503
int idx_asnotation_kind = 4;
15051504
enum asnotation_mode asnotation = ASNOTATION_UNDEFINED;
15061505
int ret;
15071506
as_t as;
1508-
struct bgp *bgp;
1507+
struct bgp *bgp = NULL;
15091508
const char *name = NULL;
15101509
enum bgp_instance_type inst_type;
15111510

@@ -1567,11 +1566,14 @@ DEFUN_NOSH (router_bgp,
15671566
asnotation = ASNOTATION_PLAIN;
15681567
}
15691568

1570-
if (inst_type == BGP_INSTANCE_TYPE_DEFAULT)
1571-
is_new_bgp = (bgp_lookup(as, name) == NULL);
1572-
1573-
ret = bgp_get_vty(&bgp, &as, name, inst_type,
1574-
argv[idx_asn]->arg, asnotation);
1569+
ret = bgp_lookup_by_as_name_type(&bgp, &as, argv[idx_asn]->arg, asnotation, name,
1570+
inst_type, true);
1571+
if (bgp && ret == BGP_INSTANCE_EXISTS)
1572+
ret = CMD_SUCCESS;
1573+
else if (bgp == NULL && ret == CMD_SUCCESS)
1574+
/* SUCCESS and bgp is NULL */
1575+
ret = bgp_get_vty(&bgp, &as, name, inst_type, argv[idx_asn]->arg,
1576+
asnotation);
15751577
switch (ret) {
15761578
case BGP_ERR_AS_MISMATCH:
15771579
vty_out(vty, "BGP is already running; AS is %s\n",
@@ -1591,7 +1593,7 @@ DEFUN_NOSH (router_bgp,
15911593
* any pending VRF-VPN leaking that was configured via
15921594
* earlier "router bgp X vrf FOO" blocks.
15931595
*/
1594-
if (is_new_bgp && inst_type == BGP_INSTANCE_TYPE_DEFAULT)
1596+
if (bgp && inst_type == BGP_INSTANCE_TYPE_DEFAULT)
15951597
vpn_leak_postchange_all();
15961598

15971599
if (inst_type == BGP_INSTANCE_TYPE_VRF ||

‎bgpd/bgpd.c

+14-10
Original file line numberDiff line numberDiff line change
@@ -3634,13 +3634,13 @@ struct bgp *bgp_lookup(as_t as, const char *name)
36343634
}
36353635

36363636
/* Lookup BGP structure by view name. */
3637-
struct bgp *bgp_lookup_by_name(const char *name)
3637+
static struct bgp *bgp_lookup_by_name_filter(const char *name, bool filter_auto)
36383638
{
36393639
struct bgp *bgp;
36403640
struct listnode *node, *nnode;
36413641

36423642
for (ALL_LIST_ELEMENTS(bm->bgp, node, nnode, bgp)) {
3643-
if (CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO))
3643+
if (filter_auto && CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO))
36443644
continue;
36453645
if ((bgp->name == NULL && name == NULL)
36463646
|| (bgp->name && name && strcmp(bgp->name, name) == 0))
@@ -3649,6 +3649,11 @@ struct bgp *bgp_lookup_by_name(const char *name)
36493649
return NULL;
36503650
}
36513651

3652+
struct bgp *bgp_lookup_by_name(const char *name)
3653+
{
3654+
return bgp_lookup_by_name_filter(name, true);
3655+
}
3656+
36523657
/* Lookup BGP instance based on VRF id. */
36533658
/* Note: Only to be used for incoming messages from Zebra. */
36543659
struct bgp *bgp_lookup_by_vrf_id(vrf_id_t vrf_id)
@@ -3734,10 +3739,9 @@ int bgp_handle_socket(struct bgp *bgp, struct vrf *vrf, vrf_id_t old_vrf_id,
37343739
return bgp_check_main_socket(create, bgp);
37353740
}
37363741

3737-
int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as,
3738-
const char *as_pretty,
3742+
int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as, const char *as_pretty,
37393743
enum asnotation_mode asnotation, const char *name,
3740-
enum bgp_instance_type inst_type)
3744+
enum bgp_instance_type inst_type, bool force_config)
37413745
{
37423746
struct bgp *bgp;
37433747
struct peer *peer = NULL;
@@ -3746,7 +3750,7 @@ int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as,
37463750

37473751
/* Multiple instance check. */
37483752
if (name)
3749-
bgp = bgp_lookup_by_name(name);
3753+
bgp = bgp_lookup_by_name_filter(name, !force_config);
37503754
else
37513755
bgp = bgp_get_default();
37523756

@@ -3756,15 +3760,16 @@ int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as,
37563760
/* Handle AS number change */
37573761
if (bgp->as != *as) {
37583762
if (hidden || CHECK_FLAG(bgp->vrf_flags, BGP_VRF_AUTO)) {
3759-
if (hidden) {
3763+
if (force_config == false && hidden) {
37603764
bgp_create(as, name, inst_type,
37613765
as_pretty, asnotation, bgp,
37623766
hidden);
37633767
UNSET_FLAG(bgp->flags,
37643768
BGP_FLAG_INSTANCE_HIDDEN);
37653769
} else {
37663770
bgp->as = *as;
3767-
UNSET_FLAG(bgp->vrf_flags, BGP_VRF_AUTO);
3771+
if (force_config == false)
3772+
UNSET_FLAG(bgp->vrf_flags, BGP_VRF_AUTO);
37683773
}
37693774

37703775
/* Set all peer's local AS with this ASN */
@@ -3801,8 +3806,7 @@ int bgp_get(struct bgp **bgp_val, as_t *as, const char *name,
38013806
struct vrf *vrf = NULL;
38023807
int ret = 0;
38033808

3804-
ret = bgp_lookup_by_as_name_type(bgp_val, as, as_pretty, asnotation,
3805-
name, inst_type);
3809+
ret = bgp_lookup_by_as_name_type(bgp_val, as, as_pretty, asnotation, name, inst_type, false);
38063810
if (ret || *bgp_val)
38073811
return ret;
38083812

‎bgpd/bgpd.h

+3-5
Original file line numberDiff line numberDiff line change
@@ -2859,11 +2859,9 @@ extern struct peer *peer_new(struct bgp *bgp);
28592859

28602860
extern struct peer *peer_lookup_in_view(struct vty *vty, struct bgp *bgp,
28612861
const char *ip_str, bool use_json);
2862-
extern int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as,
2863-
const char *as_pretty,
2864-
enum asnotation_mode asnotation,
2865-
const char *name,
2866-
enum bgp_instance_type inst_type);
2862+
extern int bgp_lookup_by_as_name_type(struct bgp **bgp_val, as_t *as, const char *as_pretty,
2863+
enum asnotation_mode asnotation, const char *name,
2864+
enum bgp_instance_type inst_type, bool force_config);
28672865

28682866
/* Hooks */
28692867
DECLARE_HOOK(bgp_vrf_status_changed, (struct bgp *bgp, struct interface *ifp),

0 commit comments

Comments
 (0)