From 6f588a188667b5f98ea4584ff9c2317759ea1bdc Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Wed, 18 Jan 2023 15:37:48 +0100 Subject: [PATCH] bgpd: vrf label is configured only when first VPN entry selected This commit addresses an optimisation issue that consists in delaying the creation of the MPLS entry for return traffic, only when the first BGP update using that label is sent. Today, MPLS entries from L3VPN instances are always created, even if there are no programmed BGP updates to export. Creating such entry uses an MPLS route entry for nothing. This commit addresses the issue by creating the MPLS entry only when needed. Let us take an example with the below L3VPN configuration. Once the label value is chosen (auto or hardset per-vrf value), the creation of the MPLS entry is triggered to handle return traffic. > router bgp 65500 vrf vrf2 > bgp router-id 1.1.1.1 > address-family ipv4 unicast > label vpn export 222 > rd vpn export 444:444 > rt vpn both 53:100 > export vpn > import vpn > exit-address-family The below show command illustrates the MPLS entry output, after having applied the above config. > r1# show bgp vrf vrf2 ipv4 > No BGP prefixes displayed, 0 exist > r1# show mpls table > Inbound Label Type Nexthop Outbound Label > ----------------------------------------------------- > [..] > 222 BGP vrf2 - With the above configuration only, the MPLS entry presence is useless. Actually, no BGP VPNvx updates have been sent (either via redistribute or network commands), so no BGP peers are supposed to know how to use the 222 label value. As consequence, there is no incoming traffic to expect with such label value. The commit proposes to create the MPLS entry, only when the first BGP update that uses that label value is selected in the bgp best selection algorithm. The commit does not change the time when the MPLS entry is removed. Signed-off-by: Philippe Guibert --- bgpd/bgp_mplsvpn.c | 7 ++++--- bgpd/bgp_mplsvpn.h | 3 +-- bgpd/bgp_route.c | 13 +++++++++++++ bgpd/bgpd.c | 3 +-- bgpd/bgpd.h | 3 ++- 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c index 8c9cac2205de..4937b258230f 100644 --- a/bgpd/bgp_mplsvpn.c +++ b/bgpd/bgp_mplsvpn.c @@ -312,8 +312,9 @@ void vpn_leak_zebra_vrf_label_update(struct bgp *bgp, afi_t afi) if (label == BGP_PREVENT_VRF_2_VRF_LEAK) label = MPLS_LABEL_NONE; - zclient_send_vrf_label(zclient, bgp->vrf_id, afi, label, ZEBRA_LSP_BGP); - bgp->vpn_policy[afi].tovpn_zebra_vrf_label_last_sent = label; + bgp->vpn_policy[afi].tovpn_zebra_vrf_label_to_send = label; + SET_FLAG(bgp->vpn_policy[afi].flags, + BGP_VPN_POLICY_TOVPN_LABEL_TO_SEND); } /* @@ -340,7 +341,7 @@ void vpn_leak_zebra_vrf_label_withdraw(struct bgp *bgp, afi_t afi) } zclient_send_vrf_label(zclient, bgp->vrf_id, afi, label, ZEBRA_LSP_BGP); - bgp->vpn_policy[afi].tovpn_zebra_vrf_label_last_sent = label; + bgp->vpn_policy[afi].tovpn_zebra_vrf_label_to_send = label; } /* diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h index 19b6f4eb777e..6baaa8d699e1 100644 --- a/bgpd/bgp_mplsvpn.h +++ b/bgpd/bgp_mplsvpn.h @@ -250,8 +250,7 @@ static inline void vpn_leak_postchange(enum vpn_policy_direction direction, if (direction == BGP_VPN_POLICY_DIR_TOVPN) { if (bgp_vrf->vpn_policy[afi].tovpn_label != - bgp_vrf->vpn_policy[afi] - .tovpn_zebra_vrf_label_last_sent) { + bgp_vrf->vpn_policy[afi].tovpn_zebra_vrf_label_to_send) { vpn_leak_zebra_vrf_label_update(bgp_vrf, afi); } diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index b627487e7c20..a0c7b656d788 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -3288,6 +3288,7 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, struct bgp_path_info *new_select; struct bgp_path_info *old_select; struct bgp_path_info_pair old_and_new; + struct bgp *new_bgp_orig = NULL; int debug = 0; if (CHECK_FLAG(bgp->flags, BGP_FLAG_DELETE_IN_PROGRESS)) { @@ -3352,6 +3353,18 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, bgp_mplsvpn_handle_label_allocation(bgp, dest, new_select, old_select, afi); + if (safi == SAFI_MPLS_VPN && new_select && new_select->extra && + new_select->extra->bgp_orig) + new_bgp_orig = new_select->extra->bgp_orig; + if (new_bgp_orig && CHECK_FLAG(new_bgp_orig->vpn_policy[afi].flags, + BGP_VPN_POLICY_TOVPN_LABEL_TO_SEND)) { + zclient_send_vrf_label(zclient, new_bgp_orig->vrf_id, afi, + new_bgp_orig->vpn_policy[afi] + .tovpn_zebra_vrf_label_to_send, + ZEBRA_LSP_BGP); + UNSET_FLAG(new_bgp_orig->vpn_policy[afi].flags, + BGP_VPN_POLICY_TOVPN_LABEL_TO_SEND); + } if (debug) zlog_debug( "%s: p=%pBD(%s) afi=%s, safi=%s, old_select=%p, new_select=%p", diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 0a01d719687f..1a4a996b6d79 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -3398,9 +3398,8 @@ static struct bgp *bgp_create(as_t *as, const char *name, bgp->vpn_policy[afi].bgp = bgp; bgp->vpn_policy[afi].afi = afi; bgp->vpn_policy[afi].tovpn_label = MPLS_LABEL_NONE; - bgp->vpn_policy[afi].tovpn_zebra_vrf_label_last_sent = + bgp->vpn_policy[afi].tovpn_zebra_vrf_label_to_send = MPLS_LABEL_NONE; - bgp->vpn_policy[afi].import_vrf = list_new(); bgp->vpn_policy[afi].import_vrf->del = bgp_vrf_string_name_delete; diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 42e4c167f6b4..91d9867357c7 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -209,7 +209,7 @@ struct vpn_policy { /* should be mpls_label_t? */ uint32_t tovpn_label; /* may be MPLS_LABEL_NONE */ - uint32_t tovpn_zebra_vrf_label_last_sent; + uint32_t tovpn_zebra_vrf_label_to_send; char *tovpn_rd_pretty; struct prefix_rd tovpn_rd; struct prefix tovpn_nexthop; /* unset => set to 0 */ @@ -219,6 +219,7 @@ struct vpn_policy { #define BGP_VPN_POLICY_TOVPN_NEXTHOP_SET (1 << 2) #define BGP_VPN_POLICY_TOVPN_SID_AUTO (1 << 3) #define BGP_VPN_POLICY_TOVPN_LABEL_PER_NEXTHOP (1 << 4) +#define BGP_VPN_POLICY_TOVPN_LABEL_TO_SEND (1 << 5) /* * If we are importing another vrf into us keep a list of