Skip to content

Commit

Permalink
bgpd: fix labels in adj-rib-in
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
  • Loading branch information
pguibert6WIND authored and louis-6wind committed Feb 26, 2024
1 parent 08dae41 commit 5a4eb39
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 5 deletions.
8 changes: 7 additions & 1 deletion bgpd/bgp_advertise.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
}
}
Expand All @@ -181,13 +185,15 @@ 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);
}

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 */
Expand Down
6 changes: 5 additions & 1 deletion bgpd/bgp_advertise.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions bgpd/bgp_label.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++;

Expand Down
6 changes: 3 additions & 3 deletions bgpd/bgp_route.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand Down Expand Up @@ -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));
Expand Down

0 comments on commit 5a4eb39

Please sign in to comment.