> On Sep 9, 2025, at 9:59 AM, Dumitru Ceara <[email protected]> wrote: > > 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
I think if there are two same addresses then it’s a problem with the CMS, it should not be using same MACs as they should be unique as per EUI-48. In general, the problem you’re describing is not tied to a MAC address but to “same IP” problem. Same can happen also if netA and netB use the same IP range and R1 and R2 LRPs have the same IP. > > 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? yes, output_port works for my use case too and we know what output_port we want to set when we’re creating the policy. For me it is fine, just thinking out loud if a mechanism exists in northd that can find the “right” output_port then it should also consider local-link-addresses too. The ambiguity, as I tried to explain above, also happens with colliding IPs, not necessarily same MAC addresses. > >> 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? Yes :) > > 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. Thanks a lot for looking at it! Jakub > > Looking forward to hearing back from you! > > Regards, > Dumitru > >> >> Reported-at: https://issues.redhat.com/browse/FDP-1554 >> Reported-by: Jakub Libosvar <[email protected]> >> Signed-off-by: Jakub Libosvar <[email protected]> >> --- >> 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 [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
