When the first workload of a datapath connected to a distributed gateway port is bound to a chassis, that chassis becomes a reference chassis of the HA chassis group of the port and every chassis of the group enables BFD on its tunnel towards it. The opposite happens when the last workload is removed. Runtime data doesn't handle changes to tunnel interfaces, so the bfd and bfd_status updates trigger a full recompute on every gateway of the group, usually more than once per event as bfd_status changes while the session comes up.
Runtime data only uses the tunnel interfaces to calculate the active tunnels, whose consumers only use them to find out whether the local chassis is the active chassis of an HA chassis group. So, if the local chassis belongs to an HA chassis group, recalculate the active tunnels on tunnel interface changes and only recompute if the groups for which the local chassis is active change, i.e., on a failover. Chassis that don't belong to any HA chassis group keep recomputing, as it is the recompute that removes the datapaths that are no longer local once they stop being a reference chassis. Assisted-by: Claude Opus 4.8, Claude Code Signed-off-by: Paulo Guilherme Silva <[email protected]> --- controller/binding.c | 12 ++++++ controller/binding.h | 4 ++ controller/ovn-controller.c | 80 +++++++++++++++++++++++++++++++++---- tests/ovn-controller.at | 60 ++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 8 deletions(-) diff --git a/controller/binding.c b/controller/binding.c index c2bce665e..3e8114e7e 100644 --- a/controller/binding.c +++ b/controller/binding.c @@ -2696,6 +2696,12 @@ consider_iface_release(const struct ovsrec_interface *iface_rec, return true; } +static bool +is_iface_tunnel(const struct ovsrec_interface *iface_rec) +{ + return smap_get(&iface_rec->options, "remote_ip") != NULL; +} + static bool is_iface_vif(const struct ovsrec_interface *iface_rec) { @@ -2824,6 +2830,12 @@ binding_handle_ovs_interface_changes(struct binding_ctx_in *b_ctx_in, const char *iface_id = smap_get(&iface_rec->external_ids, "iface-id"); const char *old_iface_id = smap_get(b_ctx_out->local_iface_ids, iface_rec->name); + if (!iface_id && !old_iface_id && is_iface_tunnel(iface_rec)) { + /* Tunnel interfaces are only relevant for the active tunnels + * which the caller is expected to recalculate. */ + b_ctx_out->tunnel_ifaces_changed = true; + continue; + } if (!iface_id && !old_iface_id && !is_iface_vif(iface_rec)) { /* Right now we are not handling ovs_interface changes if the * interface doesn't have iface-id or didn't have it diff --git a/controller/binding.h b/controller/binding.h index beca9cda9..817c4e296 100644 --- a/controller/binding.h +++ b/controller/binding.h @@ -117,6 +117,10 @@ struct binding_ctx_out { bool localnet_learn_fdb; bool localnet_learn_fdb_changed; + + /* Track if any tunnel OVS interface has been added, deleted or + * updated. */ + bool tunnel_ifaces_changed; }; /* Local bindings. binding.c module binds the logical port (represented by diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index ee49e8153..7bc3139ab 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -1444,15 +1444,18 @@ struct ed_type_runtime_data { * | qos_map | engine (used only in binding.c) and hence there | * | | there is no need to track. | * --------------------------------------------------------------------- - * | | Active tunnels is built in the | + * | active_tunnels | Active tunnels is built in the | * | | bfd_calculate_active_tunnels() for the tunnel | - * | | OVS interfaces. Any changes to non VIF OVS | - * | | interfaces results in triggering the full | - * | active_tunnels | recompute of runtime data engine and hence there | - * | | the tracked data doesn't track it. When we | - * | | support handling changes to non VIF OVS | - * | | interfaces we need to track the changes to the | - * | | active tunnels. | + * | | OVS interfaces. Its consumers only use it to | + * | | find out whether the local chassis is the active | + * | | chassis of an HA chassis group. If the local | + * | | chassis belongs to an HA chassis group, changes | + * | | to the tunnel OVS interfaces that don't change | + * | | active_ha_groups are handled without changing | + * | | any other runtime data. Any other change to non | + * | | VIF OVS interfaces triggers the full recompute | + * | | of runtime data engine and hence the tracked | + * | | data doesn't track it. | * --------------------------------------------------------------------- * | local_ha_groups | HA chassis groups that contain the local chassis | * | active_ha_groups | and the ones for which it is the active chassis. | @@ -1616,6 +1619,7 @@ init_binding_ctx(struct engine_node *node, b_ctx_out->if_mgr = ctrl_ctx->if_mgr; b_ctx_out->localnet_learn_fdb = rt_data->localnet_learn_fdb; b_ctx_out->localnet_learn_fdb_changed = false; + b_ctx_out->tunnel_ifaces_changed = false; } /* Calculates the set of 'active_tunnels', the set of HA chassis groups @@ -1649,6 +1653,61 @@ runtime_data_calculate_active_ha(struct engine_node *node, active_ha_groups); } +static bool +ha_groups_equal(const struct uuidset *a, const struct uuidset *b) +{ + if (uuidset_count(a) != uuidset_count(b)) { + return false; + } + + const struct uuidset_node *node; + UUIDSET_FOR_EACH (node, a) { + if (!uuidset_contains(b, &node->uuid)) { + return false; + } + } + return true; +} + +/* Recalculates the active tunnels. Returns false if the local chassis + * doesn't belong to any HA chassis group or if this changes the set of HA + * chassis groups for which the local chassis is the active one, in which + * case the runtime data must be recomputed. Otherwise updates + * 'rt_data->active_tunnels' and returns true. */ +static bool +runtime_data_handle_active_tunnels(struct engine_node *node, + struct ed_type_runtime_data *rt_data, + const struct binding_ctx_in *b_ctx_in) +{ + if (uuidset_is_empty(&rt_data->local_ha_groups)) { + /* The local chassis is not an HA chassis, so BFD is enabled on its + * tunnels only while it is a reference chassis, i.e., while it has + * datapaths of distributed gateway ports local. Recompute in that + * case, as removing datapaths that are no longer local is not + * supported incrementally. */ + return false; + } + + struct sset active_tunnels = SSET_INITIALIZER(&active_tunnels); + struct uuidset local_ha_groups = UUIDSET_INITIALIZER(&local_ha_groups); + struct uuidset active_ha_groups = + UUIDSET_INITIALIZER(&active_ha_groups); + + runtime_data_calculate_active_ha(node, b_ctx_in, &active_tunnels, + &local_ha_groups, &active_ha_groups); + + bool handled = ha_groups_equal(&active_ha_groups, + &rt_data->active_ha_groups); + if (handled) { + sset_swap(&active_tunnels, &rt_data->active_tunnels); + } + + sset_destroy(&active_tunnels); + uuidset_destroy(&local_ha_groups); + uuidset_destroy(&active_ha_groups); + return handled; +} + static enum engine_node_state en_runtime_data_run(struct engine_node *node, void *data) { @@ -1785,6 +1844,11 @@ runtime_data_ovs_interface_shadow_handler(struct engine_node *node, void *data) return EN_UNHANDLED; } + if (b_ctx_out.tunnel_ifaces_changed && + !runtime_data_handle_active_tunnels(node, rt_data, &b_ctx_in)) { + return EN_UNHANDLED; + } + if (b_ctx_out.local_lports_changed) { result = EN_HANDLED_UPDATED; rt_data->local_lports_changed = b_ctx_out.local_lports_changed; diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index 0ae4b4b54..d600ba2cd 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -4345,6 +4345,66 @@ check ovn-nbctl lr-nat-add lr0 snat 172.168.0.10 10.0.0.0/24 check ovn-nbctl --wait=hv sync ]) +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn-controller - HA gateway I-P for ref_chassis and BFD changes]) +AT_KEYWORDS([ovn ha-ref-chassis]) + +HA_GW_REF_CHASSIS_SETUP([20], [10]) + +gw1=$(fetch_column Chassis _uuid name=gw1) + +# Bind vm2 on hv2 so that hv2 is already a reference chassis. +as hv2 check ovs-vsctl add-port br-int vif2 -- \ + set interface vif2 external_ids:iface-id=vm2 +wait_for_ports_up vm2 +wait_row_count Port_Binding 1 logical_port=cr-lr0-pub chassis=$gw1 +OVS_WAIT_UNTIL([test "$(bfd_state gw1 gw2)" = up]) +OVS_WAIT_UNTIL([test "$(bfd_state gw1 hv2)" = up]) +OVS_WAIT_UNTIL([test "$(bfd_state gw2 hv2)" = up]) +check ovn-nbctl --wait=hv sync + +# Bind the first VM on hv1. hv1 becomes a reference chassis, so both +# gateways enable BFD towards it. This must not trigger a recompute of +# runtime data on the gateways as the active gateway doesn't change. +for gw in gw1 gw2; do + check as $gw ovn-appctl -t ovn-controller inc-engine/clear-stats +done +as hv1 check ovs-vsctl add-port br-int vif1 -- \ + set interface vif1 external_ids:iface-id=vm1 +wait_for_ports_up vm1 +OVS_WAIT_UNTIL([test "$(bfd_state gw1 hv1)" = up]) +OVS_WAIT_UNTIL([test "$(bfd_state gw2 hv1)" = up]) +check ovn-nbctl --wait=hv sync + +check_row_count Port_Binding 1 logical_port=cr-lr0-pub chassis=$gw1 +for gw in gw1 gw2; do + check_controller_engine_stats $gw runtime_data norecompute compute +done +CHECK_AFTER_RECOMPUTE([gw1], [gw1]) +CHECK_AFTER_RECOMPUTE([gw2], [gw2]) + +# Remove the last VM from hv1. Both gateways disable BFD towards it, +# again without recomputing runtime data. +for gw in gw1 gw2; do + check as $gw ovn-appctl -t ovn-controller inc-engine/clear-stats +done +as hv1 check ovs-vsctl del-port br-int vif1 +wait_row_count Port_Binding 1 logical_port=vm1 chassis=[[]] +OVS_WAIT_UNTIL([test "$(bfd_state gw1 hv1)" = disabled]) +OVS_WAIT_UNTIL([test "$(bfd_state gw2 hv1)" = disabled]) +check ovn-nbctl --wait=hv sync + +check_row_count Port_Binding 1 logical_port=cr-lr0-pub chassis=$gw1 +for gw in gw1 gw2; do + check_controller_engine_stats $gw runtime_data norecompute compute +done +CHECK_AFTER_RECOMPUTE([gw1], [gw1]) +CHECK_AFTER_RECOMPUTE([gw2], [gw2]) + +OVN_CLEANUP([gw1],[gw2],[hv1],[hv2]) +AT_CLEANUP +]) + OVN_FOR_EACH_NORTHD([ AT_SETUP([ovn-controller - HA gateway failover on ref_chassis changes]) AT_KEYWORDS([ovn ha-ref-chassis]) -- 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
