bgpd: fixed failing remove of vrf if there is a stale l3vni

Problem statement:
==================
When a vrf is deleted from the kernel, before its removed from the FRR
config, zebra gets to delete the the vrf and assiciated state.

It does so by sending a request to delete the l3 vni associated with the
vrf followed by a request to delete the vrf itself.

2023/10/06 06:22:18 ZEBRA: [JAESH-BABB8] Send L3_VNI_DEL 1001 VRF
testVRF1001 to bgp
2023/10/06 06:22:18 ZEBRA: [XC3P3-1DG4D] MESSAGE: ZEBRA_VRF_DELETE
testVRF1001

The zebra client communication is asynchronous and about 1/5 cases the
bgp client process them in a different order.

2023/10/06 06:22:18 BGP: [VP18N-HB5R6] VRF testVRF1001(766) is to be
deleted.
2023/10/06 06:22:18 BGP: [RH4KQ-X3CYT] VRF testVRF1001(766) is to be
disabled.
2023/10/06 06:22:18 BGP: [X8ZE0-9TS5H] VRF disable testVRF1001 id 766
2023/10/06 06:22:18 BGP: [X67AQ-923PR] Deregistering VRF 766
2023/10/06 06:22:18 BGP: [K52W0-YZ4T8] VRF Deletion:
testVRF1001(4294967295)
.. and a bit later :
2023/10/06 06:22:18 BGP: [MRXGD-9MHNX] DJERNAES: process L3VNI 1001 DEL
2023/10/06 06:22:18 BGP: [NCEPE-BKB1G][EC 33554467] Cannot process L3VNI
1001 Del - Could not find BGP instance

When the bgp vrf config is removed later it fails on the sanity check if
l3vni is removed.

        if (bgp->l3vni) {
            vty_out(vty, "%% Please unconfigure l3vni %u\n",
                bgp->l3vni);
            return CMD_WARNING_CONFIG_FAILED;
        }

Solution:
=========
The solution is to make bgp cleanup the l3vni a bgp instance is going
down.

The fix:
========
The fix is to add a function in bgp_evpn.c to be responsible for for
deleting the local vni, if it should be needed, and call the function
from bgp_instance_down().

Testing:
========
Created a test, which can run in container lab that remove the vrf on
the host before removing the vrf and the bgp config form frr. Running
this test in a loop trigger the problem 18 times of 100 runs. After the
fix it did not fail.

To verify the fix a log message (which is not in the code any longer)
were used when we had a stale l3vni and needed to call
bgp_evpn_local_l3vni_del() to do the cleanup. This were hit 20 times in
100 test runs.

Signed-off-by: Kacper Kwasny <kkwasny@akamai.com>
bgpd: braces {} are not necessary for single line block

Signed-off-by: Kacper Kwasny <kkwasny@akamai.com>

diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c
index ce9666d6115a23380e696bc0d157f0b638291420..495b60853f4f75bbe05a257b892ad9c0cadaf0a6 100644 (file)
--- a/bgpd/bgp_evpn.c
+++ b/bgpd/bgp_evpn.c
@@ -6971,6 +6971,17 @@ int bgp_evpn_local_l3vni_del(vni_t l3vni, vrf_id_t vrf_id)
        return 0;
 }
 
+/*
+ * When bgp instance goes down also clean up what might have been left over
+ * from evpn.
+ */
+void bgp_evpn_instance_down(struct bgp *bgp)
+{
+       /* If we have a stale local vni, delete it */
+       if (bgp->l3vni)
+               bgp_evpn_local_l3vni_del(bgp->l3vni, bgp->vrf_id);
+}
+
 /*
  * Handle del of a local VNI.
  */

diff --git a/bgpd/bgp_evpn.h b/bgpd/bgp_evpn.h
index 11a6f45dd0901f2eeff00cca2005ef3371db1996..ff2cb6ea91d074c6e1d63f139eb778b62161477e 100644 (file)
--- a/bgpd/bgp_evpn.h
+++ b/bgpd/bgp_evpn.h
@@ -153,6 +153,7 @@ extern int bgp_evpn_local_l3vni_add(vni_t vni, vrf_id_t vrf_id,
                                    struct in_addr originator_ip, int filter,
                                    ifindex_t svi_ifindex, bool is_anycast_mac);
 extern int bgp_evpn_local_l3vni_del(vni_t vni, vrf_id_t vrf_id);
+extern void bgp_evpn_instance_down(struct bgp *bgp);
 extern int bgp_evpn_local_vni_del(struct bgp *bgp, vni_t vni);
 extern int bgp_evpn_local_vni_add(struct bgp *bgp, vni_t vni,
                                  struct in_addr originator_ip,

diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c
index 09e64cf9ec31ebd416f9ea941ee2f62d5cd48bf8..a804f095fa10eceffc9f38e0684d20bb9a3c39bb 100644 (file)
--- a/bgpd/bgpd.c
+++ b/bgpd/bgpd.c
@@ -3872,6 +3872,9 @@ void bgp_instance_down(struct bgp *bgp)
        struct listnode *node;
        struct listnode *next;
 
+       /* Cleanup evpn instance state */
+       bgp_evpn_instance_down(bgp);
+
        /* Stop timers. */
        if (bgp->t_rmap_def_originate_eval)
                EVENT_OFF(bgp->t_rmap_def_originate_eval);