| Message ID | 20251219094839.78689-1-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 | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
Bleep bloop. Greetings Alexandra Rukomoinikova, I am a robot and I have tried out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line is 89 characters long (recommended limit is 79)
#85 FILE: ovn-nb.ovsschema:183:
"refTable": "Logical_Switch_Port_Health_Check",
WARNING: Line lacks whitespace around operator
#477 FILE: utilities/ovn-nbctl.c:560:
lsp-hc-add PORT PROTOCOL SOURCE_IP DST_PORT [ADDRESS]...\n\
WARNING: Line lacks whitespace around operator
#479 FILE: utilities/ovn-nbctl.c:562:
lsp-hc-del PORT HC_UUID delete health check monitoring for PORT\n\
WARNING: Line lacks whitespace around operator
#480 FILE: utilities/ovn-nbctl.c:563:
lsp-hc-list PORT print health check for PORT\n\
Lines checked: 996, Warnings: 4, Errors: 0
Please check this out. If you feel there has been an error, please email aconole@redhat.com
Thanks,
0-day Robot
On 12/19/25 10:48 AM, Alexandra Rukomoinikova wrote: > 1) Added tables for further implementation logic of service monitors for > logical switch ports: > > New table: > - Logical_Switch_Port_Health_Check: Health check configuration > for logical switch port. > > Modified tables: > - Logical_Switch_Port: Add 'health_checks' column referencing > health checks configuration. > > 2) Added commands to create, delete, and describe health checks for logical switch ports. > > Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> > --- Hi Alexandra, I started reviewing this series but before doing that I want to double check the use case. Is your plan to use these to check the availability of Logical_Switch_Ports as backends of NB.Load_Balancer? If so, how do you plan to add that mapping? I guess it would be a new feature in 26.09. Or is your goal to just expose the LSPHC.status to the CMS and then let the CMS update the list of backends of the LB? In any case we probably should: - update the documentation expanding on the use case more - update the commit log of this patch (and maybe the others too) - add a TODO.rst item for any follow up work we might be doing in 26.09. I have some more comments on the code changes themselves but I'll send those replies separately when I'm done going through the code. Thanks, Dumitru
Hi Alexandra, I have several notes inline below. On Fri, Dec 19, 2025 at 4:49 AM Alexandra Rukomoinikova <arukomoinikova@k2.cloud> wrote: > > 1) Added tables for further implementation logic of service monitors for > logical switch ports: > > New table: > - Logical_Switch_Port_Health_Check: Health check configuration > for logical switch port. > > Modified tables: > - Logical_Switch_Port: Add 'health_checks' column referencing > health checks configuration. > > 2) Added commands to create, delete, and describe health checks for logical switch ports. > > Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> > --- > lib/ovn-util.c | 17 ++ > lib/ovn-util.h | 2 + > ovn-nb.ovsschema | 28 ++- > ovn-nb.xml | 54 ++++ > tests/ovn-nbctl.at | 186 ++++++++++++++ > utilities/ovn-nbctl.8.xml | 54 ++++ > utilities/ovn-nbctl.c | 512 ++++++++++++++++++++++++++++++++++++++ > 7 files changed, 851 insertions(+), 2 deletions(-) > > diff --git a/lib/ovn-util.c b/lib/ovn-util.c > index cec029e42..16f7eb2b9 100644 > --- a/lib/ovn-util.c > +++ b/lib/ovn-util.c > @@ -1704,3 +1704,20 @@ is_partial_uuid_match(const struct uuid *uuid, const char *match) > s2 = strip_leading_zero(s2); > return !strncmp(s1, s2, strlen(s2)); > } > + > +char * > +skip_mac_address_from_lsp_address(const char *address_with_mac) > +{ > + if (!address_with_mac) { > + return NULL; > + } > + const char *ptr = address_with_mac; > + while (*ptr == ' ') { > + ptr++; > + } > + ptr += (char) 17; > + while (*ptr == ' ') { > + ptr++; > + } > + return (char *) ptr; > +} > diff --git a/lib/ovn-util.h b/lib/ovn-util.h > index daff01995..7685118e6 100644 > --- a/lib/ovn-util.h > +++ b/lib/ovn-util.h > @@ -775,4 +775,6 @@ NEIGH_REDISTRIBUTE_MODES > enum neigh_redistribute_mode > parse_neigh_dynamic_redistribute(const struct smap *options); > > +char *skip_mac_address_from_lsp_address(const char *address_with_mac); > + > #endif /* OVN_UTIL_H */ > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema > index 8c2c1d861..5373e6f59 100644 > --- a/ovn-nb.ovsschema > +++ b/ovn-nb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Northbound", > - "version": "7.15.0", > - "cksum": "4060410729 43708", > + "version": "7.16.0", > + "cksum": "3363473672 44948", > "tables": { > "NB_Global": { > "columns": { > @@ -179,6 +179,11 @@ > "refType": "strong"}, > "min": 0, > "max": 1}}, > + "health_checks": {"type": {"key": {"type": "uuid", > + "refTable": "Logical_Switch_Port_Health_Check", > + "refType": "weak"}, > + "min": 0, > + "max": "unlimited"}}, > "external_ids": { > "type": {"key": "string", "value": "string", > "min": 0, "max": "unlimited"}}}, > @@ -246,6 +251,25 @@ > "min": 0, "max": "unlimited"}}}, > "indexes": [["name"], ["id"]], > "isRoot": true}, > + "Logical_Switch_Port_Health_Check": { > + "columns": { > + "protocol": { > + "type": {"key": {"type": "string", > + "enum": ["set", ["tcp", "udp", "icmp"]]}, > + "min": 0, "max": 1}}, > + "src_ip": {"type": "string"}, > + "port": {"type": {"key": {"type": "integer", > + "minInteger": 0, > + "maxInteger": 65535}}}, > + "addresses": {"type": {"key": "string", > + "min": 0, > + "max": "unlimited"}}, > + "options": { > + "type": {"key": "string", > + "value": "string", > + "min": 0, > + "max": "unlimited"}}}, > + "isRoot": true}, IMO, it makes sense for this to have isRoot: false. I can't see a reason for a logical switch port health check to exist without being attached to a logical switch port. > "Forwarding_Group": { > "columns": { > "name": {"type": "string"}, > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 730293e97..3cc46e60c 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -2115,6 +2115,11 @@ > Please see the <ref table="Mirror"/> table. > </column> > > + <column name="health_checks"> > + Health checks configuration for this logical switch port. > + Please see the <ref table="Logical_Switch_Port_Health_Check"/> table. > + </column> > + > <column name="ha_chassis_group"> > References a row in the OVN Northbound database's > <ref table="HA_Chassis_Group" db="OVN_Northbound"/> table. > @@ -2171,6 +2176,55 @@ > </group> > </table> > > + <table name="Logical_Switch_Port_Health_Check" > + title="logical switch port health check"> > + <p> > + Each row represents health check configuration for logical switch port. > + Health checks are used to monitor reachability and health of backend > + endpoints associated with logical switch port. Health check can be > + configured to use different protocols (TCP, UDP, or ICMP) to verify > + availability of target IP addresses. > + </p> > + > + <column name="protocol"> > + Valid protocols are <code>tcp</code>, <code>udp</code>, or > + <code>icmp</code>. For <code>tcp</code> and <code>udp</code> > + protocols - destination port must be specified. > + </column> > + > + <column name="src_ip"> > + Source IP address used when sending health check probes. > + </column> > + > + <column name="port"> > + Destination port number for TCP and UDP health checks. > + </column> > + > + <column name="addresses"> > + A set of IP addresses to monitor. If no specific addresses are provided, > + health check will use all addresses configured on logical switch port. > + </column> > + > + <column name="options" key="interval" type='{"type": "integer"}'> > + The interval, in seconds, between service monitor checks. > + </column> > + > + <column name="options" key="timeout" type='{"type": "integer"}'> > + The time, in seconds, after which the service monitor check times > + out. > + </column> > + > + <column name="options" key="success_count" type='{"type": "integer"}'> > + The number of successful checks after which the service is > + considered <code>online</code>. > + </column> > + > + <column name="options" key="failure_count" type='{"type": "integer"}'> > + The number of failure checks after which the service is considered > + <code>offline</code>. > + </column> > + </table> > + > <table name="Forwarding_Group" title="forwarding group"> > <p> > Each row represents one forwarding group. > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at > index dccf30758..62b89945b 100644 > --- a/tests/ovn-nbctl.at > +++ b/tests/ovn-nbctl.at > @@ -3499,3 +3499,189 @@ AT_CHECK([ovn-nbctl --may-exist lsp-add-localnet-port ls ln_port net1], [1], [], > check ovn-nbctl lsp-set-options ln_port network_name=net1 > check ovn-nbctl --may-exist lsp-add-localnet-port ls ln_port net1 > ]) > + > +dnl --------------------------------------------------------------------- > + > +AT_SETUP([ovn-nbctl - Logical Switch Port Health Check]) > +OVN_NBCTL_TEST_START daemon > + > +# Create logical switch port > +AT_CHECK([ovn-nbctl ls-add ls0]) > +AT_CHECK([ovn-nbctl lsp-add ls0 lport0]) > +AT_CHECK([ovn-nbctl lsp-set-addresses lport0 "00:11:22:33:44:55 192.168.1.10" "00:11:22:33:44:56 192.168.1.11"]) > + > +# Create icmp health check with the specified addresses > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 192.168.0.255 192.168.1.10 192.168.1.11]) > +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl > +Logical Switch Port lport0: > + Protocol : icmp > + Source IP : 192.168.0.255 > + Addresses : 192.168.1.10, 192.168.1.11 > +]) > + > +# Check hc attaching to logical switch port > +hc_icmp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="192.168.0.255") > +check_column "$hc_icmp_uuid" nb:logical_switch_port health_checks > + > +# Create tcp health check with the specified addresses > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.2 80 192.168.1.10]) > +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl > +Logical Switch Port lport0: > + Protocol : tcp > + Source IP : 10.0.0.2 > + Port : 80 > + Addresses : 192.168.1.10 > + > + Protocol : icmp > + Source IP : 192.168.0.255 > + Addresses : 192.168.1.10, 192.168.1.11 > +]) > +hc_tcp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="10.0.0.2") > + > +# Create udp health check with the specified addresses > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 udp 10.0.0.3 53 192.168.1.11]) > +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl > +Logical Switch Port lport0: > + Protocol : tcp > + Source IP : 10.0.0.2 > + Port : 80 > + Addresses : 192.168.1.10 > + > + Protocol : udp > + Source IP : 10.0.0.3 > + Port : 53 > + Addresses : 192.168.1.11 > + > + Protocol : icmp > + Source IP : 192.168.0.255 > + Addresses : 192.168.1.10, 192.168.1.11 > +]) > +hc_udp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="10.0.0.3") > + > +check_column "$hc_icmp_uuid $hc_tcp_uuid $hc_udp_uuid" nb:logical_switch_port health_checks > + > +AT_CHECK([ovn-nbctl lsp-hc-del lport0 $hc_icmp_uuid]) > +# Check hcs detaching to logical switch port > +check_column "$hc_tcp_uuid $hc_udp_uuid" nb:logical_switch_port health_checks > + > +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl > +Logical Switch Port lport0: > + Protocol : tcp > + Source IP : 10.0.0.2 > + Port : 80 > + Addresses : 192.168.1.10 > + > + Protocol : udp > + Source IP : 10.0.0.3 > + Port : 53 > + Addresses : 192.168.1.11 > +]) > + > +AT_CHECK([ovn-nbctl lsp-hc-del lport0]) > +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl]) > + > +# Create health check without specified addresses > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.4]) > +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl > +Logical Switch Port lport0: > + Protocol : icmp > + Source IP : 10.0.0.4 > +]) > + > +# Check invalid protocol > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 stcp 10.0.0.1], [1], [], [stderr]) > +AT_CHECK([grep "Type must be icmp, tcp or udp" stderr], [0], [ignore]) > + > +# Check invalid source IP > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp invalid_ip], [1], [], [stderr]) > +AT_CHECK([grep "Not a valid IPv4 or IPv6 address" stderr], [0], [ignore]) > + > +# Check TCP/UDP without destination port > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1], [1], [], [stderr]) > +AT_CHECK([grep "Destination port required for tcp health check" stderr], [0], [ignore]) > + > +# Check invalid destination port > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 70000], [1], [], [stderr]) > +AT_CHECK([grep "port must in range 0...65535" stderr], [0], [ignore]) > + > +# Check IP address not configured on port > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.1 192.168.99.99], [1], [], [stderr]) > +AT_CHECK([grep "Address 192.168.99.99 not configured on port" stderr], [0], [ignore]) > + > +# Check Duplicate health check configuration > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.4], [1], [], [stderr]) > +AT_CHECK([grep "Health check already exists" stderr], [0], [ignore]) > + > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.1 192.168.1.10]) > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.1 192.168.1.10], [1], [], [stderr]) > +AT_CHECK([grep "Health check already exists" stderr], [0], [ignore]) > + > +# Check same protocol with different ports - it's ok > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 90 192.168.1.10]) > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 443 192.168.1.10], [0], [], [ignore]) > + > +# Check different protocol with same ports - it's ok > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 80 192.168.1.10]) > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 udp 10.0.0.1 80 192.168.1.10], [0], [], [ignore]) > + > +# Check different source ip with same ports and addresses - it's ok > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 92 192.168.1.10]) > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.2 92 192.168.1.10], [0], [], [ignore]) > + > +# Check multipal same addresses > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 93 192.168.1.10 192.168.1.11]) > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 93 192.168.1.10 192.168.1.11], [1], [], [stderr]) > +AT_CHECK([grep "Health check already exists" stderr], [0], [ignore]) > + > +# Check supported logical switch port type for monitoring > +AT_CHECK([ovn-nbctl lsp-add ls0 lport1]) > +AT_CHECK([ovn-nbctl lsp-set-type lport1 router]) > +AT_CHECK([ovn-nbctl lsp-hc-add lport1 icmp 10.0.0.1], [1], [], [stderr]) > +AT_CHECK([grep "Health check monitoring supported only for port with vif type" stderr], [0], [ignore]) > + > +AT_CHECK([ovn-nbctl lsp-hc-del lport0]) > + > +# Test IPv6 addresses support > +AT_CHECK([ovn-nbctl lsp-add ls0 lport3]) > +AT_CHECK([ovn-nbctl lsp-set-addresses lport3 "00:11:22:33:44:57 2001:db8::1"]) > +AT_CHECK([ovn-nbctl lsp-hc-add lport3 icmp 2001:db8::2 2001:db8::1]) > +AT_CHECK([ovn-nbctl lsp-hc-list lport3], [0], [dnl > +Logical Switch Port lport3: > + Protocol : icmp > + Source IP : 2001:db8::2 > + Addresses : 2001:db8::1 > +]) > + > +# Check options printing > +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.4 192.168.1.10 192.168.1.11]) > +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl > +Logical Switch Port lport0: > + Protocol : icmp > + Source IP : 10.0.0.4 > + Addresses : 192.168.1.10, 192.168.1.11 > +]) > +hc_icmp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="10.0.0.4") > + > +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:interval=3]) > +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:timeout=30]) > +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:success_count=1]) > +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:failure_count=2]) > + > +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl > +Logical Switch Port lport0: > + Protocol : icmp > + Source IP : 10.0.0.4 > + Addresses : 192.168.1.10, 192.168.1.11 > + Interval : 3 > + Timeout : 30 > + Success count : 1 > + Failure count : 2 > +]) > + > +AT_CHECK([ovn-nbctl lsp-hc-del lport0]) > +AT_CHECK([ovn-nbctl lsp-hc-del lport3]) > +check_row_count nb:Logical_Switch_Port_Health_Check 0 > + > +OVN_NBCTL_TEST_STOP "/terminating with signal 15/d" > +AT_CLEANUP > + > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml > index 7df902944..efc781d77 100644 > --- a/utilities/ovn-nbctl.8.xml > +++ b/utilities/ovn-nbctl.8.xml > @@ -1822,6 +1822,60 @@ > </dd> > </dl> > > + <h2>Health Check commands</h2> > + <dl> > + <dt><code>lsp-hc-add</code> <var>PORT</var> <var>PROTOCOL</var> > + <var>SOURCE_IP</var> [<var>DST_PORT</var>] [<var>ADDRESS</var>]...</dt> > + <dd> > + <p> > + Creates a new health check configuration for the logical switch port > + <code>PORT</code> with the below mandatory arguments. > + </p> > + > + <p> > + <var>PROTOCOL</var> specifies the health check protocol - > + <code>tcp</code>, <code>udp</code>, or <code>icmp</code>. > + </p> > + > + <p> > + <var>SOURCE_IP</var> specifies the source IP address used when > + sending health check probes. > + </p> > + > + <p> > + <var>DST_PORT</var> specifies the destination port number for > + <code>tcp</code> and <code>udp</code> health checks. This parameter > + is ignored for <code>icmp</code> protocol. > + </p> > + > + <p> > + <var>ADDRESS</var> specifies one or more IP addresses to monitor. > + If no addresses are provided, the health check will monitor all IP > + addresses configured on the logical switch port. > + </p> > + > + <p> > + Additional health check options such as interval, timeout, > + success count, and failure count can be configured separately. > + </p> > + </dd> > + > + <dt><code>lsp-hc-del</code> <var>PORT</var> [<var>HC_UUID</var>]</dt> > + <dd> > + <p> > + Deletes health check configuration for the logical switch port > + <code>PORT</code>. > + </p> > + </dd> > + > + <dt><code>lsp-hc-list</code> <var>PORT</var></dt> > + <dd> > + Lists all health check configurations for the logical switch port > + <code>PORT</code>, including protocol, source IP, destination port, > + monitored addresses, and current status. > + </dd> > + </dl> > + > <h2>Synchronization Commands</h2> > > <dl> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index cdf6b578a..b8802ab30 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -66,6 +66,18 @@ string_ptr(char *ptr) > return (ptr) ? ptr : s; > } > > +static char *OVS_WARN_UNUSED_RESULT > +parse_l4_port_range(const char *arg, int64_t *port_p) > +{ > + int64_t port; > + if (!ovs_scan(arg, "%"SCNd64, &port) > + || port < 0 || port > UINT16_MAX) { > + return xasprintf("%s: port must in range 0...65535", arg); > + } > + *port_p = port; > + return NULL; > +} > + > static void > nbctl_add_base_prerequisites(struct ovsdb_idl *idl, > enum nbctl_wait_type wait_type) > @@ -544,6 +556,12 @@ MAC_Binding commands:\n\ > Delete Static_MAC_Binding entry\n\ > static-mac-binding-list List all Static_MAC_Binding entries\n\ > \n\ > +Logical Switch Port Health Check:\n\ > + lsp-hc-add PORT PROTOCOL SOURCE_IP DST_PORT [ADDRESS]...\n\ > + add health check monitoring for PORT\n\ > + lsp-hc-del PORT HC_UUID delete health check monitoring for PORT\n\ > + lsp-hc-list PORT print health check for PORT\n\ > +\n\ > %s\ > %s\ > \n\ > @@ -8677,6 +8695,490 @@ nbctl_lsp_add_misc_port(struct ctl_context *ctx) > shash_add(&nbctx->lsp_to_ls_map, lsp_name, ls); > } > > +/* Logical Switch Port Health Check Functions. */ > +enum health_check_protocol { > + LSP_ICMP_HEALTH_CHECK, > + LSP_TCP_HEALTH_CHECK, > + LSP_UDP_HEALTH_CHECK, > +}; > + > +static bool > +parse_health_check_protocol(struct ctl_context *ctx, const char *type, > + enum health_check_protocol *protocol) > +{ > + if (!strcmp(type, "icmp")) { > + *protocol = LSP_ICMP_HEALTH_CHECK; > + return true; > + } else if (!strcmp(type, "tcp")) { > + *protocol = LSP_TCP_HEALTH_CHECK; > + return true; > + } else if (!strcmp(type, "udp")) { > + *protocol = LSP_UDP_HEALTH_CHECK; > + return true; > + } else { > + ctl_error(ctx, "%s: Type must be icmp, tcp or udp", type); > + return false; > + } > +} > + > +static void > +lsp_health_check_get_duplicate( > + int proto, const char *src_ip, > + int64_t port, struct sset *addresses, > + const struct nbrec_logical_switch_port *lsp, > + const struct nbrec_logical_switch_port_health_check **lsp_hc) > +{ Instead of returning void, this could return a const struct nbrec_logical_switch_port_health_check pointer. A NULL return means no duplicate was found. A non-NULL return means we found a duplicate. > + *lsp_hc = NULL; > + > + for (size_t i = 0; i < lsp->n_health_checks; i++) { > + const struct nbrec_logical_switch_port_health_check *lsp_hc_p = > + lsp->health_checks[i]; > + > + if (src_ip && strcmp(lsp_hc_p->src_ip, src_ip)) { > + continue; > + } > + > + const char *target_proto = > + (proto == LSP_ICMP_HEALTH_CHECK) ? "icmp" : > + (proto == LSP_TCP_HEALTH_CHECK) ? "tcp" : "udp"; > + if (strcmp(lsp_hc_p->protocol, target_proto)) { > + continue; > + } > + > + if (proto != LSP_ICMP_HEALTH_CHECK && lsp_hc_p->port != port) { > + continue; > + } > + > + if (!addresses || sset_is_empty(addresses)) { > + *lsp_hc = lsp_hc_p; > + return; > + } This seems incorrect to me. Imagine that we have a logical switch port with IP addresses A, B, and C. We start by adding a health check that specifies target addresses A and B. Then we add a new health check with no target addresses. With this code, we would return the existing health check that only monitors addresses A and B. However, since the new health check has no target addresses, the intent likely was for the new health check to monitor addresses A, B, and C. This means that address C will not be monitored. > + > + bool addresses_found = true; > + for (size_t j = 0; j < lsp_hc_p->n_addresses; j++) { > + if (!sset_contains(addresses, lsp_hc_p->addresses[j])) { > + addresses_found = false; > + break; > + } > + } > + > + if (addresses_found) { > + *lsp_hc = lsp_hc_p; > + return; > + } > + } > +} > + > +static char ** > +_get_lsp_ip_addresses(char **lsp_addresses, > + int lsp_n_addresses, > + int *lsp_n_ip_addresses) > +{ This function is unnecessary. You can just use extract_lsp_addresses() in ovn-util to get the IP addresses from a logical switch port. It's also safer to use, since skip_mac_address_from_lsp_address() has the possibility to return a pointer past the end of the string if the LSP MAC is malformed. > + char **lsp_ip_addresses_p = NULL; > + *lsp_n_ip_addresses = 0; > + size_t n_capacity = 0; > + size_t count = 0; > + > + for (size_t i = 0; i < lsp_n_addresses; i++) { > + char *address_without_mac = > + skip_mac_address_from_lsp_address(lsp_addresses[i]); > + > + if (address_without_mac && address_without_mac[0]) { > + if (count == n_capacity) { > + lsp_ip_addresses_p = x2nrealloc(lsp_ip_addresses_p, > + &n_capacity, > + sizeof *lsp_ip_addresses_p); > + } > + lsp_ip_addresses_p[count] = xstrdup(address_without_mac); > + count++; > + } > + } > + > + *lsp_n_ip_addresses = count; > + return lsp_ip_addresses_p; > +} > + > +static bool > +_lsp_contains_ip_address(char **lsp_ip_addresses, > + size_t lsp_n_ip_addresses, > + char *ip_address) > +{ > + for (size_t i = 0; i < lsp_n_ip_addresses; i++) { > + if (!strcmp(lsp_ip_addresses[i], ip_address)) { > + return true; > + } > + } > + > + return false; > +} > + > +static char * OVS_WARN_UNUSED_RESULT > +find_logical_switch_port_health_check_by_uuid( > + struct ctl_context *ctx, > + const char *id, > + const struct nbrec_logical_switch_port_health_check **lsp_hc_p) > +{ > + const struct nbrec_logical_switch_port_health_check *lsp_hc; > + *lsp_hc_p = NULL; > + > + struct uuid lsp_hc_uuid; > + bool is_uuid = uuid_from_string(&lsp_hc_uuid, id); > + > + if (!is_uuid) { > + return xasprintf("%s: Invalid UUID format", id); > + } > + > + lsp_hc = nbrec_logical_switch_port_health_check_get_for_uuid( > + ctx->idl, &lsp_hc_uuid); > + > + if (!lsp_hc) { > + return xasprintf("%s: Logical Switch Port Health Check not found", id); > + } > + > + *lsp_hc_p = lsp_hc; > + > + return NULL; > +} > + > +static void > +nbctl_pre_lsp_health_check_add(struct ctl_context *ctx) > +{ > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_col_name); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_col_addresses); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_col_health_checks); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_health_check_col_port); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_health_check_col_protocol); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_health_check_col_src_ip); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_health_check_col_addresses); > +} > + > +static void > +nbctl_lsp_health_check_add(struct ctl_context *ctx) > +{ > + const struct nbrec_logical_switch_port *lsp = NULL; > + const struct nbrec_logical_switch_port_health_check *lsp_hc = NULL; > + > + const char *port = ctx->argv[1]; > + const char *type = ctx->argv[2]; > + const char *src_ip = ctx->argv[3]; > + enum health_check_protocol protocol; > + char **lsp_ip_addresses = NULL; > + int lsp_n_ip_addresses = 0; > + char *validated_src_ip = NULL; > + struct sset target_ips; > + sset_init(&target_ips); > + > + char *error; > + error = lsp_by_name_or_uuid(ctx, port, true, &lsp); > + if (error) { > + ctx->error = error; > + return; > + } > + > + if (lsp->type[0]) { > + ctl_error(ctx, "%s: Health check monitoring supported only for" > + " port with vif type", lsp->type); > + goto cleanup; > + } > + > + if (!parse_health_check_protocol(ctx, type, &protocol)) { > + goto cleanup; > + } > + > + validated_src_ip = normalize_addr_str(src_ip); > + if (!validated_src_ip) { > + ctl_error(ctx, "%s: Not a valid IPv4 or IPv6 address", > + ctx->argv[3]); > + goto cleanup; > + } > + > + int64_t destination_port = 0; > + if (protocol == LSP_TCP_HEALTH_CHECK || > + protocol == LSP_UDP_HEALTH_CHECK) { > + if (ctx->argc < 5) { > + ctl_error(ctx, "Destination port required for %s health check", > + type); > + goto cleanup; > + } > + > + error = parse_l4_port_range(ctx->argv[4], &destination_port); > + if (error) { > + ctx->error = error; > + goto cleanup; > + } > + } > + > + size_t target_ips_start_index; > + if (protocol == LSP_ICMP_HEALTH_CHECK) { > + target_ips_start_index = 4; > + } else { > + target_ips_start_index = 5; > + } > + > + if (ctx->argc > target_ips_start_index) { > + lsp_ip_addresses = _get_lsp_ip_addresses(lsp->addresses, > + lsp->n_addresses, > + &lsp_n_ip_addresses); > + > + for (int i = target_ips_start_index; i < ctx->argc; i++) { > + char *ip_addr = ctx->argv[i]; > + > + char *validated_ip = normalize_addr_str(ip_addr); > + if (!validated_ip) { > + free(validated_ip); > + ctl_error(ctx, "%s: Not a valid IPv4 or IPv6 address", > + ip_addr); > + goto cleanup; > + } > + > + if (!_lsp_contains_ip_address(lsp_ip_addresses, > + lsp_n_ip_addresses, > + ip_addr)) { > + free(validated_ip); > + ctl_error(ctx, "%s: Address %s not configured on port", > + lsp->name, ip_addr); > + goto cleanup; > + } > + > + sset_add(&target_ips, validated_ip); > + free(validated_ip); > + } > + } > + > + lsp_health_check_get_duplicate(protocol, > + src_ip, > + destination_port, > + &target_ips, > + lsp, > + &lsp_hc); > + > + if (lsp_hc) { > + ctl_error(ctx, "Health check already exists"); > + goto cleanup; > + } > + > + lsp_hc = nbrec_logical_switch_port_health_check_insert(ctx->txn); > + nbrec_logical_switch_port_health_check_set_protocol(lsp_hc, type); > + nbrec_logical_switch_port_health_check_set_src_ip(lsp_hc, src_ip); > + nbrec_logical_switch_port_health_check_set_port(lsp_hc, destination_port); > + > + if (ctx->argc > target_ips_start_index) { > + size_t num_target_ips = ctx->argc - target_ips_start_index; > + nbrec_logical_switch_port_health_check_set_addresses( > + lsp_hc, (const char **) ctx->argv + target_ips_start_index, > + num_target_ips); > + } else { > + nbrec_logical_switch_port_health_check_set_addresses(lsp_hc, NULL, 0); > + } > + > + nbrec_logical_switch_port_update_health_checks_addvalue(lsp, lsp_hc); > + > +cleanup: > + if (lsp_ip_addresses) { > + for (size_t i = 0; i < lsp_n_ip_addresses; i++) { > + free(lsp_ip_addresses[i]); > + } > + free(lsp_ip_addresses); > + } > + sset_destroy(&target_ips); > + free(validated_src_ip); > +} > + > +static void > +nbctl_pre_lsp_health_check_del(struct ctl_context *ctx) > +{ > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_col_name); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_col_health_checks); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_health_check_col_protocol); > +} > + > +static void > +nbctl_lsp_health_check_del(struct ctl_context *ctx) > +{ > + const struct nbrec_logical_switch_port_health_check *lsp_hc = NULL; > + const struct nbrec_logical_switch_port *lsp = NULL; > + const char *port_id = ctx->argv[1]; > + const char *hc_uuid = ctx->argv[2]; > + > + char *error; > + error = lsp_by_name_or_uuid(ctx, port_id, true, &lsp); > + if (error) { > + ctx->error = error; > + return; > + } > + > + if (!lsp) { > + ctl_error(ctx, "Logical Switch Port with id %s not found", > + port_id); > + return; > + } > + > + if (hc_uuid) { > + error = find_logical_switch_port_health_check_by_uuid(ctx, > + hc_uuid, > + &lsp_hc); > + if (error) { > + ctx->error = error; > + return; > + } > + > + nbrec_logical_switch_port_update_health_checks_delvalue(lsp, lsp_hc); > + nbrec_logical_switch_port_health_check_delete(lsp_hc); > + return; > + } > + > + for (size_t i = 0; i < lsp->n_health_checks; i++) { > + nbrec_logical_switch_port_update_health_checks_delvalue( > + lsp, lsp->health_checks[i]); > + nbrec_logical_switch_port_health_check_delete(lsp->health_checks[i]); > + } > +} > + > +static int > +cmp_lsp_hc(const void *lsp_hc_1_, const void *lsp_hc_2_) > +{ > + const struct nbrec_logical_switch_port_health_check *const *lsp_hc_1p = > + lsp_hc_1_; > + const struct nbrec_logical_switch_port_health_check *const *lsp_hc_2p = > + lsp_hc_2_; > + const struct nbrec_logical_switch_port_health_check *lsp_hc_1 = > + *lsp_hc_1p; > + const struct nbrec_logical_switch_port_health_check *lsp_hc_2 = > + *lsp_hc_2p; > + > + int src_ip_cmp = strcmp(lsp_hc_1->src_ip, lsp_hc_2->src_ip); > + if (src_ip_cmp) { > + return src_ip_cmp; > + } > + > + int protocol_cmp = strcmp(lsp_hc_1->protocol, lsp_hc_2->protocol); > + if (protocol_cmp != 0) { > + return protocol_cmp; > + } > + > + if (strcmp(lsp_hc_1->protocol, "icmp") != 0) { > + if (lsp_hc_1->port != lsp_hc_2->port) { > + return lsp_hc_1->port < lsp_hc_2->port ? -1 : 1; > + } > + } > + > + if (lsp_hc_1->n_addresses != lsp_hc_2->n_addresses) { > + return lsp_hc_1->n_addresses < lsp_hc_2->n_addresses ? -1 : 1; > + } > + > + size_t min_n_addresses = lsp_hc_1->n_addresses < lsp_hc_2->n_addresses ? > + lsp_hc_1->n_addresses : lsp_hc_2->n_addresses; min_n_addresses is unnecessary. The previous if statement already ensured that lsp_hc_1->n_addresses must be equal to lsp_hc_2->n_addresses. > + > + for (size_t i = 0; i < min_n_addresses; i++) { > + int ip_cmp = strcmp(lsp_hc_1->addresses[i], lsp_hc_2->addresses[i]); > + if (ip_cmp) { > + return ip_cmp; > + } > + } > + > + return 0; > +} > + > +static void > +nbctl_pre_lsp_health_check_list(struct ctl_context *ctx) > +{ > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_col_name); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_col_health_checks); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_health_check_col_port); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_health_check_col_protocol); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_health_check_col_src_ip); > + ovsdb_idl_add_column(ctx->idl, > + &nbrec_logical_switch_port_health_check_col_addresses); > +} > + > +static void > +nbctl_lsp_health_check_list(struct ctl_context *ctx) > +{ > + const struct nbrec_logical_switch_port_health_check **lsp_hcs; > + const struct nbrec_logical_switch_port *lsp = NULL; > + const char *port = ctx->argv[1]; > + > + char *error; > + error = lsp_by_name_or_uuid(ctx, port, true, &lsp); > + if (error) { > + ctx->error = error; > + return; > + } > + > + if (!lsp->n_health_checks) { > + return; > + } > + > + lsp_hcs = xmalloc(sizeof *lsp_hcs * lsp->n_health_checks); > + for (size_t i = 0; i < lsp->n_health_checks; i++) { > + lsp_hcs[i] = lsp->health_checks[i]; > + } > + > + qsort(lsp_hcs, lsp->n_health_checks, sizeof *lsp_hcs, cmp_lsp_hc); > + > + ds_put_format(&ctx->output, "Logical Switch Port %s:\n", port); > + for (size_t i = 0; i < lsp->n_health_checks; i++) { > + const struct nbrec_logical_switch_port_health_check *hc > + = lsp_hcs[i]; > + ds_put_format(&ctx->output, " Protocol : %s\n", > + hc->protocol); > + ds_put_format(&ctx->output, " Source IP : %s\n", > + hc->src_ip); > + if (strcmp(hc->protocol, "icmp")) { > + ds_put_format(&ctx->output, " Port : %"PRId64"\n", > + hc->port); > + } > + if (hc->n_addresses) { > + ds_put_format(&ctx->output, " Addresses : "); > + for (size_t j = 0; j < hc->n_addresses; j++) { > + if (j > 0) { > + ds_put_format(&ctx->output, ", "); > + } > + ds_put_format(&ctx->output, "%s", hc->addresses[j]); > + } > + ds_put_format(&ctx->output, "\n"); > + } > + int interval = smap_get_int(&hc->options, "interval", 0); > + int timeout = smap_get_int(&hc->options, "timeout", 0); > + int success_count = smap_get_int(&hc->options, "success_count", 0); > + int failure_count = smap_get_int(&hc->options, "failure_count", 0); > + if (interval) { > + ds_put_format(&ctx->output, " Interval : %d\n", > + interval); > + } > + if (timeout) { > + ds_put_format(&ctx->output, " Timeout : %d\n", > + timeout); > + } > + if (success_count) { > + ds_put_format(&ctx->output, " Success count : %d\n", > + success_count); > + } > + if (failure_count) { > + ds_put_format(&ctx->output, " Failure count : %d\n", > + failure_count); > + } > + if (i < lsp->n_health_checks - 1) { > + ds_put_format(&ctx->output, "\n"); > + } > + } > +} > + > static const struct ctl_table_class tables[NBREC_N_TABLES] = { > [NBREC_TABLE_DHCP_OPTIONS].row_ids > = {{&nbrec_logical_switch_port_col_name, NULL, > @@ -9064,6 +9566,16 @@ static const struct ctl_command_syntax nbctl_commands[] = { > nbctl_pre_static_mac_binding, nbctl_static_mac_binding_list, NULL, > "", RO }, > > + /* Health Check commands */ > + {"lsp-hc-add", 2, INT_MAX, "PORT TYPE SRC_IP [DST_PORT] [ADDRESS]", > + nbctl_pre_lsp_health_check_add, nbctl_lsp_health_check_add, > + NULL, "", RW }, > + {"lsp-hc-del", 1, INT_MAX, "PORT [HC_UUID]", > + nbctl_pre_lsp_health_check_del, nbctl_lsp_health_check_del, > + NULL, "", RW }, > + {"lsp-hc-list", 1, 1, "PORT", nbctl_pre_lsp_health_check_list, > + nbctl_lsp_health_check_list, NULL, "", RW }, > + > {NULL, 0, 0, NULL, NULL, NULL, NULL, "", RO}, > }; > > -- > 2.48.1 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 1/21/26 8:30 PM, Mark Michelson via dev wrote: > Hi Alexandra, > > I have several notes inline below. > > On Fri, Dec 19, 2025 at 4:49 AM Alexandra Rukomoinikova > <arukomoinikova@k2.cloud> wrote: >> >> 1) Added tables for further implementation logic of service monitors for >> logical switch ports: >> >> New table: >> - Logical_Switch_Port_Health_Check: Health check configuration >> for logical switch port. >> >> Modified tables: >> - Logical_Switch_Port: Add 'health_checks' column referencing >> health checks configuration. >> >> 2) Added commands to create, delete, and describe health checks for logical switch ports. >> >> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> >> --- Hi Alexandra, Mark, Thanks for the patch! On top of Mark's comments I have a few of my own. >> lib/ovn-util.c | 17 ++ >> lib/ovn-util.h | 2 + >> ovn-nb.ovsschema | 28 ++- >> ovn-nb.xml | 54 ++++ >> tests/ovn-nbctl.at | 186 ++++++++++++++ >> utilities/ovn-nbctl.8.xml | 54 ++++ >> utilities/ovn-nbctl.c | 512 ++++++++++++++++++++++++++++++++++++++ >> 7 files changed, 851 insertions(+), 2 deletions(-) >> One of the patches in the series (maybe this one?) should add a NEWS entry about this new feature. >> diff --git a/lib/ovn-util.c b/lib/ovn-util.c >> index cec029e42..16f7eb2b9 100644 >> --- a/lib/ovn-util.c >> +++ b/lib/ovn-util.c >> @@ -1704,3 +1704,20 @@ is_partial_uuid_match(const struct uuid *uuid, const char *match) >> s2 = strip_leading_zero(s2); >> return !strncmp(s1, s2, strlen(s2)); >> } >> + >> +char * >> +skip_mac_address_from_lsp_address(const char *address_with_mac) >> +{ >> + if (!address_with_mac) { >> + return NULL; >> + } >> + const char *ptr = address_with_mac; >> + while (*ptr == ' ') { >> + ptr++; >> + } >> + ptr += (char) 17; This is unsafe. What if the LSP has addresses=["unknown"]? >> + while (*ptr == ' ') { >> + ptr++; >> + } >> + return (char *) ptr; >> +} Why not use extract_ip_addresses() that alread exists in ovn-util.c? (But Mark also mentioned that.) >> diff --git a/lib/ovn-util.h b/lib/ovn-util.h >> index daff01995..7685118e6 100644 >> --- a/lib/ovn-util.h >> +++ b/lib/ovn-util.h >> @@ -775,4 +775,6 @@ NEIGH_REDISTRIBUTE_MODES >> enum neigh_redistribute_mode >> parse_neigh_dynamic_redistribute(const struct smap *options); >> >> +char *skip_mac_address_from_lsp_address(const char *address_with_mac); >> + >> #endif /* OVN_UTIL_H */ >> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema >> index 8c2c1d861..5373e6f59 100644 >> --- a/ovn-nb.ovsschema >> +++ b/ovn-nb.ovsschema >> @@ -1,7 +1,7 @@ >> { >> "name": "OVN_Northbound", >> - "version": "7.15.0", >> - "cksum": "4060410729 43708", >> + "version": "7.16.0", >> + "cksum": "3363473672 44948", >> "tables": { >> "NB_Global": { >> "columns": { >> @@ -179,6 +179,11 @@ >> "refType": "strong"}, >> "min": 0, >> "max": 1}}, >> + "health_checks": {"type": {"key": {"type": "uuid", >> + "refTable": "Logical_Switch_Port_Health_Check", >> + "refType": "weak"}, I would call this "health_check" (singular). I assume you may want to define a list of them though if you want to have different health checks, e.g., for TCP and UDP. But to be in line with the rest of the health check mechanisms in the schema (LB and NFG) I'd use singular. >> + "min": 0, >> + "max": "unlimited"}}, >> "external_ids": { >> "type": {"key": "string", "value": "string", >> "min": 0, "max": "unlimited"}}}, >> @@ -246,6 +251,25 @@ >> "min": 0, "max": "unlimited"}}}, >> "indexes": [["name"], ["id"]], >> "isRoot": true}, >> + "Logical_Switch_Port_Health_Check": { >> + "columns": { >> + "protocol": { >> + "type": {"key": {"type": "string", >> + "enum": ["set", ["tcp", "udp", "icmp"]]}, >> + "min": 0, "max": 1}}, >> + "src_ip": {"type": "string"}, >> + "port": {"type": {"key": {"type": "integer", >> + "minInteger": 0, >> + "maxInteger": 65535}}}, >> + "addresses": {"type": {"key": "string", >> + "min": 0, >> + "max": "unlimited"}}, >> + "options": { >> + "type": {"key": "string", >> + "value": "string", >> + "min": 0, >> + "max": "unlimited"}}}, >> + "isRoot": true}, > > IMO, it makes sense for this to have isRoot: false. I can't see a > reason for a logical switch port health check to exist without being > attached to a logical switch port. > If we do decide to keep it as root table then it should have an index. Otherwise, when running in RAFT there's a small (but theoretically non-zero) chance that record are spuriously duplicated on insertion. Maybe a name/ID column in that case? >> "Forwarding_Group": { >> "columns": { >> "name": {"type": "string"}, >> diff --git a/ovn-nb.xml b/ovn-nb.xml >> index 730293e97..3cc46e60c 100644 >> --- a/ovn-nb.xml >> +++ b/ovn-nb.xml >> @@ -2115,6 +2115,11 @@ >> Please see the <ref table="Mirror"/> table. >> </column> >> >> + <column name="health_checks"> >> + Health checks configuration for this logical switch port. >> + Please see the <ref table="Logical_Switch_Port_Health_Check"/> table. >> + </column> >> + >> <column name="ha_chassis_group"> >> References a row in the OVN Northbound database's >> <ref table="HA_Chassis_Group" db="OVN_Northbound"/> table. >> @@ -2171,6 +2176,55 @@ >> </group> >> </table> >> >> + <table name="Logical_Switch_Port_Health_Check" >> + title="logical switch port health check"> >> + <p> >> + Each row represents health check configuration for logical switch port. >> + Health checks are used to monitor reachability and health of backend >> + endpoints associated with logical switch port. Health check can be >> + configured to use different protocols (TCP, UDP, or ICMP) to verify >> + availability of target IP addresses. >> + </p> >> + >> + <column name="protocol"> >> + Valid protocols are <code>tcp</code>, <code>udp</code>, or >> + <code>icmp</code>. For <code>tcp</code> and <code>udp</code> >> + protocols - destination port must be specified. >> + </column> >> + >> + <column name="src_ip"> >> + Source IP address used when sending health check probes. >> + </column> >> + >> + <column name="port"> >> + Destination port number for TCP and UDP health checks. >> + </column> >> + >> + <column name="addresses"> >> + A set of IP addresses to monitor. If no specific addresses are provided, >> + health check will use all addresses configured on logical switch port. >> + </column> >> + >> + <column name="options" key="interval" type='{"type": "integer"}'> >> + The interval, in seconds, between service monitor checks. >> + </column> >> + >> + <column name="options" key="timeout" type='{"type": "integer"}'> >> + The time, in seconds, after which the service monitor check times >> + out. >> + </column> >> + >> + <column name="options" key="success_count" type='{"type": "integer"}'> >> + The number of successful checks after which the service is >> + considered <code>online</code>. >> + </column> >> + >> + <column name="options" key="failure_count" type='{"type": "integer"}'> >> + The number of failure checks after which the service is considered >> + <code>offline</code>. >> + </column> >> + </table> >> + >> <table name="Forwarding_Group" title="forwarding group"> >> <p> >> Each row represents one forwarding group. >> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at >> index dccf30758..62b89945b 100644 >> --- a/tests/ovn-nbctl.at >> +++ b/tests/ovn-nbctl.at >> @@ -3499,3 +3499,189 @@ AT_CHECK([ovn-nbctl --may-exist lsp-add-localnet-port ls ln_port net1], [1], [], >> check ovn-nbctl lsp-set-options ln_port network_name=net1 >> check ovn-nbctl --may-exist lsp-add-localnet-port ls ln_port net1 >> ]) >> + >> +dnl --------------------------------------------------------------------- >> + >> +AT_SETUP([ovn-nbctl - Logical Switch Port Health Check]) >> +OVN_NBCTL_TEST_START daemon >> + >> +# Create logical switch port >> +AT_CHECK([ovn-nbctl ls-add ls0]) >> +AT_CHECK([ovn-nbctl lsp-add ls0 lport0]) >> +AT_CHECK([ovn-nbctl lsp-set-addresses lport0 "00:11:22:33:44:55 192.168.1.10" "00:11:22:33:44:56 192.168.1.11"]) >> + >> +# Create icmp health check with the specified addresses >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 192.168.0.255 192.168.1.10 192.168.1.11]) >> +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl >> +Logical Switch Port lport0: >> + Protocol : icmp >> + Source IP : 192.168.0.255 >> + Addresses : 192.168.1.10, 192.168.1.11 >> +]) >> + >> +# Check hc attaching to logical switch port >> +hc_icmp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="192.168.0.255") >> +check_column "$hc_icmp_uuid" nb:logical_switch_port health_checks >> + >> +# Create tcp health check with the specified addresses >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.2 80 192.168.1.10]) >> +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl >> +Logical Switch Port lport0: >> + Protocol : tcp >> + Source IP : 10.0.0.2 >> + Port : 80 >> + Addresses : 192.168.1.10 >> + >> + Protocol : icmp >> + Source IP : 192.168.0.255 >> + Addresses : 192.168.1.10, 192.168.1.11 >> +]) >> +hc_tcp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="10.0.0.2") >> + >> +# Create udp health check with the specified addresses >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 udp 10.0.0.3 53 192.168.1.11]) >> +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl >> +Logical Switch Port lport0: >> + Protocol : tcp >> + Source IP : 10.0.0.2 >> + Port : 80 >> + Addresses : 192.168.1.10 >> + >> + Protocol : udp >> + Source IP : 10.0.0.3 >> + Port : 53 >> + Addresses : 192.168.1.11 >> + >> + Protocol : icmp >> + Source IP : 192.168.0.255 >> + Addresses : 192.168.1.10, 192.168.1.11 >> +]) >> +hc_udp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="10.0.0.3") >> + >> +check_column "$hc_icmp_uuid $hc_tcp_uuid $hc_udp_uuid" nb:logical_switch_port health_checks >> + >> +AT_CHECK([ovn-nbctl lsp-hc-del lport0 $hc_icmp_uuid]) >> +# Check hcs detaching to logical switch port >> +check_column "$hc_tcp_uuid $hc_udp_uuid" nb:logical_switch_port health_checks >> + >> +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl >> +Logical Switch Port lport0: >> + Protocol : tcp >> + Source IP : 10.0.0.2 >> + Port : 80 >> + Addresses : 192.168.1.10 >> + >> + Protocol : udp >> + Source IP : 10.0.0.3 >> + Port : 53 >> + Addresses : 192.168.1.11 >> +]) >> + >> +AT_CHECK([ovn-nbctl lsp-hc-del lport0]) >> +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl]) >> + >> +# Create health check without specified addresses >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.4]) >> +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl >> +Logical Switch Port lport0: >> + Protocol : icmp >> + Source IP : 10.0.0.4 >> +]) >> + >> +# Check invalid protocol >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 stcp 10.0.0.1], [1], [], [stderr]) >> +AT_CHECK([grep "Type must be icmp, tcp or udp" stderr], [0], [ignore]) >> + >> +# Check invalid source IP >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp invalid_ip], [1], [], [stderr]) >> +AT_CHECK([grep "Not a valid IPv4 or IPv6 address" stderr], [0], [ignore]) >> + >> +# Check TCP/UDP without destination port >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1], [1], [], [stderr]) >> +AT_CHECK([grep "Destination port required for tcp health check" stderr], [0], [ignore]) >> + >> +# Check invalid destination port >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 70000], [1], [], [stderr]) >> +AT_CHECK([grep "port must in range 0...65535" stderr], [0], [ignore]) >> + >> +# Check IP address not configured on port >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.1 192.168.99.99], [1], [], [stderr]) >> +AT_CHECK([grep "Address 192.168.99.99 not configured on port" stderr], [0], [ignore]) >> + >> +# Check Duplicate health check configuration >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.4], [1], [], [stderr]) >> +AT_CHECK([grep "Health check already exists" stderr], [0], [ignore]) >> + >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.1 192.168.1.10]) >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.1 192.168.1.10], [1], [], [stderr]) >> +AT_CHECK([grep "Health check already exists" stderr], [0], [ignore]) >> + >> +# Check same protocol with different ports - it's ok >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 90 192.168.1.10]) >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 443 192.168.1.10], [0], [], [ignore]) >> + >> +# Check different protocol with same ports - it's ok >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 80 192.168.1.10]) >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 udp 10.0.0.1 80 192.168.1.10], [0], [], [ignore]) >> + >> +# Check different source ip with same ports and addresses - it's ok >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 92 192.168.1.10]) >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.2 92 192.168.1.10], [0], [], [ignore]) >> + >> +# Check multipal same addresses >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 93 192.168.1.10 192.168.1.11]) >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 93 192.168.1.10 192.168.1.11], [1], [], [stderr]) >> +AT_CHECK([grep "Health check already exists" stderr], [0], [ignore]) >> + >> +# Check supported logical switch port type for monitoring >> +AT_CHECK([ovn-nbctl lsp-add ls0 lport1]) >> +AT_CHECK([ovn-nbctl lsp-set-type lport1 router]) >> +AT_CHECK([ovn-nbctl lsp-hc-add lport1 icmp 10.0.0.1], [1], [], [stderr]) >> +AT_CHECK([grep "Health check monitoring supported only for port with vif type" stderr], [0], [ignore]) >> + >> +AT_CHECK([ovn-nbctl lsp-hc-del lport0]) >> + >> +# Test IPv6 addresses support >> +AT_CHECK([ovn-nbctl lsp-add ls0 lport3]) >> +AT_CHECK([ovn-nbctl lsp-set-addresses lport3 "00:11:22:33:44:57 2001:db8::1"]) >> +AT_CHECK([ovn-nbctl lsp-hc-add lport3 icmp 2001:db8::2 2001:db8::1]) >> +AT_CHECK([ovn-nbctl lsp-hc-list lport3], [0], [dnl >> +Logical Switch Port lport3: >> + Protocol : icmp >> + Source IP : 2001:db8::2 >> + Addresses : 2001:db8::1 >> +]) >> + >> +# Check options printing >> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.4 192.168.1.10 192.168.1.11]) >> +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl >> +Logical Switch Port lport0: >> + Protocol : icmp >> + Source IP : 10.0.0.4 >> + Addresses : 192.168.1.10, 192.168.1.11 >> +]) >> +hc_icmp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="10.0.0.4") >> + >> +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:interval=3]) >> +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:timeout=30]) >> +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:success_count=1]) >> +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:failure_count=2]) >> + >> +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl >> +Logical Switch Port lport0: >> + Protocol : icmp >> + Source IP : 10.0.0.4 >> + Addresses : 192.168.1.10, 192.168.1.11 >> + Interval : 3 >> + Timeout : 30 >> + Success count : 1 >> + Failure count : 2 >> +]) >> + >> +AT_CHECK([ovn-nbctl lsp-hc-del lport0]) >> +AT_CHECK([ovn-nbctl lsp-hc-del lport3]) >> +check_row_count nb:Logical_Switch_Port_Health_Check 0 >> + >> +OVN_NBCTL_TEST_STOP "/terminating with signal 15/d" >> +AT_CLEANUP >> + >> diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml >> index 7df902944..efc781d77 100644 >> --- a/utilities/ovn-nbctl.8.xml >> +++ b/utilities/ovn-nbctl.8.xml >> @@ -1822,6 +1822,60 @@ >> </dd> >> </dl> >> >> + <h2>Health Check commands</h2> >> + <dl> >> + <dt><code>lsp-hc-add</code> <var>PORT</var> <var>PROTOCOL</var> >> + <var>SOURCE_IP</var> [<var>DST_PORT</var>] [<var>ADDRESS</var>]...</dt> >> + <dd> >> + <p> >> + Creates a new health check configuration for the logical switch port >> + <code>PORT</code> with the below mandatory arguments. >> + </p> >> + >> + <p> >> + <var>PROTOCOL</var> specifies the health check protocol - >> + <code>tcp</code>, <code>udp</code>, or <code>icmp</code>. >> + </p> >> + >> + <p> >> + <var>SOURCE_IP</var> specifies the source IP address used when >> + sending health check probes. >> + </p> >> + >> + <p> >> + <var>DST_PORT</var> specifies the destination port number for >> + <code>tcp</code> and <code>udp</code> health checks. This parameter >> + is ignored for <code>icmp</code> protocol. >> + </p> >> + >> + <p> >> + <var>ADDRESS</var> specifies one or more IP addresses to monitor. >> + If no addresses are provided, the health check will monitor all IP >> + addresses configured on the logical switch port. >> + </p> >> + >> + <p> >> + Additional health check options such as interval, timeout, >> + success count, and failure count can be configured separately. >> + </p> >> + </dd> >> + >> + <dt><code>lsp-hc-del</code> <var>PORT</var> [<var>HC_UUID</var>]</dt> >> + <dd> >> + <p> >> + Deletes health check configuration for the logical switch port >> + <code>PORT</code>. >> + </p> >> + </dd> >> + >> + <dt><code>lsp-hc-list</code> <var>PORT</var></dt> >> + <dd> >> + Lists all health check configurations for the logical switch port >> + <code>PORT</code>, including protocol, source IP, destination port, >> + monitored addresses, and current status. >> + </dd> >> + </dl> >> + >> <h2>Synchronization Commands</h2> >> >> <dl> >> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c >> index cdf6b578a..b8802ab30 100644 >> --- a/utilities/ovn-nbctl.c >> +++ b/utilities/ovn-nbctl.c >> @@ -66,6 +66,18 @@ string_ptr(char *ptr) >> return (ptr) ? ptr : s; >> } >> >> +static char *OVS_WARN_UNUSED_RESULT >> +parse_l4_port_range(const char *arg, int64_t *port_p) >> +{ >> + int64_t port; >> + if (!ovs_scan(arg, "%"SCNd64, &port) >> + || port < 0 || port > UINT16_MAX) { >> + return xasprintf("%s: port must in range 0...65535", arg); >> + } >> + *port_p = port; >> + return NULL; >> +} >> + >> static void >> nbctl_add_base_prerequisites(struct ovsdb_idl *idl, >> enum nbctl_wait_type wait_type) >> @@ -544,6 +556,12 @@ MAC_Binding commands:\n\ >> Delete Static_MAC_Binding entry\n\ >> static-mac-binding-list List all Static_MAC_Binding entries\n\ >> \n\ >> +Logical Switch Port Health Check:\n\ >> + lsp-hc-add PORT PROTOCOL SOURCE_IP DST_PORT [ADDRESS]...\n\ >> + add health check monitoring for PORT\n\ >> + lsp-hc-del PORT HC_UUID delete health check monitoring for PORT\n\ >> + lsp-hc-list PORT print health check for PORT\n\ >> +\n\ >> %s\ >> %s\ >> \n\ >> @@ -8677,6 +8695,490 @@ nbctl_lsp_add_misc_port(struct ctl_context *ctx) >> shash_add(&nbctx->lsp_to_ls_map, lsp_name, ls); >> } >> >> +/* Logical Switch Port Health Check Functions. */ >> +enum health_check_protocol { >> + LSP_ICMP_HEALTH_CHECK, >> + LSP_TCP_HEALTH_CHECK, >> + LSP_UDP_HEALTH_CHECK, >> +}; >> + >> +static bool >> +parse_health_check_protocol(struct ctl_context *ctx, const char *type, >> + enum health_check_protocol *protocol) >> +{ >> + if (!strcmp(type, "icmp")) { >> + *protocol = LSP_ICMP_HEALTH_CHECK; >> + return true; >> + } else if (!strcmp(type, "tcp")) { >> + *protocol = LSP_TCP_HEALTH_CHECK; >> + return true; >> + } else if (!strcmp(type, "udp")) { >> + *protocol = LSP_UDP_HEALTH_CHECK; >> + return true; >> + } else { >> + ctl_error(ctx, "%s: Type must be icmp, tcp or udp", type); >> + return false; >> + } >> +} >> + >> +static void >> +lsp_health_check_get_duplicate( >> + int proto, const char *src_ip, >> + int64_t port, struct sset *addresses, >> + const struct nbrec_logical_switch_port *lsp, >> + const struct nbrec_logical_switch_port_health_check **lsp_hc) >> +{ > > Instead of returning void, this could return a const struct > nbrec_logical_switch_port_health_check pointer. A NULL return means no > duplicate was found. A non-NULL return means we found a duplicate. > >> + *lsp_hc = NULL; >> + >> + for (size_t i = 0; i < lsp->n_health_checks; i++) { >> + const struct nbrec_logical_switch_port_health_check *lsp_hc_p = >> + lsp->health_checks[i]; >> + >> + if (src_ip && strcmp(lsp_hc_p->src_ip, src_ip)) { >> + continue; >> + } >> + >> + const char *target_proto = >> + (proto == LSP_ICMP_HEALTH_CHECK) ? "icmp" : >> + (proto == LSP_TCP_HEALTH_CHECK) ? "tcp" : "udp"; >> + if (strcmp(lsp_hc_p->protocol, target_proto)) { >> + continue; >> + } >> + >> + if (proto != LSP_ICMP_HEALTH_CHECK && lsp_hc_p->port != port) { >> + continue; >> + } >> + >> + if (!addresses || sset_is_empty(addresses)) { >> + *lsp_hc = lsp_hc_p; >> + return; >> + } > > This seems incorrect to me. Imagine that we have a logical switch port > with IP addresses A, B, and C. We start by adding a health check that > specifies target addresses A and B. Then we add a new health check > with no target addresses. With this code, we would return the existing > health check that only monitors addresses A and B. However, since the > new health check has no target addresses, the intent likely was for > the new health check to monitor addresses A, B, and C. This means that > address C will not be monitored. > >> + >> + bool addresses_found = true; >> + for (size_t j = 0; j < lsp_hc_p->n_addresses; j++) { >> + if (!sset_contains(addresses, lsp_hc_p->addresses[j])) { >> + addresses_found = false; >> + break; >> + } >> + } >> + >> + if (addresses_found) { >> + *lsp_hc = lsp_hc_p; >> + return; >> + } >> + } >> +} >> + >> +static char ** >> +_get_lsp_ip_addresses(char **lsp_addresses, >> + int lsp_n_addresses, >> + int *lsp_n_ip_addresses) >> +{ > > This function is unnecessary. You can just use extract_lsp_addresses() > in ovn-util to get the IP addresses from a logical switch port. It's > also safer to use, since skip_mac_address_from_lsp_address() has the > possibility to return a pointer past the end of the string if the LSP > MAC is malformed. > +1 >> + char **lsp_ip_addresses_p = NULL; >> + *lsp_n_ip_addresses = 0; >> + size_t n_capacity = 0; >> + size_t count = 0; >> + >> + for (size_t i = 0; i < lsp_n_addresses; i++) { >> + char *address_without_mac = >> + skip_mac_address_from_lsp_address(lsp_addresses[i]); >> + >> + if (address_without_mac && address_without_mac[0]) { >> + if (count == n_capacity) { >> + lsp_ip_addresses_p = x2nrealloc(lsp_ip_addresses_p, >> + &n_capacity, >> + sizeof *lsp_ip_addresses_p); >> + } >> + lsp_ip_addresses_p[count] = xstrdup(address_without_mac); >> + count++; >> + } >> + } >> + >> + *lsp_n_ip_addresses = count; >> + return lsp_ip_addresses_p; >> +} >> + >> +static bool >> +_lsp_contains_ip_address(char **lsp_ip_addresses, >> + size_t lsp_n_ip_addresses, >> + char *ip_address) >> +{ If we use the already existing 'struct lport_addresses' helpers from ovn-util.[ch] we don't really need this function. We could use find_lport_address(). >> + for (size_t i = 0; i < lsp_n_ip_addresses; i++) { >> + if (!strcmp(lsp_ip_addresses[i], ip_address)) { >> + return true; >> + } >> + } >> + >> + return false; >> +} >> + >> +static char * OVS_WARN_UNUSED_RESULT >> +find_logical_switch_port_health_check_by_uuid( >> + struct ctl_context *ctx, >> + const char *id, >> + const struct nbrec_logical_switch_port_health_check **lsp_hc_p) >> +{ >> + const struct nbrec_logical_switch_port_health_check *lsp_hc; >> + *lsp_hc_p = NULL; >> + >> + struct uuid lsp_hc_uuid; >> + bool is_uuid = uuid_from_string(&lsp_hc_uuid, id); >> + >> + if (!is_uuid) { >> + return xasprintf("%s: Invalid UUID format", id); >> + } >> + >> + lsp_hc = nbrec_logical_switch_port_health_check_get_for_uuid( >> + ctx->idl, &lsp_hc_uuid); >> + >> + if (!lsp_hc) { >> + return xasprintf("%s: Logical Switch Port Health Check not found", id); >> + } >> + >> + *lsp_hc_p = lsp_hc; >> + >> + return NULL; >> +} >> + >> +static void >> +nbctl_pre_lsp_health_check_add(struct ctl_context *ctx) >> +{ >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_col_name); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_col_addresses); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_col_health_checks); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_health_check_col_port); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_health_check_col_protocol); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_health_check_col_src_ip); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_health_check_col_addresses); >> +} >> + >> +static void >> +nbctl_lsp_health_check_add(struct ctl_context *ctx) >> +{ >> + const struct nbrec_logical_switch_port *lsp = NULL; >> + const struct nbrec_logical_switch_port_health_check *lsp_hc = NULL; >> + >> + const char *port = ctx->argv[1]; >> + const char *type = ctx->argv[2]; >> + const char *src_ip = ctx->argv[3]; >> + enum health_check_protocol protocol; >> + char **lsp_ip_addresses = NULL; >> + int lsp_n_ip_addresses = 0; >> + char *validated_src_ip = NULL; >> + struct sset target_ips; >> + sset_init(&target_ips); >> + >> + char *error; >> + error = lsp_by_name_or_uuid(ctx, port, true, &lsp); >> + if (error) { >> + ctx->error = error; >> + return; >> + } >> + >> + if (lsp->type[0]) { >> + ctl_error(ctx, "%s: Health check monitoring supported only for" >> + " port with vif type", lsp->type); >> + goto cleanup; >> + } >> + >> + if (!parse_health_check_protocol(ctx, type, &protocol)) { >> + goto cleanup; >> + } >> + >> + validated_src_ip = normalize_addr_str(src_ip); >> + if (!validated_src_ip) { >> + ctl_error(ctx, "%s: Not a valid IPv4 or IPv6 address", >> + ctx->argv[3]); >> + goto cleanup; >> + } >> + >> + int64_t destination_port = 0; >> + if (protocol == LSP_TCP_HEALTH_CHECK || >> + protocol == LSP_UDP_HEALTH_CHECK) { >> + if (ctx->argc < 5) { >> + ctl_error(ctx, "Destination port required for %s health check", >> + type); >> + goto cleanup; >> + } >> + >> + error = parse_l4_port_range(ctx->argv[4], &destination_port); >> + if (error) { >> + ctx->error = error; >> + goto cleanup; >> + } >> + } >> + >> + size_t target_ips_start_index; >> + if (protocol == LSP_ICMP_HEALTH_CHECK) { >> + target_ips_start_index = 4; >> + } else { >> + target_ips_start_index = 5; >> + } >> + >> + if (ctx->argc > target_ips_start_index) { >> + lsp_ip_addresses = _get_lsp_ip_addresses(lsp->addresses, >> + lsp->n_addresses, >> + &lsp_n_ip_addresses); >> + >> + for (int i = target_ips_start_index; i < ctx->argc; i++) { >> + char *ip_addr = ctx->argv[i]; >> + >> + char *validated_ip = normalize_addr_str(ip_addr); >> + if (!validated_ip) { >> + free(validated_ip); >> + ctl_error(ctx, "%s: Not a valid IPv4 or IPv6 address", >> + ip_addr); >> + goto cleanup; >> + } >> + >> + if (!_lsp_contains_ip_address(lsp_ip_addresses, >> + lsp_n_ip_addresses, >> + ip_addr)) { >> + free(validated_ip); >> + ctl_error(ctx, "%s: Address %s not configured on port", >> + lsp->name, ip_addr); Do we want to be so restrictive? What if the switch port is configured just with a MAC address (or unknown) but the healthcheck IP is actually reachable inside the LSP workload? Would it make sense to remvoe this validation? >> + goto cleanup; >> + } >> + >> + sset_add(&target_ips, validated_ip); >> + free(validated_ip); >> + } >> + } >> + >> + lsp_health_check_get_duplicate(protocol, >> + src_ip, >> + destination_port, >> + &target_ips, >> + lsp, >> + &lsp_hc); >> + >> + if (lsp_hc) { >> + ctl_error(ctx, "Health check already exists"); >> + goto cleanup; >> + } >> + >> + lsp_hc = nbrec_logical_switch_port_health_check_insert(ctx->txn); >> + nbrec_logical_switch_port_health_check_set_protocol(lsp_hc, type); >> + nbrec_logical_switch_port_health_check_set_src_ip(lsp_hc, src_ip); >> + nbrec_logical_switch_port_health_check_set_port(lsp_hc, destination_port); >> + >> + if (ctx->argc > target_ips_start_index) { >> + size_t num_target_ips = ctx->argc - target_ips_start_index; >> + nbrec_logical_switch_port_health_check_set_addresses( >> + lsp_hc, (const char **) ctx->argv + target_ips_start_index, >> + num_target_ips); >> + } else { >> + nbrec_logical_switch_port_health_check_set_addresses(lsp_hc, NULL, 0); >> + } >> + >> + nbrec_logical_switch_port_update_health_checks_addvalue(lsp, lsp_hc); >> + >> +cleanup: >> + if (lsp_ip_addresses) { >> + for (size_t i = 0; i < lsp_n_ip_addresses; i++) { >> + free(lsp_ip_addresses[i]); >> + } >> + free(lsp_ip_addresses); >> + } >> + sset_destroy(&target_ips); >> + free(validated_src_ip); >> +} >> + >> +static void >> +nbctl_pre_lsp_health_check_del(struct ctl_context *ctx) >> +{ >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_col_name); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_col_health_checks); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_health_check_col_protocol); >> +} >> + >> +static void >> +nbctl_lsp_health_check_del(struct ctl_context *ctx) >> +{ >> + const struct nbrec_logical_switch_port_health_check *lsp_hc = NULL; >> + const struct nbrec_logical_switch_port *lsp = NULL; >> + const char *port_id = ctx->argv[1]; >> + const char *hc_uuid = ctx->argv[2]; >> + >> + char *error; >> + error = lsp_by_name_or_uuid(ctx, port_id, true, &lsp); >> + if (error) { >> + ctx->error = error; >> + return; >> + } >> + >> + if (!lsp) { >> + ctl_error(ctx, "Logical Switch Port with id %s not found", >> + port_id); >> + return; >> + } >> + >> + if (hc_uuid) { >> + error = find_logical_switch_port_health_check_by_uuid(ctx, >> + hc_uuid, >> + &lsp_hc); >> + if (error) { >> + ctx->error = error; >> + return; >> + } >> + >> + nbrec_logical_switch_port_update_health_checks_delvalue(lsp, lsp_hc); >> + nbrec_logical_switch_port_health_check_delete(lsp_hc); >> + return; >> + } >> + >> + for (size_t i = 0; i < lsp->n_health_checks; i++) { >> + nbrec_logical_switch_port_update_health_checks_delvalue( >> + lsp, lsp->health_checks[i]); >> + nbrec_logical_switch_port_health_check_delete(lsp->health_checks[i]); >> + } >> +} >> + >> +static int >> +cmp_lsp_hc(const void *lsp_hc_1_, const void *lsp_hc_2_) >> +{ >> + const struct nbrec_logical_switch_port_health_check *const *lsp_hc_1p = >> + lsp_hc_1_; >> + const struct nbrec_logical_switch_port_health_check *const *lsp_hc_2p = >> + lsp_hc_2_; >> + const struct nbrec_logical_switch_port_health_check *lsp_hc_1 = >> + *lsp_hc_1p; >> + const struct nbrec_logical_switch_port_health_check *lsp_hc_2 = >> + *lsp_hc_2p; >> + >> + int src_ip_cmp = strcmp(lsp_hc_1->src_ip, lsp_hc_2->src_ip); >> + if (src_ip_cmp) { >> + return src_ip_cmp; >> + } >> + >> + int protocol_cmp = strcmp(lsp_hc_1->protocol, lsp_hc_2->protocol); >> + if (protocol_cmp != 0) { >> + return protocol_cmp; >> + } >> + >> + if (strcmp(lsp_hc_1->protocol, "icmp") != 0) { >> + if (lsp_hc_1->port != lsp_hc_2->port) { >> + return lsp_hc_1->port < lsp_hc_2->port ? -1 : 1; >> + } >> + } >> + >> + if (lsp_hc_1->n_addresses != lsp_hc_2->n_addresses) { >> + return lsp_hc_1->n_addresses < lsp_hc_2->n_addresses ? -1 : 1; >> + } >> + >> + size_t min_n_addresses = lsp_hc_1->n_addresses < lsp_hc_2->n_addresses ? >> + lsp_hc_1->n_addresses : lsp_hc_2->n_addresses; > > min_n_addresses is unnecessary. The previous if statement already > ensured that lsp_hc_1->n_addresses must be equal to > lsp_hc_2->n_addresses. > > >> + >> + for (size_t i = 0; i < min_n_addresses; i++) { >> + int ip_cmp = strcmp(lsp_hc_1->addresses[i], lsp_hc_2->addresses[i]); >> + if (ip_cmp) { >> + return ip_cmp; >> + } >> + } >> + >> + return 0; >> +} >> + >> +static void >> +nbctl_pre_lsp_health_check_list(struct ctl_context *ctx) >> +{ >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_col_name); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_col_health_checks); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_health_check_col_port); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_health_check_col_protocol); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_health_check_col_src_ip); >> + ovsdb_idl_add_column(ctx->idl, >> + &nbrec_logical_switch_port_health_check_col_addresses); >> +} >> + >> +static void >> +nbctl_lsp_health_check_list(struct ctl_context *ctx) >> +{ >> + const struct nbrec_logical_switch_port_health_check **lsp_hcs; >> + const struct nbrec_logical_switch_port *lsp = NULL; >> + const char *port = ctx->argv[1]; >> + >> + char *error; >> + error = lsp_by_name_or_uuid(ctx, port, true, &lsp); >> + if (error) { >> + ctx->error = error; >> + return; >> + } >> + >> + if (!lsp->n_health_checks) { >> + return; >> + } >> + >> + lsp_hcs = xmalloc(sizeof *lsp_hcs * lsp->n_health_checks); >> + for (size_t i = 0; i < lsp->n_health_checks; i++) { >> + lsp_hcs[i] = lsp->health_checks[i]; >> + } >> + >> + qsort(lsp_hcs, lsp->n_health_checks, sizeof *lsp_hcs, cmp_lsp_hc); >> + >> + ds_put_format(&ctx->output, "Logical Switch Port %s:\n", port); >> + for (size_t i = 0; i < lsp->n_health_checks; i++) { >> + const struct nbrec_logical_switch_port_health_check *hc >> + = lsp_hcs[i]; >> + ds_put_format(&ctx->output, " Protocol : %s\n", >> + hc->protocol); >> + ds_put_format(&ctx->output, " Source IP : %s\n", >> + hc->src_ip); >> + if (strcmp(hc->protocol, "icmp")) { >> + ds_put_format(&ctx->output, " Port : %"PRId64"\n", >> + hc->port); >> + } >> + if (hc->n_addresses) { >> + ds_put_format(&ctx->output, " Addresses : "); >> + for (size_t j = 0; j < hc->n_addresses; j++) { >> + if (j > 0) { >> + ds_put_format(&ctx->output, ", "); >> + } >> + ds_put_format(&ctx->output, "%s", hc->addresses[j]); >> + } >> + ds_put_format(&ctx->output, "\n"); >> + } >> + int interval = smap_get_int(&hc->options, "interval", 0); >> + int timeout = smap_get_int(&hc->options, "timeout", 0); >> + int success_count = smap_get_int(&hc->options, "success_count", 0); >> + int failure_count = smap_get_int(&hc->options, "failure_count", 0); >> + if (interval) { >> + ds_put_format(&ctx->output, " Interval : %d\n", >> + interval); >> + } >> + if (timeout) { >> + ds_put_format(&ctx->output, " Timeout : %d\n", >> + timeout); >> + } >> + if (success_count) { >> + ds_put_format(&ctx->output, " Success count : %d\n", >> + success_count); >> + } >> + if (failure_count) { >> + ds_put_format(&ctx->output, " Failure count : %d\n", >> + failure_count); >> + } >> + if (i < lsp->n_health_checks - 1) { >> + ds_put_format(&ctx->output, "\n"); >> + } >> + } >> +} >> + >> static const struct ctl_table_class tables[NBREC_N_TABLES] = { >> [NBREC_TABLE_DHCP_OPTIONS].row_ids >> = {{&nbrec_logical_switch_port_col_name, NULL, >> @@ -9064,6 +9566,16 @@ static const struct ctl_command_syntax nbctl_commands[] = { >> nbctl_pre_static_mac_binding, nbctl_static_mac_binding_list, NULL, >> "", RO }, >> >> + /* Health Check commands */ >> + {"lsp-hc-add", 2, INT_MAX, "PORT TYPE SRC_IP [DST_PORT] [ADDRESS]", >> + nbctl_pre_lsp_health_check_add, nbctl_lsp_health_check_add, >> + NULL, "", RW }, >> + {"lsp-hc-del", 1, INT_MAX, "PORT [HC_UUID]", >> + nbctl_pre_lsp_health_check_del, nbctl_lsp_health_check_del, >> + NULL, "", RW }, >> + {"lsp-hc-list", 1, 1, "PORT", nbctl_pre_lsp_health_check_list, >> + nbctl_lsp_health_check_list, NULL, "", RW }, >> + >> {NULL, 0, 0, NULL, NULL, NULL, NULL, "", RO}, >> }; >> Regards, Dumitru
On 21.01.2026 17:41, Dumitru Ceara wrote: > Внимание: ВНЕШНИЙ отправитель! > > > Будьте осторожны с вложениями и ссылками. > > > On 12/19/25 10:48 AM, Alexandra Rukomoinikova wrote: >> 1) Added tables for further implementation logic of service monitors for >> logical switch ports: >> >> New table: >> - Logical_Switch_Port_Health_Check: Health check configuration >> for logical switch port. >> >> Modified tables: >> - Logical_Switch_Port: Add 'health_checks' column referencing >> health checks configuration. >> >> 2) Added commands to create, delete, and describe health checks for logical switch ports. >> >> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> >> --- > Hi Alexandra, > > I started reviewing this series but before doing that I want to double > check the use case. > > Is your plan to use these to check the availability of > Logical_Switch_Ports as backends of NB.Load_Balancer? > > If so, how do you plan to add that mapping? I guess it would be a new > feature in 26.09. > > Or is your goal to just expose the LSPHC.status to the CMS and then let > the CMS update the list of backends of the LB? > > In any case we probably should: > - update the documentation expanding on the use case more > - update the commit log of this patch (and maybe the others too) > - add a TODO.rst item for any follow up work we might be doing in 26.09. > > I have some more comments on the code changes themselves but I'll send > those replies separately when I'm done going through the code. > > Thanks, > Dumitru > Hi, we would like to use this functionality so that CMS can know about the availability of the virtual machine, that is, yes, it is assumed that the only client of this functionality is CMS - I can indicate this in the documentation. Which option would you prefer? Could I correct all your comments by tomorrow and have it included in the release, or should I leave it all for 26.09 ?
On 1/22/26 2:04 PM, Rukomoinikova Aleksandra wrote: > On 21.01.2026 17:41, Dumitru Ceara wrote: >> Внимание: ВНЕШНИЙ отправитель! >> >> >> Будьте осторожны с вложениями и ссылками. >> >> >> On 12/19/25 10:48 AM, Alexandra Rukomoinikova wrote: >>> 1) Added tables for further implementation logic of service monitors for >>> logical switch ports: >>> >>> New table: >>> - Logical_Switch_Port_Health_Check: Health check configuration >>> for logical switch port. >>> >>> Modified tables: >>> - Logical_Switch_Port: Add 'health_checks' column referencing >>> health checks configuration. >>> >>> 2) Added commands to create, delete, and describe health checks for logical switch ports. >>> >>> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> >>> --- >> Hi Alexandra, >> >> I started reviewing this series but before doing that I want to double >> check the use case. >> >> Is your plan to use these to check the availability of >> Logical_Switch_Ports as backends of NB.Load_Balancer? >> >> If so, how do you plan to add that mapping? I guess it would be a new >> feature in 26.09. >> >> Or is your goal to just expose the LSPHC.status to the CMS and then let >> the CMS update the list of backends of the LB? >> >> In any case we probably should: >> - update the documentation expanding on the use case more >> - update the commit log of this patch (and maybe the others too) >> - add a TODO.rst item for any follow up work we might be doing in 26.09. >> >> I have some more comments on the code changes themselves but I'll send >> those replies separately when I'm done going through the code. >> >> Thanks, >> Dumitru >> > Hi, we would like to use this functionality so that CMS can know about > the availability of the virtual machine, that is, yes, it is assumed > that the only client of this functionality is CMS - I can indicate this > in the documentation. Which option would you prefer? Could I correct > all your comments by tomorrow and have it included in the release, or > should I leave it all for 26.09 ? > Hi Alexandra, I think if it's properly documented that the CMS monitors the health check status the series still qualifies for 26.03. So we can try to include it there. In my review I didn't spot any huge blockers until now. Regards, Dumitru
On 22.01.2026 17:23, Dumitru Ceara wrote: I think if it's properly documented that the CMS monitors the health check status the series still qualifies for 26.03. So we can try to include it there. In my review I didn't spot any huge blockers until now. Thanks) Then I'll fix comments and send new v3) -- regards, Alexandra.
On 22.01.2026 17:23, Dumitru Ceara wrote: > On 1/22/26 2:04 PM, Rukomoinikova Aleksandra wrote: >> On 21.01.2026 17:41, Dumitru Ceara wrote: >>> Внимание: ВНЕШНИЙ отправитель! >>> >>> >>> Будьте осторожны с вложениями и ссылками. >>> >>> >>> On 12/19/25 10:48 AM, Alexandra Rukomoinikova wrote: >>>> 1) Added tables for further implementation logic of service monitors for >>>> logical switch ports: >>>> >>>> New table: >>>> - Logical_Switch_Port_Health_Check: Health check configuration >>>> for logical switch port. >>>> >>>> Modified tables: >>>> - Logical_Switch_Port: Add 'health_checks' column referencing >>>> health checks configuration. >>>> >>>> 2) Added commands to create, delete, and describe health checks for logical switch ports. >>>> >>>> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> >>>> --- >>> Hi Alexandra, >>> >>> I started reviewing this series but before doing that I want to double >>> check the use case. >>> >>> Is your plan to use these to check the availability of >>> Logical_Switch_Ports as backends of NB.Load_Balancer? >>> >>> If so, how do you plan to add that mapping? I guess it would be a new >>> feature in 26.09. >>> >>> Or is your goal to just expose the LSPHC.status to the CMS and then let >>> the CMS update the list of backends of the LB? >>> >>> In any case we probably should: >>> - update the documentation expanding on the use case more >>> - update the commit log of this patch (and maybe the others too) >>> - add a TODO.rst item for any follow up work we might be doing in 26.09. >>> >>> I have some more comments on the code changes themselves but I'll send >>> those replies separately when I'm done going through the code. >>> >>> Thanks, >>> Dumitru >>> >> Hi, we would like to use this functionality so that CMS can know about >> the availability of the virtual machine, that is, yes, it is assumed >> that the only client of this functionality is CMS - I can indicate this >> in the documentation. Which option would you prefer? Could I correct >> all your comments by tomorrow and have it included in the release, or >> should I leave it all for 26.09 ? >> > Hi Alexandra, > > I think if it's properly documented that the CMS monitors the health > check status the series still qualifies for 26.03. So we can try to > include it there. > > In my review I didn't spot any huge blockers until now. > > Regards, > Dumitru > Hi Dumitru and Mark, I read comments you and Mark sent me, and I don't think we should include this patch series in the release. I won't have time to properly fix everything that needed fixing today, and I don't want to break anything again or cut corners =) Thanks for the review, and I'm sorry you had to waste time on this now, but I think it would be better if I calmly improve everything. Thanks again! >
On 1/23/26 10:39 AM, Rukomoinikova Aleksandra wrote: > On 22.01.2026 17:23, Dumitru Ceara wrote: >> On 1/22/26 2:04 PM, Rukomoinikova Aleksandra wrote: >>> On 21.01.2026 17:41, Dumitru Ceara wrote: >>>> Внимание: ВНЕШНИЙ отправитель! >>>> >>>> >>>> Будьте осторожны с вложениями и ссылками. >>>> >>>> >>>> On 12/19/25 10:48 AM, Alexandra Rukomoinikova wrote: >>>>> 1) Added tables for further implementation logic of service monitors for >>>>> logical switch ports: >>>>> >>>>> New table: >>>>> - Logical_Switch_Port_Health_Check: Health check configuration >>>>> for logical switch port. >>>>> >>>>> Modified tables: >>>>> - Logical_Switch_Port: Add 'health_checks' column referencing >>>>> health checks configuration. >>>>> >>>>> 2) Added commands to create, delete, and describe health checks for logical switch ports. >>>>> >>>>> Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> >>>>> --- >>>> Hi Alexandra, >>>> >>>> I started reviewing this series but before doing that I want to double >>>> check the use case. >>>> >>>> Is your plan to use these to check the availability of >>>> Logical_Switch_Ports as backends of NB.Load_Balancer? >>>> >>>> If so, how do you plan to add that mapping? I guess it would be a new >>>> feature in 26.09. >>>> >>>> Or is your goal to just expose the LSPHC.status to the CMS and then let >>>> the CMS update the list of backends of the LB? >>>> >>>> In any case we probably should: >>>> - update the documentation expanding on the use case more >>>> - update the commit log of this patch (and maybe the others too) >>>> - add a TODO.rst item for any follow up work we might be doing in 26.09. >>>> >>>> I have some more comments on the code changes themselves but I'll send >>>> those replies separately when I'm done going through the code. >>>> >>>> Thanks, >>>> Dumitru >>>> >>> Hi, we would like to use this functionality so that CMS can know about >>> the availability of the virtual machine, that is, yes, it is assumed >>> that the only client of this functionality is CMS - I can indicate this >>> in the documentation. Which option would you prefer? Could I correct >>> all your comments by tomorrow and have it included in the release, or >>> should I leave it all for 26.09 ? >>> >> Hi Alexandra, >> >> I think if it's properly documented that the CMS monitors the health >> check status the series still qualifies for 26.03. So we can try to >> include it there. >> >> In my review I didn't spot any huge blockers until now. >> >> Regards, >> Dumitru >> > > Hi Dumitru and Mark, > Hi Alexandra, > I read comments you and Mark sent me, and I don't think we should > include this patch series in the release. I won't have time to properly > fix everything that needed fixing today, and I don't want to break > anything again or cut corners =) Thanks for the review, and I'm sorry > you had to waste time on this now, but I think it would be better if I > calmly improve everything. > Thanks for letting us know. I do want to make sure we're on the same page though. Soft freeze for 26.03 is scheduled for today. But that doesn't mean that all feature patches must be _merged_ before soft freeze. We have 4 more weeks (with soft freeze in effect) until we branch. In this period, patches for features that qualified for the release [0] can be worked on (posting new versions addressing comments) and may be merged: " 1. "Soft freeze" of the main branch. During the freeze, we ask committers to refrain from applying patches that add new features unless those patches were already posted for public review and had received public review feedback on the mailing list before the freeze began. Bug fixes are welcome at any time. Please propose and discuss exceptions on ovs-dev. " As your series has already been reviewed and no major blockers were raised, it means it _qualifies_ for 26.03.0. So you would have 4 more weeks to post new versions, receive review feedback on the new versions, address the feedback, get the patches merged into the main branch. At that point, after 4 weeks (scheduled for February 20th), we enter the "hard freeze" period, i.e., we will create branch-26.03 based on the main branch at that point, no new features are accepted on the new branch-26.03 anymore. Not even the ones that originally qualified for 26.03 but didn't get merged. New features may be merged on the main branch from this point on. Exceptions are possible though. So, TL/DR, you don't need to rush into sending a v2 now, in theory you have 4 weeks time. But the sooner the better of course. :) Regards, DUmitru [0] https://github.com/ovn-org/ovn/blob/2ddbb21ac699349c4a6122cd7bbef291a7d69ac2/Documentation/internals/release-process.rst#release-strategy > Thanks again! > >> >
diff --git a/lib/ovn-util.c b/lib/ovn-util.c index cec029e42..16f7eb2b9 100644 --- a/lib/ovn-util.c +++ b/lib/ovn-util.c @@ -1704,3 +1704,20 @@ is_partial_uuid_match(const struct uuid *uuid, const char *match) s2 = strip_leading_zero(s2); return !strncmp(s1, s2, strlen(s2)); } + +char * +skip_mac_address_from_lsp_address(const char *address_with_mac) +{ + if (!address_with_mac) { + return NULL; + } + const char *ptr = address_with_mac; + while (*ptr == ' ') { + ptr++; + } + ptr += (char) 17; + while (*ptr == ' ') { + ptr++; + } + return (char *) ptr; +} diff --git a/lib/ovn-util.h b/lib/ovn-util.h index daff01995..7685118e6 100644 --- a/lib/ovn-util.h +++ b/lib/ovn-util.h @@ -775,4 +775,6 @@ NEIGH_REDISTRIBUTE_MODES enum neigh_redistribute_mode parse_neigh_dynamic_redistribute(const struct smap *options); +char *skip_mac_address_from_lsp_address(const char *address_with_mac); + #endif /* OVN_UTIL_H */ diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema index 8c2c1d861..5373e6f59 100644 --- a/ovn-nb.ovsschema +++ b/ovn-nb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Northbound", - "version": "7.15.0", - "cksum": "4060410729 43708", + "version": "7.16.0", + "cksum": "3363473672 44948", "tables": { "NB_Global": { "columns": { @@ -179,6 +179,11 @@ "refType": "strong"}, "min": 0, "max": 1}}, + "health_checks": {"type": {"key": {"type": "uuid", + "refTable": "Logical_Switch_Port_Health_Check", + "refType": "weak"}, + "min": 0, + "max": "unlimited"}}, "external_ids": { "type": {"key": "string", "value": "string", "min": 0, "max": "unlimited"}}}, @@ -246,6 +251,25 @@ "min": 0, "max": "unlimited"}}}, "indexes": [["name"], ["id"]], "isRoot": true}, + "Logical_Switch_Port_Health_Check": { + "columns": { + "protocol": { + "type": {"key": {"type": "string", + "enum": ["set", ["tcp", "udp", "icmp"]]}, + "min": 0, "max": 1}}, + "src_ip": {"type": "string"}, + "port": {"type": {"key": {"type": "integer", + "minInteger": 0, + "maxInteger": 65535}}}, + "addresses": {"type": {"key": "string", + "min": 0, + "max": "unlimited"}}, + "options": { + "type": {"key": "string", + "value": "string", + "min": 0, + "max": "unlimited"}}}, + "isRoot": true}, "Forwarding_Group": { "columns": { "name": {"type": "string"}, diff --git a/ovn-nb.xml b/ovn-nb.xml index 730293e97..3cc46e60c 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -2115,6 +2115,11 @@ Please see the <ref table="Mirror"/> table. </column> + <column name="health_checks"> + Health checks configuration for this logical switch port. + Please see the <ref table="Logical_Switch_Port_Health_Check"/> table. + </column> + <column name="ha_chassis_group"> References a row in the OVN Northbound database's <ref table="HA_Chassis_Group" db="OVN_Northbound"/> table. @@ -2171,6 +2176,55 @@ </group> </table> + <table name="Logical_Switch_Port_Health_Check" + title="logical switch port health check"> + <p> + Each row represents health check configuration for logical switch port. + Health checks are used to monitor reachability and health of backend + endpoints associated with logical switch port. Health check can be + configured to use different protocols (TCP, UDP, or ICMP) to verify + availability of target IP addresses. + </p> + + <column name="protocol"> + Valid protocols are <code>tcp</code>, <code>udp</code>, or + <code>icmp</code>. For <code>tcp</code> and <code>udp</code> + protocols - destination port must be specified. + </column> + + <column name="src_ip"> + Source IP address used when sending health check probes. + </column> + + <column name="port"> + Destination port number for TCP and UDP health checks. + </column> + + <column name="addresses"> + A set of IP addresses to monitor. If no specific addresses are provided, + health check will use all addresses configured on logical switch port. + </column> + + <column name="options" key="interval" type='{"type": "integer"}'> + The interval, in seconds, between service monitor checks. + </column> + + <column name="options" key="timeout" type='{"type": "integer"}'> + The time, in seconds, after which the service monitor check times + out. + </column> + + <column name="options" key="success_count" type='{"type": "integer"}'> + The number of successful checks after which the service is + considered <code>online</code>. + </column> + + <column name="options" key="failure_count" type='{"type": "integer"}'> + The number of failure checks after which the service is considered + <code>offline</code>. + </column> + </table> + <table name="Forwarding_Group" title="forwarding group"> <p> Each row represents one forwarding group. diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index dccf30758..62b89945b 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -3499,3 +3499,189 @@ AT_CHECK([ovn-nbctl --may-exist lsp-add-localnet-port ls ln_port net1], [1], [], check ovn-nbctl lsp-set-options ln_port network_name=net1 check ovn-nbctl --may-exist lsp-add-localnet-port ls ln_port net1 ]) + +dnl --------------------------------------------------------------------- + +AT_SETUP([ovn-nbctl - Logical Switch Port Health Check]) +OVN_NBCTL_TEST_START daemon + +# Create logical switch port +AT_CHECK([ovn-nbctl ls-add ls0]) +AT_CHECK([ovn-nbctl lsp-add ls0 lport0]) +AT_CHECK([ovn-nbctl lsp-set-addresses lport0 "00:11:22:33:44:55 192.168.1.10" "00:11:22:33:44:56 192.168.1.11"]) + +# Create icmp health check with the specified addresses +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 192.168.0.255 192.168.1.10 192.168.1.11]) +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl +Logical Switch Port lport0: + Protocol : icmp + Source IP : 192.168.0.255 + Addresses : 192.168.1.10, 192.168.1.11 +]) + +# Check hc attaching to logical switch port +hc_icmp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="192.168.0.255") +check_column "$hc_icmp_uuid" nb:logical_switch_port health_checks + +# Create tcp health check with the specified addresses +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.2 80 192.168.1.10]) +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl +Logical Switch Port lport0: + Protocol : tcp + Source IP : 10.0.0.2 + Port : 80 + Addresses : 192.168.1.10 + + Protocol : icmp + Source IP : 192.168.0.255 + Addresses : 192.168.1.10, 192.168.1.11 +]) +hc_tcp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="10.0.0.2") + +# Create udp health check with the specified addresses +AT_CHECK([ovn-nbctl lsp-hc-add lport0 udp 10.0.0.3 53 192.168.1.11]) +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl +Logical Switch Port lport0: + Protocol : tcp + Source IP : 10.0.0.2 + Port : 80 + Addresses : 192.168.1.10 + + Protocol : udp + Source IP : 10.0.0.3 + Port : 53 + Addresses : 192.168.1.11 + + Protocol : icmp + Source IP : 192.168.0.255 + Addresses : 192.168.1.10, 192.168.1.11 +]) +hc_udp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="10.0.0.3") + +check_column "$hc_icmp_uuid $hc_tcp_uuid $hc_udp_uuid" nb:logical_switch_port health_checks + +AT_CHECK([ovn-nbctl lsp-hc-del lport0 $hc_icmp_uuid]) +# Check hcs detaching to logical switch port +check_column "$hc_tcp_uuid $hc_udp_uuid" nb:logical_switch_port health_checks + +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl +Logical Switch Port lport0: + Protocol : tcp + Source IP : 10.0.0.2 + Port : 80 + Addresses : 192.168.1.10 + + Protocol : udp + Source IP : 10.0.0.3 + Port : 53 + Addresses : 192.168.1.11 +]) + +AT_CHECK([ovn-nbctl lsp-hc-del lport0]) +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl]) + +# Create health check without specified addresses +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.4]) +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl +Logical Switch Port lport0: + Protocol : icmp + Source IP : 10.0.0.4 +]) + +# Check invalid protocol +AT_CHECK([ovn-nbctl lsp-hc-add lport0 stcp 10.0.0.1], [1], [], [stderr]) +AT_CHECK([grep "Type must be icmp, tcp or udp" stderr], [0], [ignore]) + +# Check invalid source IP +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp invalid_ip], [1], [], [stderr]) +AT_CHECK([grep "Not a valid IPv4 or IPv6 address" stderr], [0], [ignore]) + +# Check TCP/UDP without destination port +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1], [1], [], [stderr]) +AT_CHECK([grep "Destination port required for tcp health check" stderr], [0], [ignore]) + +# Check invalid destination port +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 70000], [1], [], [stderr]) +AT_CHECK([grep "port must in range 0...65535" stderr], [0], [ignore]) + +# Check IP address not configured on port +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.1 192.168.99.99], [1], [], [stderr]) +AT_CHECK([grep "Address 192.168.99.99 not configured on port" stderr], [0], [ignore]) + +# Check Duplicate health check configuration +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.4], [1], [], [stderr]) +AT_CHECK([grep "Health check already exists" stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.1 192.168.1.10]) +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.1 192.168.1.10], [1], [], [stderr]) +AT_CHECK([grep "Health check already exists" stderr], [0], [ignore]) + +# Check same protocol with different ports - it's ok +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 90 192.168.1.10]) +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 443 192.168.1.10], [0], [], [ignore]) + +# Check different protocol with same ports - it's ok +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 80 192.168.1.10]) +AT_CHECK([ovn-nbctl lsp-hc-add lport0 udp 10.0.0.1 80 192.168.1.10], [0], [], [ignore]) + +# Check different source ip with same ports and addresses - it's ok +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 92 192.168.1.10]) +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.2 92 192.168.1.10], [0], [], [ignore]) + +# Check multipal same addresses +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 93 192.168.1.10 192.168.1.11]) +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 93 192.168.1.10 192.168.1.11], [1], [], [stderr]) +AT_CHECK([grep "Health check already exists" stderr], [0], [ignore]) + +# Check supported logical switch port type for monitoring +AT_CHECK([ovn-nbctl lsp-add ls0 lport1]) +AT_CHECK([ovn-nbctl lsp-set-type lport1 router]) +AT_CHECK([ovn-nbctl lsp-hc-add lport1 icmp 10.0.0.1], [1], [], [stderr]) +AT_CHECK([grep "Health check monitoring supported only for port with vif type" stderr], [0], [ignore]) + +AT_CHECK([ovn-nbctl lsp-hc-del lport0]) + +# Test IPv6 addresses support +AT_CHECK([ovn-nbctl lsp-add ls0 lport3]) +AT_CHECK([ovn-nbctl lsp-set-addresses lport3 "00:11:22:33:44:57 2001:db8::1"]) +AT_CHECK([ovn-nbctl lsp-hc-add lport3 icmp 2001:db8::2 2001:db8::1]) +AT_CHECK([ovn-nbctl lsp-hc-list lport3], [0], [dnl +Logical Switch Port lport3: + Protocol : icmp + Source IP : 2001:db8::2 + Addresses : 2001:db8::1 +]) + +# Check options printing +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.4 192.168.1.10 192.168.1.11]) +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl +Logical Switch Port lport0: + Protocol : icmp + Source IP : 10.0.0.4 + Addresses : 192.168.1.10, 192.168.1.11 +]) +hc_icmp_uuid=$(fetch_column nb:logical_switch_port_health_check _uuid src_ip="10.0.0.4") + +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:interval=3]) +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:timeout=30]) +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:success_count=1]) +AT_CHECK([ovn-nbctl set logical_switch_port_health_check $hc_icmp_uuid options:failure_count=2]) + +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl +Logical Switch Port lport0: + Protocol : icmp + Source IP : 10.0.0.4 + Addresses : 192.168.1.10, 192.168.1.11 + Interval : 3 + Timeout : 30 + Success count : 1 + Failure count : 2 +]) + +AT_CHECK([ovn-nbctl lsp-hc-del lport0]) +AT_CHECK([ovn-nbctl lsp-hc-del lport3]) +check_row_count nb:Logical_Switch_Port_Health_Check 0 + +OVN_NBCTL_TEST_STOP "/terminating with signal 15/d" +AT_CLEANUP + diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml index 7df902944..efc781d77 100644 --- a/utilities/ovn-nbctl.8.xml +++ b/utilities/ovn-nbctl.8.xml @@ -1822,6 +1822,60 @@ </dd> </dl> + <h2>Health Check commands</h2> + <dl> + <dt><code>lsp-hc-add</code> <var>PORT</var> <var>PROTOCOL</var> + <var>SOURCE_IP</var> [<var>DST_PORT</var>] [<var>ADDRESS</var>]...</dt> + <dd> + <p> + Creates a new health check configuration for the logical switch port + <code>PORT</code> with the below mandatory arguments. + </p> + + <p> + <var>PROTOCOL</var> specifies the health check protocol - + <code>tcp</code>, <code>udp</code>, or <code>icmp</code>. + </p> + + <p> + <var>SOURCE_IP</var> specifies the source IP address used when + sending health check probes. + </p> + + <p> + <var>DST_PORT</var> specifies the destination port number for + <code>tcp</code> and <code>udp</code> health checks. This parameter + is ignored for <code>icmp</code> protocol. + </p> + + <p> + <var>ADDRESS</var> specifies one or more IP addresses to monitor. + If no addresses are provided, the health check will monitor all IP + addresses configured on the logical switch port. + </p> + + <p> + Additional health check options such as interval, timeout, + success count, and failure count can be configured separately. + </p> + </dd> + + <dt><code>lsp-hc-del</code> <var>PORT</var> [<var>HC_UUID</var>]</dt> + <dd> + <p> + Deletes health check configuration for the logical switch port + <code>PORT</code>. + </p> + </dd> + + <dt><code>lsp-hc-list</code> <var>PORT</var></dt> + <dd> + Lists all health check configurations for the logical switch port + <code>PORT</code>, including protocol, source IP, destination port, + monitored addresses, and current status. + </dd> + </dl> + <h2>Synchronization Commands</h2> <dl> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index cdf6b578a..b8802ab30 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -66,6 +66,18 @@ string_ptr(char *ptr) return (ptr) ? ptr : s; } +static char *OVS_WARN_UNUSED_RESULT +parse_l4_port_range(const char *arg, int64_t *port_p) +{ + int64_t port; + if (!ovs_scan(arg, "%"SCNd64, &port) + || port < 0 || port > UINT16_MAX) { + return xasprintf("%s: port must in range 0...65535", arg); + } + *port_p = port; + return NULL; +} + static void nbctl_add_base_prerequisites(struct ovsdb_idl *idl, enum nbctl_wait_type wait_type) @@ -544,6 +556,12 @@ MAC_Binding commands:\n\ Delete Static_MAC_Binding entry\n\ static-mac-binding-list List all Static_MAC_Binding entries\n\ \n\ +Logical Switch Port Health Check:\n\ + lsp-hc-add PORT PROTOCOL SOURCE_IP DST_PORT [ADDRESS]...\n\ + add health check monitoring for PORT\n\ + lsp-hc-del PORT HC_UUID delete health check monitoring for PORT\n\ + lsp-hc-list PORT print health check for PORT\n\ +\n\ %s\ %s\ \n\ @@ -8677,6 +8695,490 @@ nbctl_lsp_add_misc_port(struct ctl_context *ctx) shash_add(&nbctx->lsp_to_ls_map, lsp_name, ls); } +/* Logical Switch Port Health Check Functions. */ +enum health_check_protocol { + LSP_ICMP_HEALTH_CHECK, + LSP_TCP_HEALTH_CHECK, + LSP_UDP_HEALTH_CHECK, +}; + +static bool +parse_health_check_protocol(struct ctl_context *ctx, const char *type, + enum health_check_protocol *protocol) +{ + if (!strcmp(type, "icmp")) { + *protocol = LSP_ICMP_HEALTH_CHECK; + return true; + } else if (!strcmp(type, "tcp")) { + *protocol = LSP_TCP_HEALTH_CHECK; + return true; + } else if (!strcmp(type, "udp")) { + *protocol = LSP_UDP_HEALTH_CHECK; + return true; + } else { + ctl_error(ctx, "%s: Type must be icmp, tcp or udp", type); + return false; + } +} + +static void +lsp_health_check_get_duplicate( + int proto, const char *src_ip, + int64_t port, struct sset *addresses, + const struct nbrec_logical_switch_port *lsp, + const struct nbrec_logical_switch_port_health_check **lsp_hc) +{ + *lsp_hc = NULL; + + for (size_t i = 0; i < lsp->n_health_checks; i++) { + const struct nbrec_logical_switch_port_health_check *lsp_hc_p = + lsp->health_checks[i]; + + if (src_ip && strcmp(lsp_hc_p->src_ip, src_ip)) { + continue; + } + + const char *target_proto = + (proto == LSP_ICMP_HEALTH_CHECK) ? "icmp" : + (proto == LSP_TCP_HEALTH_CHECK) ? "tcp" : "udp"; + if (strcmp(lsp_hc_p->protocol, target_proto)) { + continue; + } + + if (proto != LSP_ICMP_HEALTH_CHECK && lsp_hc_p->port != port) { + continue; + } + + if (!addresses || sset_is_empty(addresses)) { + *lsp_hc = lsp_hc_p; + return; + } + + bool addresses_found = true; + for (size_t j = 0; j < lsp_hc_p->n_addresses; j++) { + if (!sset_contains(addresses, lsp_hc_p->addresses[j])) { + addresses_found = false; + break; + } + } + + if (addresses_found) { + *lsp_hc = lsp_hc_p; + return; + } + } +} + +static char ** +_get_lsp_ip_addresses(char **lsp_addresses, + int lsp_n_addresses, + int *lsp_n_ip_addresses) +{ + char **lsp_ip_addresses_p = NULL; + *lsp_n_ip_addresses = 0; + size_t n_capacity = 0; + size_t count = 0; + + for (size_t i = 0; i < lsp_n_addresses; i++) { + char *address_without_mac = + skip_mac_address_from_lsp_address(lsp_addresses[i]); + + if (address_without_mac && address_without_mac[0]) { + if (count == n_capacity) { + lsp_ip_addresses_p = x2nrealloc(lsp_ip_addresses_p, + &n_capacity, + sizeof *lsp_ip_addresses_p); + } + lsp_ip_addresses_p[count] = xstrdup(address_without_mac); + count++; + } + } + + *lsp_n_ip_addresses = count; + return lsp_ip_addresses_p; +} + +static bool +_lsp_contains_ip_address(char **lsp_ip_addresses, + size_t lsp_n_ip_addresses, + char *ip_address) +{ + for (size_t i = 0; i < lsp_n_ip_addresses; i++) { + if (!strcmp(lsp_ip_addresses[i], ip_address)) { + return true; + } + } + + return false; +} + +static char * OVS_WARN_UNUSED_RESULT +find_logical_switch_port_health_check_by_uuid( + struct ctl_context *ctx, + const char *id, + const struct nbrec_logical_switch_port_health_check **lsp_hc_p) +{ + const struct nbrec_logical_switch_port_health_check *lsp_hc; + *lsp_hc_p = NULL; + + struct uuid lsp_hc_uuid; + bool is_uuid = uuid_from_string(&lsp_hc_uuid, id); + + if (!is_uuid) { + return xasprintf("%s: Invalid UUID format", id); + } + + lsp_hc = nbrec_logical_switch_port_health_check_get_for_uuid( + ctx->idl, &lsp_hc_uuid); + + if (!lsp_hc) { + return xasprintf("%s: Logical Switch Port Health Check not found", id); + } + + *lsp_hc_p = lsp_hc; + + return NULL; +} + +static void +nbctl_pre_lsp_health_check_add(struct ctl_context *ctx) +{ + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_col_name); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_col_addresses); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_col_health_checks); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_health_check_col_port); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_health_check_col_protocol); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_health_check_col_src_ip); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_health_check_col_addresses); +} + +static void +nbctl_lsp_health_check_add(struct ctl_context *ctx) +{ + const struct nbrec_logical_switch_port *lsp = NULL; + const struct nbrec_logical_switch_port_health_check *lsp_hc = NULL; + + const char *port = ctx->argv[1]; + const char *type = ctx->argv[2]; + const char *src_ip = ctx->argv[3]; + enum health_check_protocol protocol; + char **lsp_ip_addresses = NULL; + int lsp_n_ip_addresses = 0; + char *validated_src_ip = NULL; + struct sset target_ips; + sset_init(&target_ips); + + char *error; + error = lsp_by_name_or_uuid(ctx, port, true, &lsp); + if (error) { + ctx->error = error; + return; + } + + if (lsp->type[0]) { + ctl_error(ctx, "%s: Health check monitoring supported only for" + " port with vif type", lsp->type); + goto cleanup; + } + + if (!parse_health_check_protocol(ctx, type, &protocol)) { + goto cleanup; + } + + validated_src_ip = normalize_addr_str(src_ip); + if (!validated_src_ip) { + ctl_error(ctx, "%s: Not a valid IPv4 or IPv6 address", + ctx->argv[3]); + goto cleanup; + } + + int64_t destination_port = 0; + if (protocol == LSP_TCP_HEALTH_CHECK || + protocol == LSP_UDP_HEALTH_CHECK) { + if (ctx->argc < 5) { + ctl_error(ctx, "Destination port required for %s health check", + type); + goto cleanup; + } + + error = parse_l4_port_range(ctx->argv[4], &destination_port); + if (error) { + ctx->error = error; + goto cleanup; + } + } + + size_t target_ips_start_index; + if (protocol == LSP_ICMP_HEALTH_CHECK) { + target_ips_start_index = 4; + } else { + target_ips_start_index = 5; + } + + if (ctx->argc > target_ips_start_index) { + lsp_ip_addresses = _get_lsp_ip_addresses(lsp->addresses, + lsp->n_addresses, + &lsp_n_ip_addresses); + + for (int i = target_ips_start_index; i < ctx->argc; i++) { + char *ip_addr = ctx->argv[i]; + + char *validated_ip = normalize_addr_str(ip_addr); + if (!validated_ip) { + free(validated_ip); + ctl_error(ctx, "%s: Not a valid IPv4 or IPv6 address", + ip_addr); + goto cleanup; + } + + if (!_lsp_contains_ip_address(lsp_ip_addresses, + lsp_n_ip_addresses, + ip_addr)) { + free(validated_ip); + ctl_error(ctx, "%s: Address %s not configured on port", + lsp->name, ip_addr); + goto cleanup; + } + + sset_add(&target_ips, validated_ip); + free(validated_ip); + } + } + + lsp_health_check_get_duplicate(protocol, + src_ip, + destination_port, + &target_ips, + lsp, + &lsp_hc); + + if (lsp_hc) { + ctl_error(ctx, "Health check already exists"); + goto cleanup; + } + + lsp_hc = nbrec_logical_switch_port_health_check_insert(ctx->txn); + nbrec_logical_switch_port_health_check_set_protocol(lsp_hc, type); + nbrec_logical_switch_port_health_check_set_src_ip(lsp_hc, src_ip); + nbrec_logical_switch_port_health_check_set_port(lsp_hc, destination_port); + + if (ctx->argc > target_ips_start_index) { + size_t num_target_ips = ctx->argc - target_ips_start_index; + nbrec_logical_switch_port_health_check_set_addresses( + lsp_hc, (const char **) ctx->argv + target_ips_start_index, + num_target_ips); + } else { + nbrec_logical_switch_port_health_check_set_addresses(lsp_hc, NULL, 0); + } + + nbrec_logical_switch_port_update_health_checks_addvalue(lsp, lsp_hc); + +cleanup: + if (lsp_ip_addresses) { + for (size_t i = 0; i < lsp_n_ip_addresses; i++) { + free(lsp_ip_addresses[i]); + } + free(lsp_ip_addresses); + } + sset_destroy(&target_ips); + free(validated_src_ip); +} + +static void +nbctl_pre_lsp_health_check_del(struct ctl_context *ctx) +{ + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_col_name); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_col_health_checks); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_health_check_col_protocol); +} + +static void +nbctl_lsp_health_check_del(struct ctl_context *ctx) +{ + const struct nbrec_logical_switch_port_health_check *lsp_hc = NULL; + const struct nbrec_logical_switch_port *lsp = NULL; + const char *port_id = ctx->argv[1]; + const char *hc_uuid = ctx->argv[2]; + + char *error; + error = lsp_by_name_or_uuid(ctx, port_id, true, &lsp); + if (error) { + ctx->error = error; + return; + } + + if (!lsp) { + ctl_error(ctx, "Logical Switch Port with id %s not found", + port_id); + return; + } + + if (hc_uuid) { + error = find_logical_switch_port_health_check_by_uuid(ctx, + hc_uuid, + &lsp_hc); + if (error) { + ctx->error = error; + return; + } + + nbrec_logical_switch_port_update_health_checks_delvalue(lsp, lsp_hc); + nbrec_logical_switch_port_health_check_delete(lsp_hc); + return; + } + + for (size_t i = 0; i < lsp->n_health_checks; i++) { + nbrec_logical_switch_port_update_health_checks_delvalue( + lsp, lsp->health_checks[i]); + nbrec_logical_switch_port_health_check_delete(lsp->health_checks[i]); + } +} + +static int +cmp_lsp_hc(const void *lsp_hc_1_, const void *lsp_hc_2_) +{ + const struct nbrec_logical_switch_port_health_check *const *lsp_hc_1p = + lsp_hc_1_; + const struct nbrec_logical_switch_port_health_check *const *lsp_hc_2p = + lsp_hc_2_; + const struct nbrec_logical_switch_port_health_check *lsp_hc_1 = + *lsp_hc_1p; + const struct nbrec_logical_switch_port_health_check *lsp_hc_2 = + *lsp_hc_2p; + + int src_ip_cmp = strcmp(lsp_hc_1->src_ip, lsp_hc_2->src_ip); + if (src_ip_cmp) { + return src_ip_cmp; + } + + int protocol_cmp = strcmp(lsp_hc_1->protocol, lsp_hc_2->protocol); + if (protocol_cmp != 0) { + return protocol_cmp; + } + + if (strcmp(lsp_hc_1->protocol, "icmp") != 0) { + if (lsp_hc_1->port != lsp_hc_2->port) { + return lsp_hc_1->port < lsp_hc_2->port ? -1 : 1; + } + } + + if (lsp_hc_1->n_addresses != lsp_hc_2->n_addresses) { + return lsp_hc_1->n_addresses < lsp_hc_2->n_addresses ? -1 : 1; + } + + size_t min_n_addresses = lsp_hc_1->n_addresses < lsp_hc_2->n_addresses ? + lsp_hc_1->n_addresses : lsp_hc_2->n_addresses; + + for (size_t i = 0; i < min_n_addresses; i++) { + int ip_cmp = strcmp(lsp_hc_1->addresses[i], lsp_hc_2->addresses[i]); + if (ip_cmp) { + return ip_cmp; + } + } + + return 0; +} + +static void +nbctl_pre_lsp_health_check_list(struct ctl_context *ctx) +{ + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_col_name); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_col_health_checks); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_health_check_col_port); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_health_check_col_protocol); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_health_check_col_src_ip); + ovsdb_idl_add_column(ctx->idl, + &nbrec_logical_switch_port_health_check_col_addresses); +} + +static void +nbctl_lsp_health_check_list(struct ctl_context *ctx) +{ + const struct nbrec_logical_switch_port_health_check **lsp_hcs; + const struct nbrec_logical_switch_port *lsp = NULL; + const char *port = ctx->argv[1]; + + char *error; + error = lsp_by_name_or_uuid(ctx, port, true, &lsp); + if (error) { + ctx->error = error; + return; + } + + if (!lsp->n_health_checks) { + return; + } + + lsp_hcs = xmalloc(sizeof *lsp_hcs * lsp->n_health_checks); + for (size_t i = 0; i < lsp->n_health_checks; i++) { + lsp_hcs[i] = lsp->health_checks[i]; + } + + qsort(lsp_hcs, lsp->n_health_checks, sizeof *lsp_hcs, cmp_lsp_hc); + + ds_put_format(&ctx->output, "Logical Switch Port %s:\n", port); + for (size_t i = 0; i < lsp->n_health_checks; i++) { + const struct nbrec_logical_switch_port_health_check *hc + = lsp_hcs[i]; + ds_put_format(&ctx->output, " Protocol : %s\n", + hc->protocol); + ds_put_format(&ctx->output, " Source IP : %s\n", + hc->src_ip); + if (strcmp(hc->protocol, "icmp")) { + ds_put_format(&ctx->output, " Port : %"PRId64"\n", + hc->port); + } + if (hc->n_addresses) { + ds_put_format(&ctx->output, " Addresses : "); + for (size_t j = 0; j < hc->n_addresses; j++) { + if (j > 0) { + ds_put_format(&ctx->output, ", "); + } + ds_put_format(&ctx->output, "%s", hc->addresses[j]); + } + ds_put_format(&ctx->output, "\n"); + } + int interval = smap_get_int(&hc->options, "interval", 0); + int timeout = smap_get_int(&hc->options, "timeout", 0); + int success_count = smap_get_int(&hc->options, "success_count", 0); + int failure_count = smap_get_int(&hc->options, "failure_count", 0); + if (interval) { + ds_put_format(&ctx->output, " Interval : %d\n", + interval); + } + if (timeout) { + ds_put_format(&ctx->output, " Timeout : %d\n", + timeout); + } + if (success_count) { + ds_put_format(&ctx->output, " Success count : %d\n", + success_count); + } + if (failure_count) { + ds_put_format(&ctx->output, " Failure count : %d\n", + failure_count); + } + if (i < lsp->n_health_checks - 1) { + ds_put_format(&ctx->output, "\n"); + } + } +} + static const struct ctl_table_class tables[NBREC_N_TABLES] = { [NBREC_TABLE_DHCP_OPTIONS].row_ids = {{&nbrec_logical_switch_port_col_name, NULL, @@ -9064,6 +9566,16 @@ static const struct ctl_command_syntax nbctl_commands[] = { nbctl_pre_static_mac_binding, nbctl_static_mac_binding_list, NULL, "", RO }, + /* Health Check commands */ + {"lsp-hc-add", 2, INT_MAX, "PORT TYPE SRC_IP [DST_PORT] [ADDRESS]", + nbctl_pre_lsp_health_check_add, nbctl_lsp_health_check_add, + NULL, "", RW }, + {"lsp-hc-del", 1, INT_MAX, "PORT [HC_UUID]", + nbctl_pre_lsp_health_check_del, nbctl_lsp_health_check_del, + NULL, "", RW }, + {"lsp-hc-list", 1, 1, "PORT", nbctl_pre_lsp_health_check_list, + nbctl_lsp_health_check_list, NULL, "", RW }, + {NULL, 0, 0, NULL, NULL, NULL, NULL, "", RO}, };
1) Added tables for further implementation logic of service monitors for logical switch ports: New table: - Logical_Switch_Port_Health_Check: Health check configuration for logical switch port. Modified tables: - Logical_Switch_Port: Add 'health_checks' column referencing health checks configuration. 2) Added commands to create, delete, and describe health checks for logical switch ports. Signed-off-by: Alexandra Rukomoinikova <arukomoinikova@k2.cloud> --- lib/ovn-util.c | 17 ++ lib/ovn-util.h | 2 + ovn-nb.ovsschema | 28 ++- ovn-nb.xml | 54 ++++ tests/ovn-nbctl.at | 186 ++++++++++++++ utilities/ovn-nbctl.8.xml | 54 ++++ utilities/ovn-nbctl.c | 512 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 851 insertions(+), 2 deletions(-)