Skip to content

bgpd: PMSI tunnel attribute compatibility #21507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Jafaral merged 4 commits into from
Apr 14, 2026
Merged

bgpd: PMSI tunnel attribute compatibility #21507

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c7cb4c3
bgpd: EVPN PMSI Tunnel Id interop fix with V6 support
mrin-g Nov 4, 2025
cd8af03
bgpd: EVPN PMSI Tunnel Id, show output with JSON
mrin-g Nov 4, 2025
55461a9
test: Topotest to validate PMSI V4 and V6 tunnel Id
mrin-g Dec 2, 2025
f864838
tests: Add a v6 pmsi tunnel only test.
donaldsharp Apr 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 55 additions & 10 deletions bgpd/bgp_attr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,9 @@ unsigned int attrhash_key_make(const void *p)
MIX3(attr->bh_type, attr->otc, bgp_attr_get_aigp_metric(attr));
MIX3(attr->mm_seqnum, attr->df_alg, attr->df_pref);
MIX(attr->encap_tunneltype);
MIX(bgp_attr_get_pmsi_tnl_type(attr));
if (bgp_attr_get_pmsi_tnl_type(attr) == PMSI_TNLTYPE_INGR_REPL)
key = jhash(attr->tunn_id.s6_addr, IPV6_MAX_BYTELEN, key);
Copy link
Copy Markdown
Contributor Author

@mjstapp mjstapp Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Donald had a comment in the original PR:

so from my perspective anytime we've had a situation where we had unset values as part of this attribute hash key making/comparison we've ended up with bugs. I would prefer that FRR has a v4 and a v6 side of this where we explicitly only create a key for the v4 side if v4 and if v6 use the v6 data. The way I am reading this is that the v4 and v6 share the same space, correct?

key = jhash(&attr->rmac, sizeof(attr->rmac), key);
if (bgp_attr_get_nhc(attr))
MIX(bgp_nhc_hash_key_make(bgp_attr_get_nhc(attr)));
Expand Down Expand Up @@ -1104,7 +1107,9 @@ bool attrhash_cmp(const void *p1, const void *p2)
attr1->bh_type == attr2->bh_type && attr1->otc == attr2->otc &&
!memcmp(&attr1->rmac, &attr2->rmac, sizeof(struct ethaddr)) &&
bgp_nhc_same(bgp_attr_get_nhc(attr1), bgp_attr_get_nhc(attr2)) &&
bgp_ls_attr_same(attr1->ls_attr, attr2->ls_attr))
bgp_ls_attr_same(attr1->ls_attr, attr2->ls_attr) &&
(attr1->pmsi_tnl_type == attr2->pmsi_tnl_type) &&
IPV6_ADDR_SAME(&attr1->tunn_id, &attr2->tunn_id))
return true;
}

Expand Down Expand Up @@ -3751,6 +3756,7 @@ bgp_attr_pmsi_tunnel(struct bgp_attr_parser_args *args)
const bgp_size_t length = args->length;
uint8_t tnl_type;
int attr_parse_len = 2 + BGP_LABEL_BYTES;
struct in_addr tunn_id;

