From 10f70510b952914cf3bacdd5101c52a245c5fead Mon Sep 17 00:00:00 2001 From: Ameya Dharkar Date: Thu, 7 May 2020 13:33:02 -0700 Subject: [PATCH] bgpd: EVPN RT-2 advertised with 2 labels for prefix-routes-only config L3VNI is configured with "prefix-routes-only" flag. Even in this case, intermittently, we observed that local EVPN MACIP routes are installed and advertised with 2 labels and 2 export RTs. This is a sequencing issue. Consider following case where L2VNI 200 and L3VNI 1000 are configured for tenant vrf vrf-blue. Bug is observed for following sequence of events: 1. vrf-blue BGP instance is created. 2. L2VNI is created in bgp for vni 200. It is linked to the tenant vrf vrf-blue in function bgpevpn_link_to_l3vni. Following code sets "VNI_FLAG_USE_TWO_LABELS" flag for vni 200 as L3VNI is not yet attached to vrf-blue BGP instance. /* check if we are advertising two labels for this vpn */ if (!CHECK_FLAG(bgp_vrf->vrf_flags, BGP_VRF_L3VNI_PREFIX_ROUTES_ONLY)) SET_FLAG(vpn->flags, VNI_FLAG_USE_TWO_LABELS); 2. Now L3VNI is attached to vrf-blue BGP instance. In this case, we set BGP_VRF_L3VNI_PREFIX_ROUTES_ONLY flag for vrf-blue but we do not clear VNI_FLAG_USE_TWO_LABELS flag set on the corresponding L2VNIs. This fix resolves following 2 issues observed above. 1. When L2VNI is created in BGP, flag VNI_FLAG_USE_TWO_LABELS should not be set for this VNI if BGP vrf is not attached to any L3VNI. 2. When L3VNI is attached to the BGP vrf, set "VNI_FLAG_USE_TWO_LABELS" flag if "prefix-routes-only" is not for the vrf. UT cases: 1. Flap "prefix-routes-only" config for a vrf. 2. Test following triggers for vrfs with and without "prefix-routes-only" - Flap L2VNI from kernel. - Flap L3VNI from kernel. Signed-off-by: Ameya Dharkar --- bgpd/bgp_evpn.c | 24 +++++++++++++++++++++++- bgpd/bgp_evpn_private.h | 8 ++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index a10686189d..c44a822d11 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -5813,9 +5813,31 @@ int bgp_evpn_local_l3vni_add(vni_t l3vni, vrf_id_t vrf_id, is_anycast_mac ? "Enable" : "Disable"); } /* set the right filter - are we using l3vni only for prefix routes? */ - if (filter) + if (filter) { SET_FLAG(bgp_vrf->vrf_flags, BGP_VRF_L3VNI_PREFIX_ROUTES_ONLY); + /* + * VNI_FLAG_USE_TWO_LABELS flag for linked L2VNIs should not be + * set before linking vrf to L3VNI. Thus, no need to clear + * that explicitly. + */ + } else { + UNSET_FLAG(bgp_vrf->vrf_flags, + BGP_VRF_L3VNI_PREFIX_ROUTES_ONLY); + + for (ALL_LIST_ELEMENTS_RO(bgp_vrf->l2vnis, node, vpn)) { + if (!CHECK_FLAG(vpn->flags, VNI_FLAG_USE_TWO_LABELS)) { + + /* + * If we are flapping VNI_FLAG_USE_TWO_LABELS + * flag, update all MACIP routes in this VNI + */ + SET_FLAG(vpn->flags, VNI_FLAG_USE_TWO_LABELS); + update_all_type2_routes(bgp_evpn, vpn); + } + } + } + /* Map auto derive or configured RTs */ if (!CHECK_FLAG(bgp_vrf->vrf_flags, BGP_VRF_IMPORT_RT_CFGD)) evpn_auto_rt_import_add_for_vrf(bgp_vrf); diff --git a/bgpd/bgp_evpn_private.h b/bgpd/bgp_evpn_private.h index ea1ae087f1..c7ccf69f05 100644 --- a/bgpd/bgp_evpn_private.h +++ b/bgpd/bgp_evpn_private.h @@ -270,8 +270,12 @@ static inline void bgpevpn_link_to_l3vni(struct bgpevpn *vpn) vpn->bgp_vrf = bgp_lock(bgp_vrf); listnode_add_sort(bgp_vrf->l2vnis, vpn); - /* check if we are advertising two labels for this vpn */ - if (!CHECK_FLAG(bgp_vrf->vrf_flags, BGP_VRF_L3VNI_PREFIX_ROUTES_ONLY)) + /* + * If L3VNI is configured, + * check if we are advertising two labels for this vpn + */ + if (bgp_vrf->l3vni && + !CHECK_FLAG(bgp_vrf->vrf_flags, BGP_VRF_L3VNI_PREFIX_ROUTES_ONLY)) SET_FLAG(vpn->flags, VNI_FLAG_USE_TWO_LABELS); }