Load_Balancer health checks silently fail for baremetal pool members
whose backend Logical_Switch_Port is type=external on a Logical_Switch
that has a localnet port, which is the usual layout for Neutron and the
ovn-octavia-provider baremetal driver on a provider VLAN.
A type=external LSP has no VIF of its own. The reply to a health check
probe re-enters br-int through the switch's localnet port, so by the time
it reaches ls_in_l2_lkup MFF_LOG_INPORT carries the localnet port and the
per-backend reply lflow, which matches on the backend's inport, never
fires. pinctrl_find_svc_monitor() is keyed on the backend port, so the
reply is dropped and the member's health is misreported.
The two supported protocols fail in opposite directions. A TCP monitor
times out in svc_monitors_run() and goes permanently offline, taking a
healthy member out of the pool. A UDP monitor takes the other branch of
the same timeout and goes online; the only thing that ever moves it
offline is the ICMP unreachable punted through this lflow, so a dead
member stays online forever and traffic keeps being sent to it.
Add a copy of the reply lflow per localnet port for backends that are
type=external, matching the localnet port as inport and restoring the
backend's inport before punting:
match : inport == <localnet> && ip4.dst == <source ip> &&
ip4.src == <backend ip> && eth.dst == <lrp mac> &&
(tcp.src == <backend port> | icmp4.type == 3)
action: inport = "<backend>"; handle_svc_check(inport);
Only the inport differs from the lflow that is already generated for the
backend, so UDP needs no new match shape, it reuses the icmp4.type /
icmp6.type branch that is already there for it.
Unlike 3bae90d9fe17, which rewrote the inport at ls_in_check_port_sec for
every packet carrying the backend's MAC and caused a multicast storm, the
rewrite here cannot escape a single lflow. The match requires a unicast
eth.dst of the router port, so multicast and unknown unicast can never
reach it, and the action list ends with handle_svc_check() without a
next;, so no later table observes the assigned inport.
The new lflows are owned by the load balancer's lflow_ref. That is safe
because they depend on port types that lsp_can_be_inc_processed() rejects,
localnet and external, so adding or removing either forces northd to
recompute.
Assisted-by: Claude Opus 5, Claude Code
Signed-off-by: JayGue Lee <[email protected]>
---
Documentation/ref/ovn-logical-flows.7.rst | 13 +++
northd/northd.c | 73 ++++++++++++--
tests/ovn-northd.at | 116 ++++++++++++++++++++++
3 files changed, 191 insertions(+), 11 deletions(-)
diff --git a/Documentation/ref/ovn-logical-flows.7.rst
b/Documentation/ref/ovn-logical-flows.7.rst
index 1a9168ac86..8c20ed8c33 100644
--- a/Documentation/ref/ovn-logical-flows.7.rst
+++ b/Documentation/ref/ovn-logical-flows.7.rst
@@ -1438,6 +1438,19 @@ This table implements switching behavior. It contains
these logical flows:
the service monitor mac defined in the ``options:svc_monitor_mac`` column of
``NB_Global`` table.
+- For each load balancer backend that is health checked from an address
+ belonging to a logical router port, a priority-110 flow matching the probe
+ reply on ``inport == B`` and applying ``handle_svc_check(inport)``, where
+ *B* is the backend's logical switch port.
+
+ A ``type=external`` port has no VIF of its own, so its replies reach the
+ switch through a localnet port and the flow above cannot match. For such a
+ backend an additional copy of the flow is added for each localnet port *L*
+ on the switch, matching ``inport == L`` instead and applying ``inport = B;
+ handle_svc_check(inport);`` so that ``ovn-controller`` can tell which
+ service monitor the reply belongs to. The action list ends there, so the
+ assigned ``inport`` is not visible to any later table.
+
- A priority-100 flow that punts all IGMP/MLD packets to ``ovn-controller`` if
multicast snooping is enabled on the logical switch.
diff --git a/northd/northd.c b/northd/northd.c
index 88e3ece884..55d631e8cf 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -9220,6 +9220,7 @@ build_lb_health_check_response_lflows(
const struct ovn_northd_lb_vip *lb_vip_nb,
const struct ovn_lb_datapaths *lb_dps,
const struct ovn_datapaths *lr_datapaths,
+ const struct hmap *ls_ports,
const struct shash *meter_groups,
struct ds *match,
struct ds *action)
@@ -9228,6 +9229,7 @@ build_lb_health_check_response_lflows(
* to a real LRP, install rule that punts service check replies to the
* controller. */
const struct ovn_lb_backend *backend;
+ struct ds probe_match = DS_EMPTY_INITIALIZER;
size_t j = 0;
VECTOR_FOR_EACH_PTR (&lb_vip->backends, backend) {
@@ -9243,6 +9245,14 @@ build_lb_health_check_response_lflows(
protocol = "tcp";
}
+ /* A type=external backend has no VIF of its own. Its replies reach
+ * br-int through one of the switch's localnet ports, so they need
+ * additional lflows, see below. */
+ const struct ovn_port *backend_op =
+ ovn_port_find(ls_ports, backend_nb->logical_port);
+ bool external_backend = backend_op && backend_op->nbsp &&
+ lsp_is_external(backend_op->nbsp);
+
size_t index;
DYNAMIC_BITMAP_FOR_EACH_1 (index, &lb_dps->nb_lr_map) {
struct ovn_datapath *od = sparse_array_get(&lr_datapaths->dps,
@@ -9270,32 +9280,36 @@ build_lb_health_check_response_lflows(
ds_clear(match);
ds_clear(action);
+ ds_clear(&probe_match);
- /* icmp6 type 1 and icmp4 type 3 are included in the match, because
+ /* Everything but the inport is identical for every copy of the
+ * lflow, so build that part once.
+ *
+ * icmp6 type 1 and icmp4 type 3 are included in the match, because
* the controller is using them to detect unreachable ports. */
if (addr_is_ipv6(backend_nb->svc_mon_src_ip)) {
- ds_put_format(match, "inport == \"%s\" && ip6.dst == %s && "
+ ds_put_format(&probe_match, "ip6.dst == %s && "
"ip6.src == %s && eth.dst == %s && ",
- backend_nb->logical_port,
backend_nb->svc_mon_src_ip,
backend->ip_str,
backend_nb->svc_mon_lrp->lrp_networks.ea_s);
if (!strcmp(protocol, "tcp")) {
- ds_put_format(match, "tcp.src == %s", backend->port_str);
+ ds_put_format(&probe_match, "tcp.src == %s",
+ backend->port_str);
} else {
- ds_put_cstr(match, "icmp6.type == 1");
+ ds_put_cstr(&probe_match, "icmp6.type == 1");
}
} else {
- ds_put_format(match, "inport == \"%s\" && ip4.dst == %s && "
+ ds_put_format(&probe_match, "ip4.dst == %s && "
"ip4.src == %s && eth.dst == %s && ",
- backend_nb->logical_port,
backend_nb->svc_mon_src_ip,
backend->ip_str,
backend_nb->svc_mon_lrp->lrp_networks.ea_s);
if (!strcmp(protocol, "tcp")) {
- ds_put_format(match, "tcp.src == %s", backend->port_str);
+ ds_put_format(&probe_match, "tcp.src == %s",
+ backend->port_str);
} else {
- ds_put_cstr(match, "icmp4.type == 3");
+ ds_put_cstr(&probe_match, "icmp4.type == 3");
}
}
@@ -9305,11 +9319,45 @@ build_lb_health_check_response_lflows(
const char *meter = copp_meter_get(COPP_SVC_MONITOR,
peer_switch_od->nbs->copp,
meter_groups);
+ ds_put_format(match, "inport == \"%s\" && %s",
+ backend_nb->logical_port, ds_cstr(&probe_match));
ovn_lflow_add(lflows, peer_switch_od, S_SWITCH_IN_L2_LKUP, 110,
ds_cstr(match), "handle_svc_check(inport);",
lb_dps->lflow_ref, WITH_CTRL_METER(meter));
+
+ if (!external_backend) {
+ continue;
+ }
+
+ /* The reply from a type=external backend enters br-int through a
+ * localnet port, so MFF_LOG_INPORT holds that port and the lflow
+ * above never matches. pinctrl_find_svc_monitor() is keyed on
+ * the backend port, which means the reply is dropped and the
+ * member's health is misreported: TCP monitors time out and go
+ * permanently offline, while UDP monitors, which only ever go
+ * offline through the ICMP unreachable punted here, stay online
+ * forever.
+ *
+ * Add a copy of the lflow per localnet port that restores the
+ * backend's inport before punting. The rewrite is confined to
+ * this lflow, the action list ends with handle_svc_check() and
+ * carries no next;, so no later table can observe it. */
+ ds_put_format(action, "inport = \"%s\"; handle_svc_check(inport);",
+ backend_nb->logical_port);
+
+ struct ovn_port *lp;
+ VECTOR_FOR_EACH (&peer_switch_od->localnet_ports, lp) {
+ ds_clear(match);
+ ds_put_format(match, "inport == %s && %s",
+ lp->json_key, ds_cstr(&probe_match));
+ ovn_lflow_add(lflows, peer_switch_od, S_SWITCH_IN_L2_LKUP, 110,
+ ds_cstr(match), ds_cstr(action),
+ lb_dps->lflow_ref, WITH_CTRL_METER(meter));
+ }
}
}
+
+ ds_destroy(&probe_match);
}
static void
@@ -14115,6 +14163,7 @@ build_lrouter_flows_for_lb(struct ovn_lb_datapaths
*lb_dps,
const struct shash *meter_groups,
const struct ovn_datapaths *lr_datapaths,
const struct lr_stateful_table *lr_stateful_table,
+ const struct hmap *ls_ports,
const struct svc_monitors_map_data *svc_mons_data,
struct ds *match, struct ds *action)
{
@@ -14138,7 +14187,7 @@ build_lrouter_flows_for_lb(struct ovn_lb_datapaths
*lb_dps,
build_lb_health_check_response_lflows(
lflows, lb, lb_vip, &lb->vips_nb[i], lb_dps, lr_datapaths,
- meter_groups, match, action);
+ ls_ports, meter_groups, match, action);
if (!build_empty_lb_event_flow(lb_vip, lb, match, action)) {
continue;
@@ -20605,6 +20654,7 @@ build_lflows_thread(void *arg)
lsi->meter_groups,
lsi->lr_datapaths,
lsi->lr_stateful_table,
+ lsi->ls_ports,
&svc_mons_data,
&lsi->match, &lsi->actions);
build_lswitch_flows_for_lb(lb_dps, lsi->lflows,
@@ -20848,7 +20898,7 @@ build_lswitch_and_lrouter_flows(
lsi.lr_datapaths, &lsi.match);
build_lrouter_flows_for_lb(lb_dps, lsi.lflows, lsi.meter_groups,
lsi.lr_datapaths, lsi.lr_stateful_table,
- svc_mons_data,
+ lsi.ls_ports, svc_mons_data,
&lsi.match, &lsi.actions);
build_lswitch_flows_for_lb(lb_dps, lsi.lflows, lsi.meter_groups,
lsi.ls_datapaths,
@@ -21259,6 +21309,7 @@ lflow_handle_northd_lb_changes(struct ovsdb_idl_txn
*ovnsb_txn,
lflow_input->meter_groups,
lflow_input->lr_datapaths,
lflow_input->lr_stateful_table,
+ lflow_input->ls_ports,
&svc_mons_data,
&match, &actions);
build_lswitch_flows_for_lb(lb_dps, lflows,
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 6d191c1a0b..2d3f59546f 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1806,6 +1806,122 @@ OVN_CLEANUP_NORTHD
AT_CLEANUP
])
+OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([
+AT_SETUP([Load balancer health check reply lflows for type=external backends])
+ovn_start
+
+# Topology:
+#
+# lr0 --(lr0-sw0)-- sw0 (tenant LS, no localnet)
+# `-- vm-port (regular VIF backend)
+#
+# lr0 --(lr0-prov)-- prov (provider LS with a localnet port)
+# |-- prov-localnet (type=localnet)
+# |-- bm-tcp (type=external, TCP pool member)
+# |-- bm-udp (type=external, UDP pool member)
+# `-- prov-vm (regular VIF backend)
+#
+# A type=external LSP has no VIF of its own, so the health check reply from
+# the baremetal member re-enters br-int through the localnet port and
+# MFF_LOG_INPORT carries the localnet port, not the backend. Every external
+# backend therefore gets an extra copy of the reply lflow per localnet port
+# that restores the backend's inport before punting to the controller.
+# Backends that are not type=external keep a single lflow even when the
+# switch does have a localnet port.
+
+check ovn-nbctl lr-add lr0
+check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:01:01 10.0.0.1/24
+check ovn-nbctl lrp-add lr0 lr0-prov 00:00:00:00:02:01 10.0.50.1/24
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl --wait=sb lsp-add sw0 sw0-lr0 \
+ -- lsp-set-type sw0-lr0 router \
+ -- lsp-set-options sw0-lr0 router-port=lr0-sw0 \
+ -- lsp-set-addresses sw0-lr0 router
+check ovn-nbctl --wait=sb lsp-add sw0 vm-port \
+ -- lsp-set-addresses vm-port "00:00:00:00:01:02 10.0.0.10"
+
+check ovn-nbctl ls-add prov
+check ovn-nbctl --wait=sb lsp-add prov prov-lr0 \
+ -- lsp-set-type prov-lr0 router \
+ -- lsp-set-options prov-lr0 router-port=lr0-prov \
+ -- lsp-set-addresses prov-lr0 router
+check ovn-nbctl --wait=sb lsp-add prov prov-localnet \
+ -- lsp-set-type prov-localnet localnet \
+ -- lsp-set-options prov-localnet network_name=physnet1 \
+ -- lsp-set-addresses prov-localnet unknown
+check ovn-nbctl --wait=sb lsp-add prov bm-tcp \
+ -- lsp-set-type bm-tcp external \
+ -- lsp-set-addresses bm-tcp "00:00:00:00:02:0a 10.0.50.10"
+check ovn-nbctl --wait=sb lsp-add prov bm-udp \
+ -- lsp-set-type bm-udp external \
+ -- lsp-set-addresses bm-udp "00:00:00:00:02:0b 10.0.50.20"
+check ovn-nbctl --wait=sb lsp-add prov prov-vm \
+ -- lsp-set-addresses prov-vm "00:00:00:00:02:0c 10.0.50.30"
+
+check ovn-sbctl chassis-add hv1 geneve 127.0.0.1
+check ovn-sbctl lsp-bind vm-port hv1
+check ovn-sbctl lsp-bind bm-tcp hv1
+check ovn-sbctl lsp-bind bm-udp hv1
+check ovn-sbctl lsp-bind prov-vm hv1
+
+# TCP LB: one regular backend on sw0, one external and one regular backend
+# on the provider switch.
+check ovn-nbctl lb-add lb1 192.168.0.10:80 \
+ 10.0.0.10:80,10.0.50.10:80,10.0.50.30:80 tcp
+check ovn-nbctl --wait=sb set load_balancer lb1 \
+ ip_port_mappings:10.0.0.10=vm-port:10.0.0.1
+check ovn-nbctl --wait=sb set load_balancer lb1 \
+ ip_port_mappings:10.0.50.10=bm-tcp:10.0.50.1
+check ovn-nbctl --wait=sb set load_balancer lb1 \
+ ip_port_mappings:10.0.50.30=prov-vm:10.0.50.1
+check_uuid ovn-nbctl --wait=sb -- --id=@hc create Load_Balancer_Health_Check \
+ vip="192.168.0.10\:80" -- add Load_Balancer lb1 health_check @hc
+
+# UDP LB with an external backend. A UDP monitor is only ever moved to
+# offline by the ICMP unreachable punted through this lflow.
+check ovn-nbctl lb-add lb2 192.168.0.20:80 10.0.50.20:80 udp
+check ovn-nbctl --wait=sb set load_balancer lb2 \
+ ip_port_mappings:10.0.50.20=bm-udp:10.0.50.1
+check_uuid ovn-nbctl --wait=sb -- --id=@hc create Load_Balancer_Health_Check \
+ vip="192.168.0.20\:80" -- add Load_Balancer lb2 health_check @hc
+
+check ovn-nbctl lr-lb-add lr0 lb1
+check ovn-nbctl lr-lb-add lr0 lb2
+check ovn-nbctl ls-lb-add sw0 lb1
+check ovn-nbctl ls-lb-add prov lb1
+check ovn-nbctl --wait=sb ls-lb-add prov lb2
+
+AS_BOX([Regular VIF backend on a switch without a localnet port.])
+AT_CAPTURE_FILE([sw0_lflows])
+AT_CHECK([ovn-sbctl dump-flows sw0 | tee sw0_lflows | grep ls_in_l2_lkup dnl
+ | grep handle_svc_check | ovn_strip_lflows], [0], [dnl
+ table=??(ls_in_l2_lkup ), priority=110 , match=(eth.dst ==
$svc_monitor_mac && (tcp || icmp || icmp6)), action=(handle_svc_check(inport);)
+ table=??(ls_in_l2_lkup ), priority=110 , match=(inport == "vm-port" &&
ip4.dst == 10.0.0.1 && ip4.src == 10.0.0.10 && eth.dst == 00:00:00:00:01:01 &&
tcp.src == 80), action=(handle_svc_check(inport);)
+])
+
+AS_BOX([External backends get a localnet copy, the regular one does not.])
+AT_CAPTURE_FILE([prov_lflows])
+AT_CHECK([ovn-sbctl dump-flows prov | tee prov_lflows | grep ls_in_l2_lkup dnl
+ | grep handle_svc_check | ovn_strip_lflows], [0], [dnl
+ table=??(ls_in_l2_lkup ), priority=110 , match=(eth.dst ==
$svc_monitor_mac && (tcp || icmp || icmp6)), action=(handle_svc_check(inport);)
+ table=??(ls_in_l2_lkup ), priority=110 , match=(inport == "bm-tcp" &&
ip4.dst == 10.0.50.1 && ip4.src == 10.0.50.10 && eth.dst == 00:00:00:00:02:01
&& tcp.src == 80), action=(handle_svc_check(inport);)
+ table=??(ls_in_l2_lkup ), priority=110 , match=(inport == "bm-udp" &&
ip4.dst == 10.0.50.1 && ip4.src == 10.0.50.20 && eth.dst == 00:00:00:00:02:01
&& icmp4.type == 3), action=(handle_svc_check(inport);)
+ table=??(ls_in_l2_lkup ), priority=110 , match=(inport ==
"prov-localnet" && ip4.dst == 10.0.50.1 && ip4.src == 10.0.50.10 && eth.dst ==
00:00:00:00:02:01 && tcp.src == 80), action=(inport = "bm-tcp";
handle_svc_check(inport);)
+ table=??(ls_in_l2_lkup ), priority=110 , match=(inport ==
"prov-localnet" && ip4.dst == 10.0.50.1 && ip4.src == 10.0.50.20 && eth.dst ==
00:00:00:00:02:01 && icmp4.type == 3), action=(inport = "bm-udp";
handle_svc_check(inport);)
+ table=??(ls_in_l2_lkup ), priority=110 , match=(inport == "prov-vm" &&
ip4.dst == 10.0.50.1 && ip4.src == 10.0.50.30 && eth.dst == 00:00:00:00:02:01
&& tcp.src == 80), action=(handle_svc_check(inport);)
+])
+
+AS_BOX([Removing the localnet port removes the extra lflows.])
+check ovn-nbctl --wait=sb lsp-del prov-localnet
+AT_CHECK([ovn-sbctl dump-flows prov | grep ls_in_l2_lkup dnl
+ | grep 'inport = ' | wc -l | tr -d ' '], [0], [0
+])
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
+
OVN_FOR_EACH_NORTHD_NO_HV([
AT_SETUP([Load balancer VIP in NAT entries])
AT_SKIP_IF([test $HAVE_PYTHON = no])
--
2.49.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
