Hi Dimitru, Thank you very much for your support !
Michal (kevko), Michal Arbet Openstack Engineer Ultimum Technologies a.s. Na Poříčí 1047/26, 11000 Praha 1 Czech Republic +420 604 228 897 [email protected] *https://ultimum.io <https://ultimum.io/>* LinkedIn <https://www.linkedin.com/company/ultimum-technologies> | Twitter <https://twitter.com/ultimumtech> | Facebook <https://www.facebook.com/ultimumtechnologies/timeline> On Wed, Sep 30, 2026, 8:24 PM Dumitru Ceara <[email protected]> wrote: > On 9/28/26 4:01 PM, Michal Arbet wrote: > > 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. > > > > Hi Michal, > > Thanks for the fix! > > > 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 > > ]) > > - > > Nit: unrelated removal. > > > 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]) > > The cleanup checks fail occasionally (both in GitHub CI and locally) > because at this point the "ext" localnet switch has multiple "peer" > ports to logical router "r0". > > In physical.c we generate conjunctive flows for each of the peers in: > > > https://github.com/ovn-org/ovn/blob/3bc96bd8b9fa52241ce2b3fbd2b39ee58a958fee/controller/physical.c#L1154 > > But with a recompute (which is what the OVN_CLEANUP implementation does) > the order of the peer ports in the vector may change, resulting in > slightly different (equivalent) conjunctive flows. > > We should fix that in OVN but until then let's skip the check for your > test. > > I added: > > diff --git a/tests/ovn.at b/tests/ovn.at > index f647c886dc..783c822a3e 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -12945,7 +12945,11 @@ 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]) > +OVN_CLEANUP([gw1 > +ignored_tables=OFTABLE_PHY_TO_LOG > +], [gw2 > +ignored_tables=OFTABLE_PHY_TO_LOG > +]) > AT_CLEANUP > ] > > > +AT_CLEANUP > > +]) > > + > > OVN_FOR_EACH_NORTHD([ > > AT_SETUP([4 HV, 1 LS, 1 LR, packet test with HA distributed router > gateway port]) > > ovn_start > > > With this fixed up, I applied the patch to main, 26.09 and 26.03. I > also added you to the AUTHORS.rst list. > > Regards, > Dumitru > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
