build_lb_health_check_response_lflows() builds a per-backend
service_monitor reply punt lflow with the match:
inport == "<backend_lsp>" && ip4.dst == <svc_mon_src_ip> &&
ip4.src == <backend_ip> && eth.dst == <svc_mon_lrp_mac> &&
tcp.src == <backend_port>
When the backend's LSP is type=external on a Logical_Switch that has
a localnet port (the typical configuration produced by Neutron /
ovn-octavia-provider for baremetal pool members on a provider VLAN),
the response packet re-enters br-int through the localnet LSP, so
MFF_LOG_INPORT at S_SWITCH_IN_L2_LKUP is set to the localnet LSP's
tunnel_key, not the backend LSP's. The existing inport == "..."
clause therefore never matches, the response is never punted to
ovn-controller via ACTION_OPCODE_HANDLE_SVC_CHECK, and the
Service_Monitor.status of those backends stays "offline"
indefinitely -- even though the backend physically replies on the
wire.
Fix this by emitting an additional reply punt lflow on every
peer_switch_od that has a localnet port AND a type=external backend
LSP. The new lflow drops the inport == "..." constraint, replaces
it with eth.src == <backend_mac>, and uses
outport = "<backend_lsp>";
handle_svc_check(outport);
outport = "";
so MFF_LOG_INPORT is loaded with the backend LSP's tunnel_key for
the controller dispatch (handle_svc_check(outport) borrows reg15
into reg14 via push/pop), and outport is then cleared to prevent
the cleared MFF_LOG_OUTPORT from leaking into egress and reflecting
the reply back to the backend.
The patch does not introduce or change any OVN action, so
OVN_INTERNAL_MINOR_VER is not bumped.
Signed-off-by: JayGue Lee <[email protected]>
---
v2:
- Fixed SIGSEGV reported by the ovsrobot CI on v1. v1 used
ovn_port_find(&peer_switch_od->ports, ...) to look up the backend
ovn_port by its LSP name. ovn_port_find() walks the hmap with
HMAP_FOR_EACH_WITH_HASH using the 'key_node' hmap_node, but
ovn_datapath::ports is keyed on 'dp_node' (per ovn_port_alloc()
paths in northd.c that hmap_insert() with &op->dp_node), so the
iteration container_of()'s a wrong field offset and produces a
garbage struct ovn_port * that crashes on the first deref. Switch
to an inline HMAP_FOR_EACH (op, dp_node, &peer_switch_od->ports)
loop matching by op->key, which is the established pattern used
elsewhere in northd.c (e.g. ovn_port_find_port_or_child()) for
per-datapath port lookups.
- No functional change in the lflow that is generated; only the way
the corresponding ovn_port is located.
NEWS | 7 ++++
northd/northd.c | 83 +++++++++++++++++++++++++++++++++++++++++++
tests/ovn-northd.at | 86 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 176 insertions(+)
diff --git a/NEWS b/NEWS
index 8633ba8..b2a956c 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,13 @@ Post v26.03.0
- Dynamic Routing:
* Add support for hub-and-spoke propagation via the "hub-spoke" option
in dynamic-routing-redistribute settings.
+ - Fixed Load_Balancer health checks for backends referenced via a
+ Logical_Switch_Port whose type is "external" on a Logical_Switch that
+ also has a localnet port (typical Neutron / ovn-octavia-provider
+ baremetal pool member configuration). Previously such backends
+ remained permanently offline in Service_Monitor.status because the
+ per-backend reply punt lflow's "inport == <backend_lsp>" match did
+ not fit traffic that re-enters br-int through the localnet port.
OVN v26.03.0 - xxx xx xxxx
--------------------------
diff --git a/northd/northd.c b/northd/northd.c
index 0b52db6..879bfd1 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -8962,6 +8962,89 @@ build_lb_health_check_response_lflows(
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));
+
+ /* The lflow above only matches replies whose ingress logical
+ * inport is the backend LSP itself. This holds for backends
+ * behind tunnels or local OVS ports. For backends whose LSP
+ * is type=external on a Logical_Switch that has a localnet
+ * port (typical for Neutron / ovn-octavia-provider baremetal
+ * pool members on a provider VLAN), the reply re-enters
+ * br-int through the localnet port; MFF_LOG_INPORT therefore
+ * holds the localnet LSP's tunnel_key and the lflow above
+ * never matches. pinctrl_handle_svc_check() then fails its
+ * (dp_key, port_key) lookup and Service_Monitor.status stays
+ * 'offline' indefinitely, even though the backend is fully
+ * reachable on the wire.
+ *
+ * Emit an additional reply punt lflow that:
+ * - identifies the backend by eth.src instead of inport,
+ * - loads the backend LSP's tunnel_key into MFF_LOG_INPORT
+ * via 'outport = "<lsp>"; handle_svc_check(outport);'
+ * (the latter borrows MFF_LOG_OUTPORT's value into
+ * MFF_LOG_INPORT for the controller dispatch),
+ * - resets outport to "" so the cleared MFF_LOG_OUTPORT
+ * does not leak into egress and reflect the reply back
+ * to the backend.
+ */
+ if (vector_is_empty(&peer_switch_od->localnet_ports)) {
+ continue;
+ }
+ /* peer_switch_od->ports is an hmap keyed by 'dp_node' (not the
+ * 'key_node' that ovn_port_find() expects), so iterate it with
+ * HMAP_FOR_EACH directly to find the backend LSP by name. */
+ struct ovn_port *backend_op = NULL;
+ struct ovn_port *op;
+ HMAP_FOR_EACH (op, dp_node, &peer_switch_od->ports) {
+ if (op->nbsp && op->key
+ && !strcmp(op->key, backend_nb->logical_port)) {
+ backend_op = op;
+ break;
+ }
+ }
+ if (!backend_op || !backend_op->nbsp || !backend_op->nbsp->type
+ || strcmp(backend_op->nbsp->type, "external")
+ || !backend_op->n_lsp_addrs) {
+ continue;
+ }
+
+ ds_clear(match);
+ ds_clear(action);
+
+ const char *bm_mac = backend_op->lsp_addrs[0].ea_s;
+ if (addr_is_ipv6(backend_nb->svc_mon_src_ip)) {
+ ds_put_format(match, "eth.src == %s && ip6.dst == %s && "
+ "ip6.src == %s && eth.dst == %s && ",
+ bm_mac,
+ 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);
+ } else {
+ ds_put_cstr(match, "icmp6.type == 1");
+ }
+ } else {
+ ds_put_format(match, "eth.src == %s && ip4.dst == %s && "
+ "ip4.src == %s && eth.dst == %s && ",
+ bm_mac,
+ 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);
+ } else {
+ ds_put_cstr(match, "icmp4.type == 3");
+ }
+ }
+ ds_put_format(action,
+ "outport = \"%s\"; handle_svc_check(outport); "
+ "outport = \"\";",
+ backend_nb->logical_port);
+ 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));
}
}
}
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 1d7bd6c..c95ce39 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -20672,3 +20672,89 @@ check_column "$global_svc_mon_mac" sb:Service_Monitor
src_mac port=2
OVN_CLEANUP_NORTHD
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD_NO_HV_PARALLELIZATION([
+AT_SETUP([Load balancer health checks - external LSP backend on localnet LS])
+ovn_start
+
+dnl Topology:
+dnl lr0 -- ls-tenant (vm-port: type="", 10.0.0.10)
+dnl \--- ls-prov (bm-port: type=external, 192.168.0.10
+dnl MAC 02:bb:bb:00:00:01;
+dnl provnet-physnet1: type=localnet)
+dnl
+dnl LB lb0 attached to lr0:
+dnl vip 10.10.10.10:80 -> 10.0.0.10:80,192.168.0.10:80 (tcp)
+dnl ip_port_mappings: 10.0.0.10 = vm-port:10.0.0.1
+dnl 192.168.0.10 = bm-port:192.168.0.1
+dnl
+dnl Expectation:
+dnl 1) On ls-tenant the existing per-backend reply lflow is emitted
+dnl with `inport == "vm-port"`.
+dnl 2) On ls-prov the existing per-backend reply lflow is emitted
+dnl with `inport == "bm-port"` AND, additionally, the new lflow
+dnl added by this commit is emitted with `eth.src == <bm-mac>`
+dnl and action `outport = "bm-port"; handle_svc_check(outport);
+dnl outport = "";`.
+
+check ovn-nbctl \
+ -- lr-add lr0 \
+ -- lrp-add lr0 lr0-tenant 02:00:00:00:01:01 10.0.0.1/24 \
+ -- lrp-add lr0 lr0-prov 02:00:00:00:02:01 192.168.0.1/24
+
+check ovn-nbctl \
+ -- ls-add ls-tenant \
+ -- lsp-add ls-tenant ls-tenant-lr0 \
+ -- lsp-set-type ls-tenant-lr0 router \
+ -- lsp-set-options ls-tenant-lr0 router-port=lr0-tenant \
+ -- lsp-set-addresses ls-tenant-lr0 router \
+ -- lsp-add ls-tenant vm-port \
+ -- lsp-set-addresses vm-port "02:aa:aa:00:00:01 10.0.0.10"
+
+check ovn-nbctl \
+ -- ls-add ls-prov \
+ -- lsp-add ls-prov ls-prov-lr0 \
+ -- lsp-set-type ls-prov-lr0 router \
+ -- lsp-set-options ls-prov-lr0 router-port=lr0-prov \
+ -- lsp-set-addresses ls-prov-lr0 router \
+ -- lsp-add ls-prov bm-port \
+ -- lsp-set-type bm-port external \
+ -- lsp-set-addresses bm-port "02:bb:bb:00:00:01 192.168.0.10" \
+ -- lsp-add ls-prov provnet-physnet1 \
+ -- lsp-set-type provnet-physnet1 localnet \
+ -- lsp-set-addresses provnet-physnet1 unknown \
+ -- lsp-set-options provnet-physnet1 network_name=physnet1
+
+check ovn-nbctl --wait=sb lb-add lb0 \
+ 10.10.10.10:80 10.0.0.10:80,192.168.0.10:80 tcp
+lb0=$(fetch_column nb:Load_Balancer _uuid name=lb0)
+check ovn-nbctl \
+ -- set load_balancer $lb0 ip_port_mappings:10.0.0.10=vm-port:10.0.0.1 \
+ -- set load_balancer $lb0 ip_port_mappings:192.168.0.10=bm-port:192.168.0.1
+
+check_uuid ovn-nbctl --id=@hc create Load_Balancer_Health_Check \
+ vip="10.10.10.10\:80" -- add Load_Balancer $lb0 health_check @hc
+check ovn-nbctl --wait=sb lr-lb-add lr0 lb0
+
+dnl Existing per-backend reply lflow on the tenant (VM) datapath.
+AT_CHECK([ovn-sbctl lflow-list ls-tenant | 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 == 02:00:00:00:01:01 &&
tcp.src == 80), action=(handle_svc_check(inport);)
+])
+
+dnl On the provider datapath, BOTH the existing per-backend reply lflow
+dnl AND the new external-LSP reply lflow added by this commit must be
+dnl present. The new lflow uses eth.src to identify the backend (so it
+dnl matches replies that re-enter via the localnet port) and forces
+dnl MFF_LOG_INPORT to the backend LSP's tunnel_key by going through
+dnl outport, then resets outport to "" to avoid reflecting the reply
+dnl back to the backend.
+AT_CHECK([ovn-sbctl lflow-list ls-prov | 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=(eth.src ==
02:bb:bb:00:00:01 && ip4.dst == 192.168.0.1 && ip4.src == 192.168.0.10 &&
eth.dst == 02:00:00:00:02:01 && tcp.src == 80), action=(outport = "bm-port";
handle_svc_check(outport); outport = "";)
+ table=??(ls_in_l2_lkup ), priority=110 , match=(inport == "bm-port" &&
ip4.dst == 192.168.0.1 && ip4.src == 192.168.0.10 && eth.dst ==
02:00:00:00:02:01 && tcp.src == 80), action=(handle_svc_check(inport);)
+])
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
--
2.49.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
