Skip to content
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.

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

bgpd: fix 6vpe nexthop #15368

Merged
merged 2 commits into from
Feb 28, 2024
Merged

bgpd: fix 6vpe nexthop #15368

merged 2 commits into from
Feb 28, 2024

Conversation

louis-6wind
Copy link
Contributor

@louis-6wind louis-6wind commented Feb 14, 2024

6vPE enables the announcement of IPv6 VPN prefixes through an IPv4 BGP
session. In this scenario, the next hop addresses for these prefixes are
represented in an IPv4-mapped IPv6 format, noted as ::ffff:[IPv4]. This
format indicates to the peer that it should route these IPv6 addresses
using information from the IPv4 nexthop. For example:

Path Attribute - MP_REACH_NLRI
[...]
Address family identifier (AFI): IPv6 (2)
Subsequent address family identifier (SAFI): Labeled VPN Unicast (128)
Next hop: RD=0:0 IPv6=::ffff:192.0.2.5 RD=0:0 Link-local=fe80::501d:42ff:feef:b021
Number of Subnetwork points of attachment (SNPA): 0

This rule is set out in RFC4798:

The IPv4 address of the egress 6PE router MUST be encoded as an
IPv4-mapped IPv6 address in the BGP Next Hop field.

However, in some situations, bgpd sends a standard nexthop IPv6 address
instead of an IPv4-mapped IPv6 address because the outgoing interface for
the BGP session has a valid IPv6 address. This is problematic because
the peer router may not be able to route the nexthop IPv6 address (ie.
if the outgoing interface has not IPv6).

Fix the issue by always sending a IPv4-mapped IPv6 address as nexthop
when the BGP session is on IPv4 and address family IPv6.

Link: https://datatracker.ietf.org/doc/html/rfc4798#section-2
Fixes: 92d6f76 ("lib,zebra,bgpd: Fix for nexthop as IPv4 mapped IPv6 address")

@louis-6wind louis-6wind marked this pull request as draft February 14, 2024 13:31
@louis-6wind louis-6wind marked this pull request as ready for review February 15, 2024 14:01
Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

A couple of nits, other than that LGTM.

@ton31337
Copy link
Member

@Mergifyio backport dev/10.0 stable/9.1 stable/9.0

Copy link

mergify bot commented Feb 18, 2024

backport dev/10.0 stable/9.1 stable/9.0

✅ Backports have been created

}
if (peer->nexthop.v4.s_addr != INADDR_ANY &&
(IN6_IS_ADDR_UNSPECIFIED(mod_v6nhg) ||
(peer->connection->su.sa.sa_family == AF_INET &&
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it only be (IN6_IS_ADDR_UNSPECIFIED(mod_v6nhg) && (peer->connection->su.sa.sa_family == AF_INET && paf->afi == AFI_IP6)))

Copy link
Contributor Author

@louis-6wind louis-6wind Feb 21, 2024

Choose a reason for hiding this comment

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

No it is correct.

When it is an IPv6 prefix over an IPv4 BGP session and the outgoing interface to the peer has a standard IPv6 address, mod_v6nhg contains this standard IPv6 address.

IN6_IS_ADDR_UNSPECIFIED(mod_v6nhg) is for the case of an IPv4 prefix over an IPv6 unnumbered BGP session. See tests/topotests/bgp_ipv4_over_ipv6/test_rfc5549_ebgp_unnumbered_nbr.py -> def bgp_prefix_received_v4_mapped_v6_nh(router)

Copy link
Member

@ton31337 ton31337 left a comment

Choose a reason for hiding this comment

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

Please add a pytestmark for a topotest.

pguibert6WIND and others added 2 commits February 26, 2024 10:16
This test uses the connected ipv4 mapped ipv6 prefix
to resolve the received BGP routes.

Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
Signed-off-by: Philippe Guibert <philippe.guibert@6wind.com>
Signed-off-by: François Dumontet <francois.dumontet@6wind.com>
6vPE enables the announcement of IPv6 VPN prefixes through an IPv4 BGP
session. In this scenario, the next hop addresses for these prefixes are
represented in an IPv4-mapped IPv6 format, noted as ::ffff:[IPv4]. This
format indicates to the peer that it should route these IPv6 addresses
using information from the IPv4 nexthop. For example:

> Path Attribute - MP_REACH_NLRI
> [...]
>     Address family identifier (AFI): IPv6 (2)
>     Subsequent address family identifier (SAFI): Labeled VPN Unicast (128)
>     Next hop:  RD=0:0 IPv6=::ffff:192.0.2.5 RD=0:0 Link-local=fe80::501d:42ff:feef:b021
>     Number of Subnetwork points of attachment (SNPA): 0

This rule is set out in RFC4798:

> The IPv4 address of the egress 6PE router MUST be encoded as an
> IPv4-mapped IPv6 address in the BGP Next Hop field.

However, in some situations, bgpd sends a standard nexthop IPv6 address
instead of an IPv4-mapped IPv6 address because the outgoing interface for
the BGP session has a valid IPv6 address. This is problematic because
the peer router may not be able to route the nexthop IPv6 address (ie.
if the outgoing interface has not IPv6).

Fix the issue by always sending a IPv4-mapped IPv6 address as nexthop
when the BGP session is on IPv4 and address family IPv6.

Link: https://datatracker.ietf.org/doc/html/rfc4798#section-2
Fixes: 92d6f76 ("lib,zebra,bgpd: Fix for nexthop as IPv4 mapped IPv6 address")
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
@louis-6wind
Copy link
Contributor Author

Please add a pytestmark for a topotest.

done

Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

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

looks good

@ton31337 ton31337 merged commit df98e88 into FRRouting:master Feb 28, 2024
9 checks passed
ton31337 added a commit that referenced this pull request Feb 28, 2024
ton31337 added a commit that referenced this pull request Feb 29, 2024
Jafaral added a commit to Jafaral/frr that referenced this pull request Aug 21, 2024
Jafaral added a commit to Jafaral/frr that referenced this pull request Aug 21, 2024
Jafaral added a commit to Jafaral/frr that referenced this pull request Aug 22, 2024
Jafaral added a commit to Jafaral/frr that referenced this pull request Aug 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants