On 12/19/25 10:48 AM, Alexandra Rukomoinikova wrote:
> This commit implements synchronization of health check for logical
> switch port status from SB Service_Monitor table to NB
> Logical_Switch_Port_Health_Check table.
>
> Signed-off-by: Alexandra Rukomoinikova <[email protected]>
> ---
Hi Alexandra,
Thanks for the patch!
> northd/en-sync-from-sb.c | 4 ++
> northd/en-sync-from-sb.h | 3 ++
> northd/inc-proc-northd.c | 8 ++++
> northd/northd.c | 79 ++++++++++++++++++++++++++++++++
> ovn-nb.ovsschema | 8 +++-
> ovn-nb.xml | 5 ++
> tests/ovn-inc-proc-graph-dump.at | 1 +
> tests/system-ovn.at | 20 ++++++--
> 8 files changed, 123 insertions(+), 5 deletions(-)
>
> diff --git a/northd/en-sync-from-sb.c b/northd/en-sync-from-sb.c
> index 6d4ff3e39..11f3886dc 100644
> --- a/northd/en-sync-from-sb.c
> +++ b/northd/en-sync-from-sb.c
> @@ -52,6 +52,10 @@ en_sync_from_sb_get_input_data(struct engine_node *node,
> EN_OVSDB_GET(engine_get_input("SB_port_binding", node));
> data->sb_ha_ch_grp_table =
> EN_OVSDB_GET(engine_get_input("SB_ha_chassis_group", node));
> + data->sbrec_service_monitor_by_service_type =
> + engine_ovsdb_node_get_index(
> + engine_get_input("SB_service_monitor", node),
> + "sbrec_service_monitor_by_service_type");
> }
>
> enum engine_node_state
> diff --git a/northd/en-sync-from-sb.h b/northd/en-sync-from-sb.h
> index bea248c45..177d75de0 100644
> --- a/northd/en-sync-from-sb.h
> +++ b/northd/en-sync-from-sb.h
> @@ -7,6 +7,9 @@ struct en_sync_from_sb_data {
> /* Southbound table references */
> const struct sbrec_port_binding_table *sb_pb_table;
> const struct sbrec_ha_chassis_group_table *sb_ha_ch_grp_table;
> +
> + /* Indexes */
> + struct ovsdb_idl_index *sbrec_service_monitor_by_service_type;
> };
>
> void *en_sync_from_sb_init(struct engine_node *, struct engine_arg *);
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index bb740a9ba..40a1d727c 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -471,6 +471,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> sync_from_sb_northd_handler);
> engine_add_input(&en_sync_from_sb, &en_sb_port_binding, NULL);
> engine_add_input(&en_sync_from_sb, &en_sb_ha_chassis_group, NULL);
> + engine_add_input(&en_sync_from_sb, &en_sb_service_monitor, NULL);
>
> engine_add_input(&en_northd_output, &en_acl_id, NULL);
> engine_add_input(&en_northd_output, &en_sync_from_sb, NULL);
> @@ -586,6 +587,13 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> "sbrec_service_monitor_by_learned_type",
> sbrec_service_monitor_by_learned_type);
>
> + struct ovsdb_idl_index *sbrec_service_monitor_by_service_type
> + = ovsdb_idl_index_create1(sb->idl,
> + &sbrec_service_monitor_col_type);
> + engine_ovsdb_node_add_index(&en_sb_service_monitor,
> + "sbrec_service_monitor_by_service_type",
> + sbrec_service_monitor_by_service_type);
> +
> struct ed_type_global_config *global_config =
> engine_get_internal_data(&en_global_config);
> unixctl_command_register("debug/chassis-features-list", "", 0, 0,
> diff --git a/northd/northd.c b/northd/northd.c
> index f6411dd3b..a80e36219 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -20990,6 +20990,80 @@ handle_port_binding_changes(
> hmapx_destroy(&lr_groups);
> }
>
> +static bool
> +lsp_monitor_info_matches(const struct sbrec_service_monitor *sbrec_mon,
> + const char *protocol, uint16_t service_port,
> + char **addresses, int n_addresses)
> +{
> + if (strcmp(sbrec_mon->protocol, protocol) != 0) {
> + return false;
> + }
> +
> + if (!strcmp(protocol, "tcp") || !strcmp(protocol, "udp")) {
> + if (sbrec_mon->port != service_port) {
> + return false;
> + }
> + }
> +
> + if (!addresses) {
> + return true;
> + }
> +
> + for (size_t i = 0; i < n_addresses; i++) {
> + if (!strcmp(sbrec_mon->ip, addresses[i])) {
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> +static void
> +handle_service_monitor_changes(
> + struct ovsdb_idl_index *sbrec_service_monitor_by_service_type,
> + const struct hmap *ls_ports)
> +{
> + const struct sbrec_service_monitor *sbrec_mon;
> + struct sbrec_service_monitor *key =
> + sbrec_service_monitor_index_init_row(
> + sbrec_service_monitor_by_service_type);
> +
> + sbrec_service_monitor_set_type(key, "logical-switch-port");
> +
> + SBREC_SERVICE_MONITOR_FOR_EACH_EQUAL (sbrec_mon, key,
> + sbrec_service_monitor_by_service_type) {
> +
> + struct ovn_port *op = ovn_port_find(ls_ports,
> sbrec_mon->logical_port);
> + if (!op) {
> + continue;
> + }
> +
> + ovs_assert(op->nbsp && op->nbsp->n_health_checks);
> +
> + for (size_t i = 0; i < op->nbsp->n_health_checks; i++) {
> + const struct nbrec_logical_switch_port_health_check *lsp_hc =
> + op->nbsp->health_checks[i];
> +
> + if (!lsp_monitor_info_matches(sbrec_mon, lsp_hc->protocol,
> + lsp_hc->port, lsp_hc->addresses,
> + lsp_hc->n_addresses)) {
> + continue;
> + }
> +
> + const char *desired_status = sbrec_mon->status;
> + if (desired_status) {
> + if (!lsp_hc->status ||
> + strcmp(lsp_hc->status, desired_status)) {
> + nbrec_logical_switch_port_health_check_set_status(
> + lsp_hc, sbrec_mon->status);
As far as I understand a LSP health check can be configured to use
multiple IP addresses.
If some of those are online and the rest are online, based on the order
in which we iterate through them we would he setting the LSPHC.status to
either "online" or "offline".
That's because we overwrite the status of the previous address with the
status of the current one.
Should we say that if at least one address in LSPHC.addresses is offline
then the LSPHC.status should be set to offline? We can also say "if at
least one is online then status should be online". But the current code
produces random results.
> + }
> + }
> + }
> + }
> +
> + sbrec_service_monitor_index_destroy_row(key);
> +}
> +
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
