From 5a4eb39c107a4d1329731f0a58e945def29ab4c6 Mon Sep 17 00:00:00 2001 From: Philippe Guibert Date: Fri, 13 Jan 2023 15:59:52 +0100 Subject: [PATCH] bgpd: fix labels in adj-rib-in In a BGP L3VPN context using ADJ-RIB-IN (ie. enabled with 'soft-reconfiguration inbound'), after applying a deny route-map and removing it, the remote MPLS label information is lost. As a result, BGP is unable to re-install the related routes in the RIB. For example, > router bgp 65500 > [..] > neighbor 192.0.2.2 remote-as 65501 > address-family ipv4 vpn > neighbor 192.0.2.2 activate > neighbor 192.0.2.2 soft-reconfiguration inbound The 192.168.0.0/24 prefix has a remote label value of 102 in the BGP RIB. > # show bgp ipv4 vpn 192.168.0.0/24 > BGP routing table entry for 444:1:192.168.0.0/24, version 2 > [..] > 192.168.0.0 from 192.0.2.2 > Origin incomplete, metric 0, valid, external, best (First path received) > Extended Community: RT:52:100 > Remote label: 102 A route-map now filter all incoming BGP updates: > route-map rmap deny 1 > router bgp 65500 > address-family ipv4 vpn > neighbor 192.0.2.2 route-map rmap in The prefix is now filtered: > # show bgp ipv4 vpn 192.168.0.0/24 > # The route-map is detached: > router bgp 65500 > address-family ipv4 vpn > no neighbor 192.168.0.1 route-map rmap in The BGP RIB entry is present but the remote label is lost: > # show bgp ipv4 vpn 192.168.0.0/24 > BGP routing table entry for 444:1:192.168.0.0/24, version 2 > [..] > 192.168.0.0 from 192.0.2.2 > Origin incomplete, metric 0, valid, external, best (First path received) > Extended Community: RT:52:100 The reason for the loose is that labels are stored within struct attr -> struct extra -> struct bgp_labels but not in the struct bgp_adj_in. Reference the bgp_labels pointer in struct bgp_adj_in and use its values when doing a soft reconfiguration of the BGP table. Signed-off-by: Philippe Guibert Signed-off-by: Louis Scalbert --- bgpd/bgp_advertise.c | 8 +++++++- bgpd/bgp_advertise.h | 6 +++++- bgpd/bgp_label.c | 4 ++++ bgpd/bgp_route.c | 6 +++--- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/bgpd/bgp_advertise.c b/bgpd/bgp_advertise.c index a81f288c7acc..b42c05ffcd73 100644 --- a/bgpd/bgp_advertise.c +++ b/bgpd/bgp_advertise.c @@ -163,7 +163,7 @@ bool bgp_adj_out_lookup(struct peer *peer, struct bgp_dest *dest, void bgp_adj_in_set(struct bgp_dest *dest, struct peer *peer, struct attr *attr, - uint32_t addpath_id) + uint32_t addpath_id, struct bgp_labels *labels) { struct bgp_adj_in *adj; @@ -173,6 +173,10 @@ void bgp_adj_in_set(struct bgp_dest *dest, struct peer *peer, struct attr *attr, bgp_attr_unintern(&adj->attr); adj->attr = bgp_attr_intern(attr); } + if (!bgp_labels_cmp(adj->labels, labels)) { + bgp_labels_unintern(&adj->labels); + adj->labels = bgp_labels_intern(labels); + } return; } } @@ -181,6 +185,7 @@ void bgp_adj_in_set(struct bgp_dest *dest, struct peer *peer, struct attr *attr, adj->attr = bgp_attr_intern(attr); adj->uptime = monotime(NULL); adj->addpath_rx_id = addpath_id; + adj->labels = bgp_labels_intern(labels); BGP_ADJ_IN_ADD(dest, adj); bgp_dest_lock_node(dest); } @@ -188,6 +193,7 @@ void bgp_adj_in_set(struct bgp_dest *dest, struct peer *peer, struct attr *attr, void bgp_adj_in_remove(struct bgp_dest **dest, struct bgp_adj_in *bai) { bgp_attr_unintern(&bai->attr); + bgp_labels_unintern(&bai->labels); BGP_ADJ_IN_DEL(*dest, bai); *dest = bgp_dest_unlock_node(*dest); peer_unlock(bai->peer); /* adj_in peer reference */ diff --git a/bgpd/bgp_advertise.h b/bgpd/bgp_advertise.h index 49821061b1b8..e3cd743d43d2 100644 --- a/bgpd/bgp_advertise.h +++ b/bgpd/bgp_advertise.h @@ -95,6 +95,9 @@ struct bgp_adj_in { /* Received attribute. */ struct attr *attr; + /* VPN label information */ + struct bgp_labels *labels; + /* timestamp (monotime) */ time_t uptime; @@ -135,7 +138,8 @@ struct bgp_synchronize { extern bool bgp_adj_out_lookup(struct peer *peer, struct bgp_dest *dest, uint32_t addpath_tx_id); extern void bgp_adj_in_set(struct bgp_dest *dest, struct peer *peer, - struct attr *attr, uint32_t addpath_id); + struct attr *attr, uint32_t addpath_id, + struct bgp_labels *labels); extern bool bgp_adj_in_unset(struct bgp_dest **dest, struct peer *peer, uint32_t addpath_id); extern void bgp_adj_in_remove(struct bgp_dest **dest, struct bgp_adj_in *bai); diff --git a/bgpd/bgp_label.c b/bgpd/bgp_label.c index 106fe93a03f4..9b5b92795b1c 100644 --- a/bgpd/bgp_label.c +++ b/bgpd/bgp_label.c @@ -90,6 +90,10 @@ struct bgp_labels *bgp_labels_intern(struct bgp_labels *labels) if (!labels) return NULL; + if (!labels->num_labels) + /* do not intern void labels structure */ + return NULL; + find = (struct bgp_labels *)hash_get(labels_hash, labels, bgp_labels_hash_alloc); find->refcnt++; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 5927153e0a03..10838e9e3e0c 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -4278,7 +4278,7 @@ void bgp_update(struct peer *peer, const struct prefix *p, uint32_t addpath_id, memcpy(&attr->evpn_overlay, evpn, sizeof(struct bgp_route_evpn)); } - bgp_adj_in_set(dest, peer, attr, addpath_id); + bgp_adj_in_set(dest, peer, attr, addpath_id, &bgp_labels); } /* Update permitted loop count */ @@ -5351,8 +5351,8 @@ static void bgp_soft_reconfig_table_update(struct peer *peer, if (pi->peer == peer) break; - num_labels = bgp_path_info_num_labels(pi); - label_pnt = num_labels ? &pi->extra->labels->label[0] : NULL; + num_labels = ain->labels && ain->labels->num_labels; + label_pnt = num_labels ? &ain->labels->label[0] : NULL; if (pi) memcpy(&evpn, bgp_attr_get_evpn_overlay(pi->attr), sizeof(evpn));