| Message ID | 20251219094839.78689-6-arukomoinikova@k2.cloud |
|---|---|
| State | Changes Requested |
| Delegated to: | Dumitru Ceara |
| Headers | show |
| Series | [ovs-dev,1/6] ovn-nb, ovn-nbctl: Add LSP Health Check schema support. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
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 <arukomoinikova@k2.cloud> > --- 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
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); + } + } + } + } + + sbrec_service_monitor_index_destroy_row(key); +} + /* Handle a fairly small set of changes in the southbound database. */ void ovnsb_db_run(struct ovsdb_idl_txn *ovnsb_txn, @@ -21007,6 +21081,11 @@ ovnsb_db_run(struct ovsdb_idl_txn *ovnsb_txn, &northd_data->lr_ports, &ha_ref_chassis_map); + handle_service_monitor_changes( + sync_from_sb_data->sbrec_service_monitor_by_service_type, + &northd_data->ls_ports); + + update_sb_ha_group_ref_chassis(sync_from_sb_data->sb_ha_ch_grp_table, &ha_ref_chassis_map); diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema index 5373e6f59..557e2d37c 100644 --- a/ovn-nb.ovsschema +++ b/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", - "version": "7.16.0", - "cksum": "3363473672 44948", + "version": "7.17.0", + "cksum": "175732137 45161", "tables": { "NB_Global": { "columns": { @@ -264,6 +264,10 @@ "addresses": {"type": {"key": "string", "min": 0, "max": "unlimited"}}, + "status": { + "type": {"key": {"type": "string", + "enum": ["set", ["online", "offline", "error"]]}, + "min": 0, "max": 1}}, "options": { "type": {"key": "string", "value": "string", diff --git a/ovn-nb.xml b/ovn-nb.xml index 3cc46e60c..61dea37d8 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -2205,6 +2205,11 @@ health check will use all addresses configured on logical switch port. </column> + <column name="status"> + Health status synchronized from the <code>status</code> field of + corresponding service monitor in the southbound database. + </column> + <column name="options" key="interval" type='{"type": "integer"}'> The interval, in seconds, between service monitor checks. </column> diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ovn-inc-proc-graph-dump.at index 27f111eb1..17a3bcb0f 100644 --- a/tests/ovn-inc-proc-graph-dump.at +++ b/tests/ovn-inc-proc-graph-dump.at @@ -97,6 +97,7 @@ digraph "Incremental-Processing-Engine" { northd -> sync_from_sb [[label="sync_from_sb_northd_handler"]]; SB_port_binding -> sync_from_sb [[label=""]]; SB_ha_chassis_group -> sync_from_sb [[label=""]]; + SB_service_monitor -> sync_from_sb [[label=""]]; lr_nat [[style=filled, shape=box, fillcolor=white, label="lr_nat"]]; northd -> lr_nat [[label="lr_nat_northd_handler"]]; lr_stateful [[style=filled, shape=box, fillcolor=white, label="lr_stateful"]]; diff --git a/tests/system-ovn.at b/tests/system-ovn.at index 15b7491bb..521a26e96 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -19092,25 +19092,39 @@ OVS_WAIT_UNTIL([ # Wait until the services are set to online. wait_row_count Service_Monitor 1 status=online -# Create one more health check on logical switch port -NETNS_DAEMONIZE([lport], [nc -l -k 192.168.0.10 4041], [lport_tcp.pid]) +# Check status +check_column "online" nb:Logical_Switch_Port_Health_Check status protocol=icmp +# Check tcp monitor check ovn-nbctl lsp-hc-add lport tcp 192.168.0.255 4041 lport1_uuid2=$(fetch_column nb:Logical_Switch_Port_Health_Check _uuid protocol=tcp) check_row_count sb:Service_Monitor 2 +ovn-sbctl list service_monitor + +check_column "offline" nb:Logical_Switch_Port_Health_Check status protocol=tcp + +# Create one more health check on logical switch port +NETNS_DAEMONIZE([lport], [nc -l -k 192.168.0.10 4041], [lport_tcp.pid]) + # Wait until the services are set to online. wait_row_count Service_Monitor 2 status=online -NETNS_DAEMONIZE([lport], [nc -ulp 4042], [lport_udp.pid]) +check_column "online" nb:Logical_Switch_Port_Health_Check status protocol=tcp check ovn-nbctl lsp-hc-add lport udp 192.168.0.255 4042 lport1_uuid3=$(fetch_column nb:Logical_Switch_Port_Health_Check _uuid protocol=udp) +check_column "offline" nb:Logical_Switch_Port_Health_Check status protocol=udp + +NETNS_DAEMONIZE([lport], [nc -ulp 4042], [lport_udp.pid]) + check_row_count sb:Service_Monitor 3 # Wait until the services are set to online. wait_row_count Service_Monitor 3 status=online +check_column "online" nb:Logical_Switch_Port_Health_Check status protocol=udp + AT_CLEANUP ])
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 <arukomoinikova@k2.cloud> --- 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(-)