Recheck-request: github-robot-_Build_and_Test Recheck-request: github-robot-_ovn-kubernetes
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> po 28. 9. 2026 v 16:03 odesílatel Michal Arbet <[email protected]> napsal: > 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
