When multiple logical router ports on the same router share a connected
prefix and BFD is enabled on each port, northd generates BFD helper routes
with identical matches but different actions.

ovn-controller represents desired flows with the same OpenFlow match using
a single installed flow, so only one of these routes becomes active.  The
selected route can also change after a full recompute.  As a result, BFD
traffic for one logical router port can be routed through another port and
redirected to a different gateway chassis.

BFD packets generated by pinctrl already carry the BFD logical router port
as MFF_LOG_INPORT.  Include that logical inport in the BFD helper route
match so that each BFD session selects the route associated with its own
logical router port.

Avoid adding the inport twice for IPv6 link-local connected routes, which
are already scoped to their logical router port.

Add a northd regression test with two logical router ports in the same
IPv4 subnet and ECMP+BFD routes to the same nexthop.  Also add a packet
test with two gateway chassis that captures controller-generated BFD
traffic at the provider uplinks.  Verify each session uses its own LRP
before and after controller recompute, and after moving the LRPs onto
the same chassis and back.  The same-chassis check detects the wrong
source MAC independently of conflicting flow ordering.

All four BFD tests pass with the fix.  Both variants of the new packet
test fail when run with ovn-northd built without the fix.  In one run,
BFD traffic follows the correct path initially but is redirected to
the wrong gateway after controller recompute.

Reported-at: https://github.com/ovn-org/ovn/issues/330
Submitted-at: https://github.com/ovn-org/ovn/pull/331
Assisted-by: GPT-6, OpenAI Codex
Signed-off-by: Michal Arbet <[email protected]>
---
Changes in v2:
- Add a packet-forwarding regression test for BFD sessions on LRPs
  sharing a connected subnet, including controller recompute and
  gateway placement changes.
- Verify both packet-test variants fail without the fix and all four
  BFD tests pass with the fix.
- Add Assisted-by and update the testing description.

 northd/northd.c     |   7 +++
 tests/ovn-northd.at |  42 +++++++++++++-
 tests/ovn.at        | 134 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 182 insertions(+), 1 deletion(-)

diff --git a/northd/northd.c b/northd/northd.c
index 4eb2ea44b..0ad7969ca 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -13422,6 +13422,13 @@ add_route(struct lflow_table *lflows, const struct 
ovn_datapath *od,
                   ds_cstr(&match), ds_cstr(&actions), lflow_ref,
                   WITH_HINT(stage_hint));
     if (op && bfd_is_port_running(bfd_ports, op->key)) {
+        /* BFD packets generated by ovn-controller are injected with their
+         * logical router port set as the logical inport.  Scope this helper
+         * route to that port so LRPs sharing a connected prefix do not
+         * generate conflicting flows with identical matches. */
+        if (!op_inport) {
+            ds_put_format(&match, " && inport == %s", op->json_key);
+        }
         ds_put_format(&match, " && udp.dst == 3784");
         ovn_lflow_add(lflows, op->od, S_ROUTER_IN_IP_ROUTING, priority + 1,
                       ds_cstr(&match), ds_cstr(&common_actions), lflow_ref,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 6572b1318..8f8aa8c54 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -4653,6 +4653,47 @@ OVN_CLEANUP_NORTHD
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([BFD routes on LRPs sharing a connected subnet])
+AT_KEYWORDS([northd-bfd])
+ovn_start
+
+check ovn-nbctl lr-add r0
+check ovn-nbctl lrp-add r0 r0-ext-a 00:00:00:00:00:01 10.0.0.10/24
+check ovn-nbctl lrp-add r0 r0-ext-b 00:00:00:00:00:02 10.0.0.20/24
+check ovn-nbctl ls-add ext
+check ovn-nbctl lsp-add-router-port ext ext-r0-a r0-ext-a
+check ovn-nbctl lsp-add-router-port ext ext-r0-b r0-ext-b
+
+# Neutron creates these routes and BFD records directly in the NB database.
+# Use the same approach here because lr-route-add rejects ECMP routes with a
+# duplicate nexthop, even when they use different output ports.
+check_uuid ovn-nbctl --wait=sb \
+    --id=@bfd_a create bfd logical_port=r0-ext-a dst_ip=10.0.0.1 -- \
+    --id=@route_a create logical_router_static_route ip_prefix=0.0.0.0/0 \
+        nexthop=10.0.0.1 output_port=r0-ext-a bfd=@bfd_a -- \
+    add logical_router r0 static_routes @route_a -- \
+    --id=@bfd_b create bfd logical_port=r0-ext-b dst_ip=10.0.0.1 -- \
+    --id=@route_b create logical_router_static_route ip_prefix=0.0.0.0/0 \
+        nexthop=10.0.0.1 output_port=r0-ext-b bfd=@bfd_b -- \
+    add logical_router r0 static_routes @route_b
+
+AT_CHECK([ovn-sbctl lflow-list | grep 'lr_in_ip_routing' | \
+    grep '10.0.0.0/24' | grep 'udp.dst == 3784' | wc -l], [0], [2
+])
+AT_CHECK([ovn-sbctl lflow-list | grep 'lr_in_ip_routing' | \
+    grep '10.0.0.0/24' | grep 'udp.dst == 3784' | \
+    grep -c 'inport == "r0-ext-a"'], [0], [1
+])
+AT_CHECK([ovn-sbctl lflow-list | grep 'lr_in_ip_routing' | \
+    grep '10.0.0.0/24' | grep 'udp.dst == 3784' | \
+    grep -c 'inport == "r0-ext-b"'], [0], [1
+])
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD_NO_HV([
 AT_SETUP([ovn -- check CoPP config])
 AT_KEYWORDS([northd-CoPP])
@@ -24351,4 +24392,3 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
 OVN_CLEANUP_NORTHD
 AT_CLEANUP
 ])
-
diff --git a/tests/ovn.at b/tests/ovn.at
index 13e95f9db..f647c886d 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -12815,6 +12815,140 @@ OVN_CLEANUP([hv1])
 AT_CLEANUP
 ])
 
