Until now any change to a logical switch port of type "virtual" fell back
to a full northd recompute, because lsp_can_be_inc_processed() only allowed
plain VIF and remote ports. Enable incremental processing for creation and
deletion of virtual ports as well.
The per-port lflows of a virtual port (the "bind_vport" ARP/ND responder
flows) are anchored on the port's own lflow_ref and are already handled by
the generic created/deleted tracked-port paths in the lflow engine. The
dependency on the virtual parents is likewise already handled (existing
virtual ports are re-tracked when a parent VIF is created or deleted).
Some virtual ports, however, have dependencies that live outside of their
own lflow_ref and that the incremental LSP path does not keep in sync. For
those we detect the situation and fall back to a full recompute:
- A distributed NAT rule whose logical_port is the virtual port adds a
S_ROUTER_IN_GW_REDIRECT drop flow owned by the router datapath's
lflow_ref (see build_lrouter_nat_defrag_and_lb()).
- Dynamic routing on a connected router advertises host routes for a
virtual port based on its SB "virtual_parent", i.e. once the port is
claimed. The claim reaches northd as an "up"-only change on the NB
port, which the incremental path otherwise ignores, so the advertised
route engine would not be re-run. The fallback is therefore also
applied on the update path, before the "ignore_lsp_down" shortcut.
Add tests covering incremental create/delete of a virtual port as well as
the recompute fallbacks for the distributed-NAT and dynamic-routing cases.
Assisted-by: Claude Opus 4.8, ClaudeCode
Signed-off-by: Lucas Vargas Dias <[email protected]>
---
northd/northd.c | 75 +++++++++++++++++++++++++-
tests/ovn-northd.at | 129 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 202 insertions(+), 2 deletions(-)
diff --git a/northd/northd.c b/northd/northd.c
index 3a4afa063..4f4c0733a 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -1278,6 +1278,12 @@ lsp_is_remote(const struct nbrec_logical_switch_port
*nbsp)
return !strcmp(nbsp->type, "remote");
}
+static bool
+lsp_is_virtual(const struct nbrec_logical_switch_port *nbsp)
+{
+ return !strcmp(nbsp->type, "virtual");
+}
+
static bool
lsp_is_localnet(const struct nbrec_logical_switch_port *nbsp)
{
@@ -4620,8 +4626,8 @@ destroy_northd_tracked_data(struct northd_data *nd)
static bool
lsp_can_be_inc_processed(const struct nbrec_logical_switch_port *nbsp)
{
- /* Support only normal VIF and remote ports for now. */
- if (nbsp->type[0] && !lsp_is_remote(nbsp)) {
+ /* Support only normal VIF, remote and virtual ports for now. */
+ if (nbsp->type[0] && !lsp_is_remote(nbsp) && !lsp_is_virtual(nbsp)) {
return false;
}
@@ -4666,6 +4672,48 @@ lsp_can_be_inc_processed(const struct
nbrec_logical_switch_port *nbsp)
return true;
}
+/* A logical switch port of type "virtual" can have dependencies that live
+ * outside of its own 'lflow_ref' and that the incremental LSP path does not
+ * keep in sync. When any such dependency is present on a logical router
+ * connected to the port's logical switch, the caller must fall back to a full
+ * recompute. The known dependencies are:
+ *
+ * - A distributed NAT rule whose 'logical_port' is this virtual port. It
+ * adds a S_ROUTER_IN_GW_REDIRECT drop flow (see
+ * build_lrouter_nat_defrag_and_lb()) owned by the router datapath's
+ * 'lflow_ref', not by the virtual port.
+ *
+ * - Dynamic routing on the connected router. Host routes for a virtual port
+ * are advertised based on its SB 'virtual_parent' (i.e. once the port is
+ * claimed, see publish_host_routes_for_virtual_ports()). The claim reaches
+ * northd as an "up"-only change on the NB port, which the incremental path
+ * ignores, so the advertised-route engine would not be re-run. */
+static bool
+virtual_lsp_needs_recompute(struct ovn_datapath *od, const char *lport)
+{
+ struct ovn_port *rp;
+ VECTOR_FOR_EACH (&od->router_ports, rp) {
+ struct ovn_port *lrp = rp->peer;
+ if (!lrp || !lrp->od || !lrp->od->nbr) {
+ continue;
+ }
+
+ if (lrp->od->dynamic_routing) {
+ return true;
+ }
+
+ const struct nbrec_logical_router *nbr = lrp->od->nbr;
+ for (size_t i = 0; i < nbr->n_nat; i++) {
+ const struct nbrec_nat *nat = nbr->nat[i];
+ if (nat->logical_port && !strcmp(nat->logical_port, lport) &&
+ is_nat_distributed(nat, lrp->od)) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
static bool
ls_port_has_changed(const struct nbrec_logical_switch_port *new)
{
@@ -4963,6 +5011,13 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
*ovnsb_idl_txn,
if (!lsp_can_be_inc_processed(new_nbsp)) {
goto fail;
}
+ if (lsp_is_virtual(new_nbsp) &&
+ virtual_lsp_needs_recompute(od, new_nbsp->name)) {
+ /* The new virtual port has a dependency on a connected
+ * router that can't be handled incrementally. Fall back
+ * to recompute. */
+ goto fail;
+ }
op = ls_port_create(ovnsb_idl_txn, &nd->ls_ports,
new_nbsp->name, new_nbsp, od,
ni->sbrec_mirror_table,
@@ -4981,6 +5036,15 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
*ovnsb_idl_txn,
!lsp_can_be_inc_processed(new_nbsp)) {
goto fail;
}
+ if (lsp_is_virtual(new_nbsp) &&
+ virtual_lsp_needs_recompute(od, new_nbsp->name)) {
+ /* This virtual port has a dependency on a connected router
+ * that can't be handled incrementally. In particular a
+ * claim (which reaches northd as an "up"-only change) must
+ * re-run the advertised-route engine. Fall back to
+ * recompute. */
+ goto fail;
+ }
const struct sbrec_port_binding *sb = op->sb;
if (sset_contains(&nd->svc_monitor_lsps, new_nbsp->name)) {
/* This port is used for svc monitor, which may be impacted
@@ -5038,6 +5102,13 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn
*ovnsb_idl_txn,
if (!op->lsp_can_be_inc_processed) {
goto fail;
}
+ if (lsp_is_virtual(op->nbsp) &&
+ virtual_lsp_needs_recompute(op->od, op->key)) {
+ /* This virtual port has a dependency on a connected router
+ * that can't be regenerated incrementally. Fall back to
+ * recompute. */
+ goto fail;
+ }
if (sset_contains(&nd->svc_monitor_lsps, op->key)) {
/* This port was used for svc monitor, which may be
* impacted by this deletion. Fallback to recompute. */
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 86cab3d5b..ed7aef72f 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -12043,6 +12043,135 @@ ignored_dp=ls0
AT_CLEANUP
])
+AT_SETUP([Virtual port incremental processing])
+AT_KEYWORDS([incremental processing])
+ovn_start
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl --wait=sb lsp-add sw0 vif0 \
+ -- lsp-set-addresses vif0 "00:00:00:00:00:01 10.0.0.4"
+
+# Creating a virtual port should be incrementally processed.
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb lsp-add sw0 vip0 \
+ -- lsp-set-type vip0 virtual \
+ -- set Logical_Switch_Port vip0 \
+ options:virtual-ip=10.0.0.10 options:virtual-parents=vif0
+check_engine_compute northd incremental
+check_engine_compute lflow incremental
+
+# The bind_vport flow for the virtual port is present.
+AT_CHECK([ovn-sbctl dump-flows sw0 | grep ls_in_arp_rsp | grep bind_vport \
+ | ovn_strip_lflows], [0], [dnl
+ table=??(ls_in_arp_rsp ), priority=100 , match=(inport == "vif0" &&
((arp.op == 1 && arp.spa == 10.0.0.10 && arp.tpa == 10.0.0.10) || (arp.op == 2
&& arp.spa == 10.0.0.10))), action=(bind_vport("vip0", inport); next;)
+])
+
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+# Deleting a virtual port should be incrementally processed.
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb lsp-del vip0
+check_engine_compute northd incremental
+check_engine_compute lflow incremental
+
+# The bind_vport flow for the virtual port is gone.
+AT_CHECK([ovn-sbctl dump-flows sw0 | grep ls_in_arp_rsp | grep bind_vport \
+ | ovn_strip_lflows], [0], [])
+
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+
+AT_SETUP([Virtual port incremental processing fallback with distributed NAT])
+AT_KEYWORDS([incremental processing])
+ovn_start
+
+check ovn-sbctl chassis-add gw1 geneve 127.0.0.1
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl lr-add lr0
+check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
+check ovn-nbctl --wait=sb lsp-add-router-port sw0 sw0-lr0 lr0-sw0
+
+# Distributed gateway port so that dnat_and_snat NATs become distributed.
+check ovn-nbctl ls-add public
+check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 172.168.0.100/24
+check ovn-nbctl --wait=sb lsp-add-router-port public public-lr0 lr0-public
+check ovn-nbctl lsp-add-localnet-port public ln-public public
+check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr0-public gw1 20
+
+# Parent VIF and the virtual port. No NAT references it yet, so creating it is
+# incrementally processed.
+check ovn-nbctl --wait=sb lsp-add sw0 vif0 \
+ -- lsp-set-addresses vif0 "00:00:20:20:12:01 10.0.0.4"
+check ovn-nbctl --wait=sb lsp-add sw0 vip0 \
+ -- lsp-set-type vip0 virtual \
+ -- set Logical_Switch_Port vip0 \
+ options:virtual-ip=10.0.0.5 options:virtual-parents=vif0
+
+# Distributed dnat_and_snat whose logical_port is the virtual port 'vip0'.
+# The router's S_ROUTER_IN_GW_REDIRECT drop flow for this NAT depends on
+# 'vip0' being a virtual port, but that flow is owned by the router datapath's
+# lflow_ref, not by 'vip0'.
+check ovn-nbctl --wait=sb lr-nat-add lr0 dnat_and_snat \
+ 172.168.0.110 10.0.0.5 vip0 30:54:00:00:00:03
+
+# Deleting the virtual port must fall back to recompute.
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb lsp-del vip0
+check_engine_compute northd recompute
+
+# Re-creating the virtual port (still referenced by the NAT) must also fall
+# back to recompute.
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb lsp-add sw0 vip0 \
+ -- lsp-set-type vip0 virtual \
+ -- set Logical_Switch_Port vip0 \
+ options:virtual-ip=10.0.0.5 options:virtual-parents=vif0
+check_engine_compute northd recompute
+
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+
+AT_SETUP([Virtual port incremental processing fallback with dynamic routing])
+AT_KEYWORDS([incremental processing])
+ovn_start
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl lr-add lr0 \
+ -- set Logical_Router lr0 options:dynamic-routing=true
+check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 \
+ -- set Logical_Router_Port lr0-sw0 \
+ options:dynamic-routing-redistribute=connected-as-host
+check ovn-nbctl --wait=sb lsp-add-router-port sw0 sw0-lr0 lr0-sw0
+
+check ovn-nbctl --wait=sb lsp-add sw0 vif0 \
+ -- lsp-set-addresses vif0 "00:00:00:00:00:01 10.0.0.4"
+
+# Creating a virtual port on a switch connected to a dynamic-routing router
+# must fall back to recompute: advertised host routes for virtual ports depend
+# on the SB "virtual_parent" (claim), which the incremental path does not
+# track.
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb lsp-add sw0 vip0 \
+ -- lsp-set-type vip0 virtual \
+ -- set Logical_Switch_Port vip0 \
+ options:virtual-ip=10.0.0.5 options:virtual-parents=vif0
+check_engine_compute northd recompute
+
+# Deleting it must also fall back to recompute.
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb lsp-del vip0
+check_engine_compute northd recompute
+
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+
OVN_FOR_EACH_NORTHD_NO_HV([
AT_SETUP([SB Port binding incremental processing])
ovn_start
--
2.43.0
--
_'Esta mensagem é direcionada apenas para os endereços constantes no
cabeçalho inicial. Se você não está listado nos endereços constantes no
cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa
mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão
imediatamente anuladas e proibidas'._
* **'Apesar do Magazine Luiza tomar
todas as precauções razoáveis para assegurar que nenhum vírus esteja
presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por
quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.*
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
