Hi Jakub,
Thanks for the bug report and for the fix! Sorry for the delay in
reviewing this.
On 7/28/25 7:52 PM, Jakub Libosvar via dev wrote:
> The logical router port for outport of a logical router was calculated
> based on the subnet matching of LRPs on the given LR. This doesn't work
> in case IPv6 local-link addresses are used.
>
> The patch introduces a generic function that calculates outport by
> strict matching on the nexthop address. This function is used when
> generating the logical flow from the logical router policy.
>
> This problem becomes more complex when a router is connected to
> complicated topology where the L2 domain is stretched over multiple
> logical switches or even localnet ports. This complex solution is not
> included in this patch and will be part of a followup patch.
>
What worries me a bit about this approach is that, in general, the only
restriction for mac addresses is that they need to be unique in the L2
domain they're used in.
Let's consider a topology that's not covered by the cases you tested below:
R1 ------- R2 -------- R3
netA netB
We have two IP networks, netA and netB.
In theory R1 and R3 could have the same MAC address, e.g.,
00:00:00:00:00:01 and that shouldn't cause any routing issues. The only
constraints are:
- R1 and R2 should have different MAC addresses on netA
- R2 and R3 should have different MAC addresses on netB
Now, if we add a policy on R2:
if dst == 2001::/64 then reroute 0000:00ff:fe00:0001
Then ovn-northd still can't choose any port. Arguably the policy is
incorrect though, because it's ambiguous. So, in order to fix that,
what we could do is add support for "output_port" for
Logical_Router_Policy (like we have for Logical_Router_Static_Route).
The CMS (configuring the policy) should always be able know which
output_port to configure in these cases?
> Same problem seems to exist when finding the outport from the logical
> router static route. The function responsible for finding the right LRP
> is genereic enough so it can be re-used later for the static route too
> in another followup patch.
As mentioned above, I think the CMS should probably be using
"output_port" for the static routes in these cases. From the man page
of "output_port":
'When this is specified and there are multiple IP addresses on the
router port and none of them are in the same subnet of "nexthop", OVN
chooses the first IP address as the one via which the "nexthop" is
reachable.'
If OVN would support output_port for policies too, would that be enough
for the use case you're targetting?
I had started reviewing the code in this patch but only left some
comments on the test itself below. I think it's better to first decide
on the best approach to solve the problem before continuing with the review.
Looking forward to hearing back from you!
Regards,
Dumitru
>
> Reported-at: https://issues.redhat.com/browse/FDP-1554
> Reported-by: Jakub Libosvar <jlibo...@redhat.com>
> Signed-off-by: Jakub Libosvar <jlibo...@redhat.com>
> ---
> northd/northd.c | 257 ++++++++++++++++++++++++++++++++++++++++++--
> tests/ovn-northd.at | 117 ++++++++++++++++++++
> 2 files changed, 364 insertions(+), 10 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 764575f21..84dc76770 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -11080,6 +11080,235 @@ lrp_find_member_ip(const struct ovn_port *op, const
> char *ip_s)
> return find_lport_address(&op->lrp_networks, ip_s);
> }
>
> +/*
> + * Check if the port has the given IPv6 link-local address.
> + *
> + * Parameters:
> + * op: The ovn_port to check
> + * lla_address: The IPv6 link-local address to check for
> + *
> + * Returns:
> + * true if the port has the given IPv6 link-local address, false otherwise
> +*/
> +static bool lrp_has_ipv6_lla(const struct ovn_port *op, const char
> *lla_address)
> +{
> + struct in6_addr lla_addr;
> + if (!ipv6_parse(lla_address, &lla_addr) || !in6_is_lla(&lla_addr)) {
> + return false;
> + }
> +
> + /* Check for exact LLA match in this port's addresses */
> + for (size_t j = 0; j < op->lrp_networks.n_ipv6_addrs; j++) {
> + struct ipv6_netaddr *addr = &op->lrp_networks.ipv6_addrs[j];
> + if (ipv6_addr_equals(&addr->addr, &lla_addr)) {
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +/* Generic helper function to find router port that can reach IPv6
> link-local address.
> + *
> + * IPv6 Link-Local Addresses (LLAs) are interface-scoped and require exact
> + * address matching per RFC 4291, not subnet matching like regular IPv6.
> + *
> + * This function searches for the exact LLA in the following order:
> + * 1. Local router ports on the same router
> + * 2. Directly peered router ports (peer= connections)
> + * 3. Router ports reachable via logical switch connections
> + * 4. Router ports reachable via localnet (same physical network)
> + *
> + * Parameters:
> + * od: The router datapath to search from
> + * lr_ports: Map of all logical router ports
> + * lla_address: The IPv6 link-local address string to resolve
> + *
> + * Returns:
> + * The ovn_port that can reach the target LLA, or NULL if not found.
> + *
> + * Note: This function is designed to be reusable for both routing policies
> + * and static routes. The caller determines how to use the returned
> port.
> + */
> +static struct ovn_port *
> +find_lrp_for_ipv6_lla(struct ovn_datapath *od,
> + const struct hmap *lr_ports,
> + const char *lla_address)
> +{
> + struct in6_addr lla_addr;
> + if (!ipv6_parse(lla_address, &lla_addr) ||
> + !in6_is_lla(&lla_addr)) {
> + return NULL;
> + }
> +
> + /* First, check if any of this router's ports has the exact LLA */
> + for (int i = 0; i < od->nbr->n_ports; i++) {
> + struct nbrec_logical_router_port *lrp = od->nbr->ports[i];
> + struct ovn_port *out_port = ovn_port_find(lr_ports, lrp->name);
> + if (!out_port) {
> + continue;
> + }
> +
> + if (lrp_has_ipv6_lla(out_port, lla_address)) {
> + return out_port;
> + }
> + }
> +
> + /* Second, check if the LLA is on a directly peered router port.
> + * LRPs can be directly connected to other LRPs via the peer column. */
> + for (int i = 0; i < od->nbr->n_ports; i++) {
> + struct nbrec_logical_router_port *lrp = od->nbr->ports[i];
> + struct ovn_port *out_port = ovn_port_find(lr_ports, lrp->name);
> + if (!out_port || !lrp->peer) {
> + continue;
> + }
> +
> + /* Find the peer LRP */
> + struct ovn_port *peer_lrp = ovn_port_find(lr_ports, lrp->peer);
> + if (!peer_lrp || !peer_lrp->nbrp) {
> + continue;
> + }
> +
> + /* Check if the peer LRP has the target LLA */
> + if (lrp_has_ipv6_lla(peer_lrp, lla_address)) {
> + return out_port;
> + }
> + }
> +
> + /* Third, check if the LLA is reachable through logical switch
> connections.
> + * We need to find router ports that connect to logical switches which
> + * might have the target LLA as a connected router port. */
> + for (int i = 0; i < od->nbr->n_ports; i++) {
> + struct nbrec_logical_router_port *lrp = od->nbr->ports[i];
> + struct ovn_port *out_port = ovn_port_find(lr_ports, lrp->name);
> + if (!out_port || !out_port->peer) {
> + continue;
> + }
> +
> + /* This router port connects to a logical switch */
> + struct ovn_datapath *peer_ls = out_port->peer->od;
> + if (!peer_ls || !peer_ls->nbs) {
> + continue;
> + }
> +
> + /* Check all router ports connected to this logical switch */
> + struct ovn_port *op;
> + VECTOR_FOR_EACH (&peer_ls->router_ports, op) {
> + if (!op->peer) {
> + continue;
> + }
> +
> + /* Get the actual router port (not the switch-side port) */
> + struct ovn_port *remote_lrp = op->peer;
> + if (!remote_lrp || !remote_lrp->nbrp) {
> + continue;
> + }
> +
> + /* Check if this remote router port has the target LLA */
> + if (lrp_has_ipv6_lla(remote_lrp, lla_address)) {
> + return out_port;
> + }
> + }
> + }
> +
> + /* Fourth, complex topologies with localnet connections.
> + * TODO: For complex topologies, we could search through multiple logical
> + * switch hops connected via localnet ports on the same physical network.
> + * This would handle cases like: Router1 -> LS1(localnet) <->
> LS2(localnet) -> Router2
> + * However, since LLAs are typically link-scoped, we leave this for
> future
> + * enhancement and focus on the common direct connection cases above.
> + */
> +
> + return NULL;
> +}
> +
> +/* Wrapper function for IPv6 LLA nexthop resolution.
> + *
> + * Determines if the given nexthop is an IPv6 LLA and resolves it using exact
> + * address matching. For non-LLA addresses, returns NULL so caller can use
> + * regular subnet-based matching.
> + *
> + * This wrapper is designed for easy integration into both routing policies
> + * and static routes without duplicating the LLA detection logic.
> + *
> + * Returns:
> + * - Non-NULL: LRP that can reach the IPv6 LLA nexthop
> + * - NULL: Not an LLA or LLA not found (use regular resolution)
> + */
> +static struct ovn_port *
> +resolve_ipv6_lla_nexthop(struct ovn_datapath *od,
> + const struct hmap *lr_ports,
> + const char *nexthop)
> +{
> + if (!nexthop) {
> + return NULL;
> + }
> +
> + struct in6_addr nexthop_addr;
> + if (ipv6_parse(nexthop, &nexthop_addr) && in6_is_lla(&nexthop_addr)) {
> + return find_lrp_for_ipv6_lla(od, lr_ports, nexthop);
> + }
> +
> + return NULL; /* Not an IPv6 LLA, use regular resolution */
> +}
> +
> +/* Generic function to find the outport for a given nexthop address.
> + *
> + * This function handles all possible OVN topology scenarios for nexthop
> resolution:
> + * 1. IPv6 Link-Local Addresses (LLAs) - Uses exact address matching per RFC
> 4291
> + * 2. Regular IPv4/IPv6 addresses - Uses subnet-based matching
> + * 3. Router-to-router peer connections via peer column
> + * 4. Router connections through logical switches
> + * 5. Complex topologies with localnet connections
> + *
> + * Parameters:
> + * od: The source router datapath
> + * lr_ports: Map of all logical router ports
> + * nexthop: The nexthop IP address string to resolve
> + *
> + * Returns:
> + * The ovn_port that can reach the nexthop, or NULL if not found.
> + *
> + * Note: This function is designed to be reusable across OVN components
> + * (routing policies, static routes, etc.) without duplicating
> topology logic.
> + */
> +static struct ovn_port *
> +get_outport_for_nexthop(struct ovn_datapath *od,
> + const struct hmap *lr_ports,
> + const char *nexthop)
> +{
> + if (!nexthop) {
> + return NULL;
> + }
> +
> + /* Special handling for IPv6 link-local addresses.
> + * LLAs require exact address matching, not subnet matching. */
> + struct ovn_port *lla_port = resolve_ipv6_lla_nexthop(od, lr_ports,
> nexthop);
> + if (lla_port) {
> + return lla_port;
> + }
> +
> + /* Check if this was an LLA that we couldn't resolve.
> + * For LLAs, we don't fall back to subnet matching as it would be
> incorrect. */
> + struct in6_addr nexthop_addr;
> + if (ipv6_parse(nexthop, &nexthop_addr) && in6_is_lla(&nexthop_addr)) {
> + /* LLA nexthop not found - return NULL and let caller handle logging
> */
> + return NULL;
> + }
> +
> + /* For non-LLA addresses (IPv4 and regular IPv6), use subnet-based
> matching.
> + * This searches through all router ports to find one whose network
> contains the nexthop. */
> + for (int i = 0; i < od->nbr->n_ports; i++) {
> + struct nbrec_logical_router_port *lrp = od->nbr->ports[i];
> + struct ovn_port *out_port = ovn_port_find(lr_ports, lrp->name);
> +
> + if (out_port && lrp_find_member_ip(out_port, nexthop)) {
> + return out_port;
> + }
> + }
> +
> + return NULL;
> +}
> +
> static struct ovn_port*
> get_outport_for_routing_policy_nexthop(struct ovn_datapath *od,
> const struct hmap *lr_ports,
> @@ -11089,19 +11318,27 @@ get_outport_for_routing_policy_nexthop(struct
> ovn_datapath *od,
> return NULL;
> }
>
> - /* Find the router port matching the next hop. */
> - for (int i = 0; i < od->nbr->n_ports; i++) {
> - struct nbrec_logical_router_port *lrp = od->nbr->ports[i];
> -
> - struct ovn_port *out_port = ovn_port_find(lr_ports, lrp->name);
> - if (out_port && lrp_find_member_ip(out_port, nexthop)) {
> - return out_port;
> - }
> + /* Use the generic nexthop resolution function */
> + struct ovn_port *out_port = get_outport_for_nexthop(od, lr_ports,
> nexthop);
> + if (out_port) {
> + return out_port;
> }
>
> + /* Handle logging for unresolved nexthops with routing policy context */
> + struct in6_addr nexthop_addr;
> + bool is_ipv6_lla = (ipv6_parse(nexthop, &nexthop_addr) &&
> + in6_is_lla(&nexthop_addr));
> +
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
> - VLOG_WARN_RL(&rl, "No path for routing policy priority %d on router %s; "
> - "next hop %s", priority, od->nbr->name, nexthop);
> + if (is_ipv6_lla) {
> + VLOG_WARN_RL(&rl, "IPv6 link-local nexthop %s not found on router %s
> "
> + "or reachable logical switches; LLAs require exact
> address matching",
> + nexthop, od->nbr->name);
> + } else {
> + VLOG_WARN_RL(&rl, "No path for routing policy priority %d on router
> %s; "
> + "next hop %s", priority, od->nbr->name, nexthop);
> + }
> +
> return NULL;
> }
>
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 5ddb15587..00926ac57 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -3643,6 +3643,123 @@ AT_CHECK([grep "lr_in_policy[[^_]]" lr0flows2 |
> ovn_strip_lflows | sort], [0], [
> AT_CLEANUP
> ])
>
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([Router policies - IPv6 Link-Local Address (LLA) nexthop])
Thanks for adding tests!
> +AT_KEYWORDS([router policies ipv6 lla link-local])
> +ovn_start
> +
> +# Test 1: Valid IPv6 LLA nexthop to peer router
> +# Create logical switches
Nit: for all comments in this test: comments should be sentences (start
with a capital letter and end with a dot).
> +check ovn-nbctl ls-add transit-sw
> +
> +# Create first router with port
> +check ovn-nbctl lr-add router1
> +check ovn-nbctl lrp-add router1 r1-port1 02:de:ad:be:ef:01 192.168.1.1/24
> +
> +# Connect router1 to transit switch
Nit: "transit switch" is an overloaded term in OVN. I'd call this "sw"
or something similar.
> +check ovn-nbctl lsp-add transit-sw r1-lsp1
> +check ovn-nbctl lsp-set-type r1-lsp1 router
> +check ovn-nbctl lsp-set-addresses r1-lsp1 router
> +check ovn-nbctl lsp-set-options r1-lsp1 router-port=r1-port1
> +
> +# Create second router with port
> +check ovn-nbctl lr-add router2
> +check ovn-nbctl lrp-add router2 r2-port1 02:de:ad:be:ef:02 192.168.1.2/24
> +
> +# Connect router2 to same transit switch
> +check ovn-nbctl lsp-add transit-sw r2-lsp1
> +check ovn-nbctl lsp-set-type r2-lsp1 router
> +check ovn-nbctl lsp-set-addresses r2-lsp1 router
> +check ovn-nbctl lsp-set-options r2-lsp1 router-port=r2-port1
> +
> +# Add routing policy with valid LLA nexthop pointing to router2's
> auto-generated LLA
> +# MAC 02:de:ad:be:ef:02 generates LLA: fe80::de:adff:febe:ef02
> +check ovn-nbctl --wait=sb lr-policy-add router1 100 "ip6.dst ==
> 2001:db8::/64" reroute fe80::de:adff:febe:ef02
> +
> +# Verify logical flow was created with correct outport
> +ovn-sbctl dump-flows router1 > router1flows
> +AT_CAPTURE_FILE([router1flows])
> +
> +AT_CHECK([grep "lr_in_policy" router1flows | grep "priority=100" |
> ovn_strip_lflows], [0], [dnl
> + table=??(lr_in_policy ), priority=100 , match=(ip6.dst ==
> 2001:db8::/64), action=(xxreg0 = fe80::de:adff:febe:ef02; xxreg1 =
> fe80::de:adff:febe:ef01; eth.src = 02:de:ad:be:ef:01; outport = "r1-port1";
> flags.loopback = 1; reg8[[0..15]] = 0; reg9[[9]] = 0; next;)
> +])
> +
> +# Test 2: Invalid IPv6 LLA nexthop should not create flow
> +check ovn-nbctl --wait=sb lr-policy-add router1 200 "ip6.dst ==
> 2001:db8:1::/64" reroute fe80::dead:beef:cafe:babe
> +
> +# Check that NO flow was created for invalid LLA nexthop (should fail
> gracefully)
We could maybe check the northd log for the warn message:
WARN|IPv6 link-local nexthop fe80::dead:beef:cafe:babe not found on
router router1 or reachable logical switches; LLAs require exact address
matching.
> +ovn-sbctl dump-flows router1 > router1flows2
> +AT_CAPTURE_FILE([router1flows2])
> +
> +AT_CHECK([grep "lr_in_policy" router1flows2 | grep "priority=200" | wc -l],
> [0], [0
> +])
> +
> +# Test 3: Regular IPv6 (non-LLA) should still use subnet matching
> +# First, delete the existing port and recreate it with IPv6 network
> +check ovn-nbctl lrp-del r1-port1
> +check ovn-nbctl lrp-add router1 r1-port1 02:de:ad:be:ef:01 192.168.1.1/24
> 2001:db8:1::1/64
> +check ovn-nbctl --wait=sb lr-policy-add router1 300 "ip6.dst ==
> 2001:db8:2::/64" reroute 2001:db8:1::100
> +
> +# Verify regular IPv6 routing still works (subnet matching)
> +ovn-sbctl dump-flows router1 > router1flows3
> +AT_CAPTURE_FILE([router1flows3])
> +
> +AT_CHECK([grep "lr_in_policy" router1flows3 | grep "priority=300" |
> ovn_strip_lflows], [0], [dnl
> + table=??(lr_in_policy ), priority=300 , match=(ip6.dst ==
> 2001:db8:2::/64), action=(xxreg0 = 2001:db8:1::100; xxreg1 = 2001:db8:1::1;
> eth.src = 02:de:ad:be:ef:01; outport = "r1-port1"; flags.loopback = 1;
> reg8[[0..15]] = 0; reg9[[9]] = 0; next;)
> +])
> +
> +# Test 4: IPv4 routing should remain unchanged
> +check ovn-nbctl --wait=sb lr-policy-add router1 400 "ip4.dst == 10.0.0.0/16"
> reroute 192.168.1.100
> +
> +# Verify IPv4 routing still works normally
> +ovn-sbctl dump-flows router1 > router1flows4
> +AT_CAPTURE_FILE([router1flows4])
> +
> +AT_CHECK([grep "lr_in_policy" router1flows4 | grep "priority=400" |
> ovn_strip_lflows], [0], [dnl
> + table=??(lr_in_policy ), priority=400 , match=(ip4.dst ==
> 10.0.0.0/16), action=(reg0 = 192.168.1.100; reg5 = 192.168.1.1; eth.src =
> 02:de:ad:be:ef:01; outport = "r1-port1"; flags.loopback = 1; reg8[[0..15]] =
> 0; reg9[[9]] = 1; next;)
> +])
We also support routing IPv4 packets with IPv6 next-hops. Would it be
possible to add a test for that too?
> +
> +# Test 5: Direct LLA on local router port
> +check ovn-nbctl lr-add router3
> +check ovn-nbctl lrp-add router3 r3-port1 02:de:ad:be:ef:03 192.168.2.1/24
> +
> +# Add policy with LLA pointing to router3's own port (should work as direct
> match)
Hmm, should it though? This blackholes the traffic.
> +# MAC 02:de:ad:be:ef:03 generates LLA: fe80::de:adff:febe:ef03
> +check ovn-nbctl --wait=sb lr-policy-add router3 500 "ip6.dst ==
> 2001:db8:3::/64" reroute fe80::de:adff:febe:ef03
> +
> +# Verify direct LLA match works
> +ovn-sbctl dump-flows router3 > router3flows
> +AT_CAPTURE_FILE([router3flows])
> +
> +AT_CHECK([grep "lr_in_policy" router3flows | grep "priority=500" |
> ovn_strip_lflows], [0], [dnl
> + table=??(lr_in_policy ), priority=500 , match=(ip6.dst ==
> 2001:db8:3::/64), action=(xxreg0 = fe80::de:adff:febe:ef03; xxreg1 =
> fe80::de:adff:febe:ef03; eth.src = 02:de:ad:be:ef:03; outport = "r3-port1";
> flags.loopback = 1; reg8[[0..15]] = 0; reg9[[9]] = 0; next;)
> +])
> +
> +# Test 6: IPv6 LLA nexthop to directly peered router port
> +# Create two routers with directly peered ports using peer= parameter
> +check ovn-nbctl lr-add router4
> +check ovn-nbctl lr-add router5
> +
> +# Create peered router ports with IPv6 networks
> +# MAC 02:de:ad:be:ef:04 generates LLA: fe80::de:adff:febe:ef04
> +# MAC 02:de:ad:be:ef:05 generates LLA: fe80::de:adff:febe:ef05
> +check ovn-nbctl lrp-add router4 r4-r5 02:de:ad:be:ef:04 192.168.4.1/24
> 2001:db8:4::1/64 peer=r5-r4
> +check ovn-nbctl lrp-add router5 r5-r4 02:de:ad:be:ef:05 192.168.4.2/24
> 2001:db8:4::2/64 peer=r4-r5
> +
> +# Add routing policy on router4 with LLA nexthop pointing to router5's
> auto-generated LLA
> +check ovn-nbctl --wait=sb lr-policy-add router4 600 "ip6.dst ==
> 2001:db8:6::/64" reroute fe80::de:adff:febe:ef05
> +
> +# Verify logical flow was created with correct outport for peer connection
> +ovn-sbctl dump-flows router4 > router4flows
> +AT_CAPTURE_FILE([router4flows])
> +
> +AT_CHECK([grep "lr_in_policy" router4flows | grep "priority=600" |
> ovn_strip_lflows], [0], [dnl
> + table=??(lr_in_policy ), priority=600 , match=(ip6.dst ==
> 2001:db8:6::/64), action=(xxreg0 = fe80::de:adff:febe:ef05; xxreg1 =
> fe80::de:adff:febe:ef04; eth.src = 02:de:ad:be:ef:04; outport = "r4-r5";
> flags.loopback = 1; reg8[[0..15]] = 0; reg9[[9]] = 0; next;)
> +])
> +
> +AT_CLEANUP
> +])
> +
> OVN_FOR_EACH_NORTHD_NO_HV([
> AT_SETUP([ACL allow-stateless omit conntrack - Logical_Switch])
> ovn_start
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
