From 826c3f6db358e9bbc48848c5eadaab7916f5c3f9 Mon Sep 17 00:00:00 2001 From: Trey Aspelund Date: Wed, 25 Jan 2023 13:07:43 -0500 Subject: [PATCH 1/2] bgpd: only unimport routes if tunnel-ip changes When processing a new local VNI, we were always walking the global EVPN table to look for routes that needed to be removed due to a martian nexthop change (specifically a tunnel-ip change). Since the martian TIP table is global (all VNIs) + the walk is also in the global table (all VNIs), we can trust that any new TIP from any VNI would result in routes getting removed from the global table and unimported from all live (L2)VNIs. i.e. The only time this update is actionable is if we are adding/removing an IP from the martian TIP table, and we do not need to walk the table for normal refcount adjustments. Signed-off-by: Trey Aspelund --- bgpd/bgp_evpn.c | 22 ++++++++++++++-------- bgpd/bgp_nexthop.c | 19 +++++++++++++++++-- bgpd/bgp_nexthop.h | 2 +- 3 files changed, 32 insertions(+), 11 deletions(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index c9e935668e..dbd8bebe5a 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -2749,10 +2749,13 @@ static int handle_tunnel_ip_change(struct bgp *bgp, struct bgpevpn *vpn, /* Update the tunnel-ip hash */ bgp_tip_del(bgp, &vpn->originator_ip); - bgp_tip_add(bgp, &originator_ip); - - /* filter routes as martian nexthop db has changed */ - bgp_filter_evpn_routes_upon_martian_nh_change(bgp); + if (bgp_tip_add(bgp, &originator_ip)) + /* The originator_ip was not already present in the + * bgp martian next-hop table as a tunnel-ip, so we + * need to go back and filter routes matching the new + * martian next-hop. + */ + bgp_filter_evpn_routes_upon_martian_nh_change(bgp); /* Need to withdraw type-3 route as the originator IP is part * of the key. @@ -6567,10 +6570,13 @@ int bgp_evpn_local_vni_add(struct bgp *bgp, vni_t vni, SET_FLAG(vpn->flags, VNI_FLAG_LIVE); /* tunnel is now active, add tunnel-ip to db */ - bgp_tip_add(bgp, &originator_ip); - - /* filter routes as nexthop database has changed */ - bgp_filter_evpn_routes_upon_martian_nh_change(bgp); + if (bgp_tip_add(bgp, &originator_ip)) + /* The originator_ip was not already present in the + * bgp martian next-hop table as a tunnel-ip, so we + * need to go back and filter routes matching the new + * martian next-hop. + */ + bgp_filter_evpn_routes_upon_martian_nh_change(bgp); /* * Create EVPN type-3 route and schedule for processing. diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c index d9822ee974..77e26037c0 100644 --- a/bgpd/bgp_nexthop.c +++ b/bgpd/bgp_nexthop.c @@ -188,15 +188,30 @@ void bgp_tip_hash_destroy(struct bgp *bgp) bgp->tip_hash = NULL; } -void bgp_tip_add(struct bgp *bgp, struct in_addr *tip) +/* Add/Update Tunnel-IP entry of bgp martian next-hop table. + * + * Returns true only if we add a _new_ TIP so the caller knows that an + * actionable change has occurred. If we find an existing TIP then we + * only need to update the refcnt, since the collection of known TIPs + * has not changed. + */ +bool bgp_tip_add(struct bgp *bgp, struct in_addr *tip) { struct tip_addr tmp; struct tip_addr *addr; + bool tip_added = false; tmp.addr = *tip; - addr = hash_get(bgp->tip_hash, &tmp, bgp_tip_hash_alloc); + addr = hash_lookup(bgp->tip_hash, &tmp); + if (!addr) { + addr = hash_get(bgp->tip_hash, &tmp, bgp_tip_hash_alloc); + tip_added = true; + } + addr->refcnt++; + + return tip_added; } void bgp_tip_del(struct bgp *bgp, struct in_addr *tip) diff --git a/bgpd/bgp_nexthop.h b/bgpd/bgp_nexthop.h index efad906d0a..d765bdd261 100644 --- a/bgpd/bgp_nexthop.h +++ b/bgpd/bgp_nexthop.h @@ -166,7 +166,7 @@ extern void bgp_scan_finish(struct bgp *bgp); extern void bgp_scan_vty_init(void); extern void bgp_address_init(struct bgp *bgp); extern void bgp_address_destroy(struct bgp *bgp); -extern void bgp_tip_add(struct bgp *bgp, struct in_addr *tip); +extern bool bgp_tip_add(struct bgp *bgp, struct in_addr *tip); extern void bgp_tip_del(struct bgp *bgp, struct in_addr *tip); extern void bgp_tip_hash_init(struct bgp *bgp); extern void bgp_tip_hash_destroy(struct bgp *bgp); From 4dabdde32a11d7dfcdc1af7a5add7e912f170685 Mon Sep 17 00:00:00 2001 From: Trey Aspelund Date: Wed, 25 Jan 2023 13:25:20 -0500 Subject: [PATCH 2/2] bgpd: move tunnel-ip comparison into handler Moves the old/new IP comparison into handle_tunnel_ip_change instead of expecting the caller to do the check on their own. Also changes handle_tunnel_ip_change to return void since it only ever returned 0 in all cases. Signed-off-by: Trey Aspelund --- bgpd/bgp_evpn.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index dbd8bebe5a..155e33da91 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -2736,15 +2736,18 @@ static int bgp_evpn_mcast_grp_change(struct bgp *bgp, struct bgpevpn *vpn, * Note: Route re-advertisement happens elsewhere after other processing * other changes. */ -static int handle_tunnel_ip_change(struct bgp *bgp, struct bgpevpn *vpn, - struct in_addr originator_ip) +static void handle_tunnel_ip_change(struct bgp *bgp, struct bgpevpn *vpn, + struct in_addr originator_ip) { struct prefix_evpn p; + if (IPV4_ADDR_SAME(&vpn->originator_ip, &originator_ip)) + return; + /* If VNI is not live, we only need to update the originator ip */ if (!is_vni_live(vpn)) { vpn->originator_ip = originator_ip; - return 0; + return; } /* Update the tunnel-ip hash */ @@ -2765,7 +2768,7 @@ static int handle_tunnel_ip_change(struct bgp *bgp, struct bgpevpn *vpn, /* Update the tunnel IP and re-advertise all routes for this VNI. */ vpn->originator_ip = originator_ip; - return 0; + return; } static struct bgp_path_info * @@ -6548,8 +6551,7 @@ int bgp_evpn_local_vni_add(struct bgp *bgp, vni_t vni, /* If tunnel endpoint IP has changed, update (and delete prior * type-3 route, if needed.) */ - if (!IPV4_ADDR_SAME(&vpn->originator_ip, &originator_ip)) - handle_tunnel_ip_change(bgp, vpn, originator_ip); + handle_tunnel_ip_change(bgp, vpn, originator_ip); /* Update all routes with new endpoint IP and/or export RT * for VRFs