The ovn-ic instance holding the IC-SB lock writes, among others, the
transit switch and router Datapath_Bindings through the locked IC-SB
connection. Its transaction isn't available while the previous one is
being committed, but ovn-ic runs the engine anyway. The nodes then skip
their IC-SB writes: the en_tunnel_key handlers fall back to a recompute,
which skips them as well, and the tracked changes are cleared, so e.g.
the Datapath_Binding of a deleted transit switch isn't garbage collected
until a later recompute.
Postpone the engine run while the locked IC-SB transaction isn't
available, keeping the tracked changes, as is already done for the other
connections.
Also, ignore the IC-SB Datapath_Binding deletions whose IC-NB transit
switch or router is gone in en_tunnel_key. They are the echo of its own
garbage collection and there is nothing to reconcile for them.
Both made the "transit switch handler scoping" test flaky, as deleting
a transit switch could trigger a recompute of en_tunnel_key.
Fixes: 0ad737079717 ("ic: Add the en_tunnel_key node.")
Signed-off-by: Paulo Guilherme Silva <[email protected]>
---
ic/en-tunnel-key.c | 30 ++++++++++++++++++++++++++----
ic/ovn-ic.c | 11 ++++++++++-
2 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/ic/en-tunnel-key.c b/ic/en-tunnel-key.c
index 0a63a6c90..834c2143e 100644
--- a/ic/en-tunnel-key.c
+++ b/ic/en-tunnel-key.c
@@ -377,11 +377,28 @@ en_tunnel_key_icnb_transit_router_handler(struct
engine_node *node,
return ret;
}
+static bool
+icnb_transit_switch_exists(struct ic_context *ctx, const char *name)
+{
+ const struct icnbrec_transit_switch *key =
+ icnbrec_transit_switch_index_init_row(
+ ctx->icnbrec_transit_switch_by_name);
+ icnbrec_transit_switch_index_set_name(key, name);
+ const struct icnbrec_transit_switch *ts =
+ icnbrec_transit_switch_index_find(ctx->icnbrec_transit_switch_by_name,
+ key);
+ icnbrec_transit_switch_index_destroy_row(key);
+ return ts != NULL;
+}
+
/* IC-SB Datapath_Binding: react only to a binding *deletion* by re-creating it
* for the still-present IC-NB entity (self-healing after an out-of-band
- * delete). Inserts and modifies are this node's own writes (or already
- * consistent), so they are a no-op here - the requested-tnl-key follow-up is
- * driven by en_ts/en_tr's own handlers on this same table. */
+ * delete). The deletion of a binding whose IC-NB entity is gone is the echo
+ * of this node's own garbage collection and needs nothing, so it must not
+ * depend on the IC-SB transaction being available. Inserts and modifies are
+ * this node's own writes (or already consistent), so they are a no-op here -
+ * the requested-tnl-key follow-up is driven by en_ts/en_tr's own handlers on
+ * this same table. */
enum engine_input_handler_result
en_tunnel_key_icsb_datapath_binding_handler(struct engine_node *node,
void *data_)
@@ -402,10 +419,15 @@ en_tunnel_key_icsb_datapath_binding_handler(struct
engine_node *node,
/* Not inserted by ovn-ic: nothing to reconcile. */
continue;
}
+ if (!icnbrec_transit_router_get_for_uuid(ctx->ovninb_idl,
+ isb_dp->nb_ic_uuid)) {
+ continue;
+ }
char *uuid_str = uuid_to_string(isb_dp->nb_ic_uuid);
sset_add(&tr_scope, uuid_str);
free(uuid_str);
- } else {
+ } else if (icnb_transit_switch_exists(ctx,
+ isb_dp->transit_switch)) {
sset_add(&ts_scope, isb_dp->transit_switch);
}
}
diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
index 7e9b3139f..e65256577 100644
--- a/ic/ovn-ic.c
+++ b/ic/ovn-ic.c
@@ -1449,8 +1449,17 @@ main(int argc, char *argv[])
ovsdb_idl_has_ever_connected(ctx.ovninb_idl) &&
ovsdb_idl_has_ever_connected(ctx.ovnisb_idl) &&
ovsdb_idl_has_ever_connected(ctx.ovnisb_unlocked_idl)) {
+ /* The AZ holding the IC-SB lock writes through the locked
+ * connection, whose transaction isn't available while the
+ * previous one is being committed. Postpone the engine run
+ * in that case, keeping the tracked changes, as the nodes
+ * would otherwise skip their writes and the changes they
+ * handle incrementally would be lost. */
+ bool isb_txn_ready = ctx.ovnisb_txn ||
+ !ovsdb_idl_has_lock(ctx.ovnisb_idl);
if (ctx.ovnnb_txn && ctx.ovnsb_txn && ctx.ovninb_txn &&
- ctx.ovnisb_unlocked_txn && inc_proc_ic_can_run(&eng_ctx)) {
+ ctx.ovnisb_unlocked_txn && isb_txn_ready &&
+ inc_proc_ic_can_run(&eng_ctx)) {
/* The availability zone is now resolved by the en_az
* engine node. The subsystem nodes skip their work when
* there is no AZ; the sequence-number bookkeeping below
--
2.34.1
--
_'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