+OVN_FOR_EACH_NORTHD([
+AT_SETUP([BFD packets on LRPs sharing a connected subnet])
+AT_KEYWORDS([ovn-bfd bfd-shared-subnet])
+ovn_start
+
+net_add underlay
+net_add provider
+
+for i in 1 2; do
+    sim_add gw$i
+    as gw$i
+    check ovs-vsctl add-br br-phys
+    ovn_attach underlay br-phys 192.168.0.$i
+    check ovs-vsctl add-br br-ex
+    net_attach provider br-ex
+    check ovs-vsctl set Open_vSwitch . \
+        external-ids:ovn-bridge-mappings=phys:br-ex
+done
+OVN_POPULATE_ARP
+
+check ovn-nbctl lr-add r0
+check ovn-nbctl ls-add ext
+check ovn-nbctl lsp-add-localnet-port ext ln-ext phys
+for i in 1 2; do
+    check ovn-nbctl lrp-add r0 r0-ext$i 00:00:00:00:00:0$i 10.0.0.$i/24
+    check ovn-nbctl lsp-add-router-port ext ext-r0-$i r0-ext$i
+    check ovn-nbctl lrp-set-gateway-chassis r0-ext$i gw$i
+    # Avoid depending on ARP replies from an external BFD peer.
+    check ovn-nbctl static-mac-binding-add r0-ext$i 10.0.0.254 \
+        00:00:00:00:00:fe
+done
+
+# Like Neutron, create the routes directly: lr-route-add rejects duplicate
+# ECMP nexthops even when the output ports differ.
+check_uuid ovn-nbctl --wait=hv \
+    --id=@bfd1 create BFD logical_port=r0-ext1 dst_ip=10.0.0.254 -- \
+    --id=@route1 create Logical_Router_Static_Route ip_prefix=0.0.0.0/0 \
+        nexthop=10.0.0.254 output_port=r0-ext1 bfd=@bfd1 -- \
+    add Logical_Router r0 static_routes @route1 -- \
+    --id=@bfd2 create BFD logical_port=r0-ext2 dst_ip=10.0.0.254 -- \
+    --id=@route2 create Logical_Router_Static_Route ip_prefix=0.0.0.0/0 \
+        nexthop=10.0.0.254 output_port=r0-ext2 bfd=@bfd2 -- \
+    add Logical_Router r0 static_routes @route2
+
+for i in 1 2; do
+    chassis=$(fetch_column Chassis _uuid name=gw$i)
+    wait_column "$chassis" Port_Binding chassis logical_port=cr-r0-ext$i
+
+    # Identify each controller-generated BFD session by its source port and
+    # discriminator, and check its Ethernet and IP addresses on the wire.
+    src_port=$(printf '%04x' $(fetch_column BFD src_port 
logical_port=r0-ext$i))
+    disc=$(printf '%08x' $(fetch_column BFD disc logical_port=r0-ext$i))
+    echo 
"0000000000fe00000000000${i}08000a00000${i}0a0000fe${src_port}0ec8${disc}" \
+        > bfd$i.expected
+done
+
+OVN_WAIT_PATCH_PORT_FLOWS([ln-ext], [gw1 gw2])
+check ovn-nbctl --wait=hv sync
+
+check_bfd_packets() {
+    local colocated=$1 gw
+
+    for gw in gw1 gw2; do
+        as $gw reset_pcap_file br-ex_provider $gw/br-ex_provider
+    done
+    if test "$colocated" = yes; then
+        cat bfd1.expected bfd2.expected | sort > gw1.expected
+        : > gw2.expected
+    else
+        cp bfd1.expected gw1.expected
+        cp bfd2.expected gw2.expected
+    fi
+
+    # These are packets emitted by pinctrl and forwarded by ovs-vswitchd,
+    # not traces or flow dumps.  Ignore ARP and compare the set of BFD
+    # sessions at each provider uplink, including the source MAC selected
+    # by routing.  Wait for several packets, and reject unexpected sessions.
+    OVS_WAIT_UNTIL([
+        for gw in gw1 gw2; do
+            $PYTHON "$ovs_srcdir/utilities/ovs-pcap.in" \
+                $gw/br-ex_provider-tx.pcap | \
+                grep -E '^.{24}080045.{16}11.{24}0ec8' | \
+                cut -c 1-28,53-76,93-100 > $gw.bfd
+            sort -u $gw.bfd > $gw.actual
+        done
+        test $(wc -l < gw1.bfd) -ge 3 &&
+        { test "$colocated" = yes || test $(wc -l < gw2.bfd) -ge 3; } &&
+        diff -u gw1.expected gw1.actual &&
+        diff -u gw2.expected gw2.actual
+    ])
+}
+
+AT_CAPTURE_FILE([gw1.actual])
+AT_CAPTURE_FILE([gw2.actual])
+AT_CAPTURE_FILE([gw1.expected])
+AT_CAPTURE_FILE([gw2.expected])
+
+AS_BOX([BFD sessions on separate gateways])
+# Each gateway must send only its own session through its provider uplink.
+check_bfd_packets no
+
+AS_BOX([BFD sessions after controller recompute])
+# Recomputing the controllers must not change either session's egress path.
+for gw in gw1 gw2; do
+    check as $gw ovn-appctl -t ovn-controller inc-engine/recompute
+done
+check ovn-nbctl --wait=hv sync
+check_bfd_packets no
+
+AS_BOX([BFD sessions on the same gateway])
+# Also check both LRPs on one chassis.  Without the fix only one of the
+# conflicting routes can win, so at least one session gets the wrong source
+# MAC regardless of flow ordering.  This makes the regression deterministic.
+check ovn-nbctl --wait=hv \
+    lrp-del-gateway-chassis r0-ext2 gw2 -- \
+    lrp-set-gateway-chassis r0-ext2 gw1
+chassis=$(fetch_column Chassis _uuid name=gw1)
+wait_column "$chassis" Port_Binding chassis logical_port=cr-r0-ext2
+check ovn-nbctl --wait=hv sync
+check_bfd_packets yes
+
+AS_BOX([BFD sessions back on separate gateways])
+check ovn-nbctl --wait=hv \
+    lrp-del-gateway-chassis r0-ext2 gw1 -- \
+    lrp-set-gateway-chassis r0-ext2 gw2
+chassis=$(fetch_column Chassis _uuid name=gw2)
+wait_column "$chassis" Port_Binding chassis logical_port=cr-r0-ext2
+check ovn-nbctl --wait=hv sync
+check_bfd_packets no
+
+OVN_CLEANUP([gw1], [gw2])
+AT_CLEANUP
+])
+
 OVN_FOR_EACH_NORTHD([
 AT_SETUP([4 HV, 1 LS, 1 LR, packet test with HA distributed router gateway 
port])
 ovn_start
-- 
2.53.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to