if (peer->discard_attrs[args->type] || peer->withdraw_attrs[args->type])
goto pmsi_tunnel_ignore;
Expand All @@ -3773,9 +3779,14 @@ bgp_attr_pmsi_tunnel(struct bgp_attr_parser_args *args)
args->total);
}
if (tnl_type == PMSI_TNLTYPE_INGR_REPL) {
if (length != 9) {
/* The length should be one of these values:
* 1 (Flags) + 1 (Tunnel Type) + 3 (Label) + 4 (IPv4 address) = 9 octets
* 1 (Flags) + 1 (Tunnel Type) + 3 (Label) + 16 (IPv6 address) = 21 octets
*/
if (length != BGP_ATTR_PMSI_TUNNEL_V4_LENGTH &&
length != BGP_ATTR_PMSI_TUNNEL_V6_LENGTH) {
flog_err(EC_BGP_ATTR_PMSI_LEN,
"Bad PMSI tunnel attribute length %d for IR",
"Bad PMSI tunnel attribute length %d for Ingress Replication",
length);
return bgp_attr_malformed(
args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
Expand All @@ -3787,7 +3798,19 @@ bgp_attr_pmsi_tunnel(struct bgp_attr_parser_args *args)
bgp_attr_set_pmsi_tnl_type(attr, tnl_type);
stream_get(&attr->label, connection->curr, BGP_LABEL_BYTES);

/* Forward read pointer of input stream. */
/* Decode ingress-replication tunnel id */
if (tnl_type == PMSI_TNLTYPE_INGR_REPL) {
if (length == BGP_ATTR_PMSI_TUNNEL_V4_LENGTH) {
tunn_id.s_addr = stream_get_ipv4(connection->curr);
ipv4_to_ipv4_mapped_ipv6(&attr->tunn_id, tunn_id);
attr_parse_len += IPV4_MAX_BYTELEN;
} else {
stream_get(&attr->tunn_id, connection->curr, IPV6_MAX_BYTELEN);
attr_parse_len += IPV6_MAX_BYTELEN;
}
}

/* Forward read pointer of input stream to skip anything we didn't parse */
stream_forward_getp(connection->curr, length - attr_parse_len);

return BGP_ATTR_PARSE_PROCEED;
Expand Down Expand Up @@ -5652,15 +5675,37 @@ bgp_size_t bgp_packet_attribute(struct bgp *bgp, struct peer *peer, struct strea

/* PMSI Tunnel */
if (bgp_attr_exists(attr, BGP_ATTR_PMSI_TUNNEL)) {
uint8_t tunn_id_len = 0;
uint8_t *nh;
struct in_addr tunn_id;

stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_TRANS);
stream_putc(s, BGP_ATTR_PMSI_TUNNEL);
stream_putc(s, 9); // Length
stream_putc(s, 0); // Flags

/* Encode tunnel id for known tunnel type */
if (bgp_attr_get_pmsi_tnl_type(attr) == PMSI_TNLTYPE_INGR_REPL) {
if (IS_MAPPED_IPV6(&attr->tunn_id)) {
stream_putc(s, BGP_ATTR_PMSI_TUNNEL_V4_LENGTH);
tunn_id_len = IPV4_MAX_BYTELEN;
ipv4_mapped_ipv6_to_ipv4(&attr->tunn_id, &tunn_id);
Copy link
Copy Markdown
Member

@donaldsharp donaldsharp Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems wrong. We are supposed to be writing to the stream, yet we are setting the attr->tunn_id not the stream, right? The non mapped case writes the value to the stream. Also I am dubious about any pattern where we are setting attr values in a write function to a stream. This is the wrong place to do it.

Copy link
Copy Markdown
Member

@donaldsharp donaldsharp Apr 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shit I read this wrong. The ipv4_mapped_ipv6_to_ipv4 sets tunn_id.

nh = (uint8_t *)&tunn_id;
} else {
stream_putc(s, BGP_ATTR_PMSI_TUNNEL_V6_LENGTH);
tunn_id_len = IPV6_MAX_BYTELEN;
nh = (uint8_t *)&attr->tunn_id;
}
} else {
/* Encode label part only */
stream_putc(s, BGP_ATTR_PMSI_TUNNEL_LBL_ONLY_LEN);
}

stream_putc(s, 0); /* Flags */
stream_putc(s, bgp_attr_get_pmsi_tnl_type(attr));
stream_put(s, &(attr->label),
BGP_LABEL_BYTES); // MPLS Label / VXLAN VNI
stream_put_ipv4(s, attr->nexthop.s_addr);
// Unicast tunnel endpoint IP address
stream_put(s, &(attr->label), BGP_LABEL_BYTES); /* MPLS Label/VXLAN VNI */

/* Unicast tunnel endpoint IP address */
if (tunn_id_len > 0)
stream_put(s, nh, tunn_id_len);
}

/* OTC */
Expand Down
8 changes: 7 additions & 1 deletion bgpd/bgp_attr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ enum pta_type {
PMSI_TNLTYPE_MAX = PMSI_TNLTYPE_MLDP_MP2MP
};

/* PMSI lengths for label-only, ipv4, and ipv6 "identifier" */
#define BGP_ATTR_PMSI_TUNNEL_LBL_ONLY_LEN 5
#define BGP_ATTR_PMSI_TUNNEL_V4_LENGTH 9
#define BGP_ATTR_PMSI_TUNNEL_V6_LENGTH 21

/*
* Prefix-SID type-4
* SRv6-VPN-SID-TLV
Expand Down Expand Up @@ -217,6 +222,7 @@ struct attr {

/* PMSI tunnel type (RFC 6514). */
enum pta_type pmsi_tnl_type;
struct in6_addr tunn_id; /* PMSI Tunnel Id */

/* Multi-Protocol Nexthop, AFI IPv6 */
struct in6_addr mp_nexthop_global;
Expand Down Expand Up @@ -509,7 +515,7 @@ static inline uint32_t mac_mobility_seqnum(struct attr *attr)
return (attr) ? attr->mm_seqnum : 0;
}

static inline enum pta_type bgp_attr_get_pmsi_tnl_type(struct attr *attr)
static inline enum pta_type bgp_attr_get_pmsi_tnl_type(const struct attr *attr)
{
return attr->pmsi_tnl_type;
}
Expand Down
4 changes: 4 additions & 0 deletions bgpd/bgp_evpn.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2318,6 +2318,10 @@ static int update_evpn_route(struct bgp *bgp, struct bgpevpn *vpn,
if (p->prefix.route_type == BGP_EVPN_IMET_ROUTE) {
SET_FLAG(attr.flag, ATTR_FLAG_BIT(BGP_ATTR_PMSI_TUNNEL));
bgp_attr_set_pmsi_tnl_type(&attr, PMSI_TNLTYPE_INGR_REPL);
if (attr.mp_nexthop_len == BGP_ATTR_NHLEN_IPV4)
ipv4_to_ipv4_mapped_ipv6(&attr.tunn_id, attr.mp_nexthop_global_in);
else
IPV6_ADDR_COPY(&attr.tunn_id, &attr.mp_nexthop_global);
}

/* router mac is only needed for type-2 routes here. */
Expand Down