On 11.02.2026 18:15, Dumitru Ceara wrote:
> On 2/7/26 10:48 PM, Alexandra Rukomoinikova wrote:
>> Add schema and CLI support for health checks on logical switch
>> ports by introducing a new health check table and linking it
>> from logical switch ports. Implement corresponding ovn-nbctl
>> commands to manage LSP health checks. Also extend service
>> monitoring to support the logical-switch-port type.
>>
>> Signed-off-by: Alexandra Rukomoinikova <[email protected]>
>> ---
>> v1 --> v2: corrected all the comments and made one service monitor bind to
>> only one address
>> ---
> Hi Alexandra,
>
> At a first glance this seems OK to me, only a few minor comments below.
> I'll review the rest of the patches in the series and come back to this
> one with an Ack if applicable.
>
> Thanks,
> Dumitru
Sorry, I didn't cope with the last letter)
Hi! I started fixing your comments and I had a question. I'll write it below.
>
>> ovn-nb.ovsschema | 28 ++-
>> ovn-nb.xml | 53 ++++
>> ovn-sb.ovsschema | 7 +-
>> ovn-sb.xml | 11 +
>> tests/ovn-nbctl.at | 196 +++++++++++++++
>> utilities/ovn-nbctl.8.xml | 52 ++++
>> utilities/ovn-nbctl.c | 516 ++++++++++++++++++++++++++++++++++++--
>> 7 files changed, 842 insertions(+), 21 deletions(-)
>>
>> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
>> index 8c2c1d861..538b37456 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": "3492890733 44939",
>> "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": "strong"},
> Nit: indentation is weird, "type" and "refTable"/"refType" should be at
> the same level.
>
>> + "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}},
> Same here.
>
>> + "src_ip": {"type": "string"},
>> + "port": {"type": {"key": {"type": "integer",
>> + "minInteger": 0,
>> + "maxInteger": 65535}}},
>> + "address": {"type": {"key": "string",
>> + "min": 0,
>> + "max": 1}},
>> + "options": {
>> + "type": {"key": "string",
>> + "value": "string",
>> + "min": 0,
>> + "max": "unlimited"}}},
>> + "isRoot": false},
>> "Forwarding_Group": {
>> "columns": {
>> "name": {"type": "string"},
>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>> index 1acbf202b..e60edbd8d 100644
>> --- a/ovn-nb.xml
>> +++ b/ovn-nb.xml
>> @@ -2136,6 +2136,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.
>> @@ -2192,6 +2197,54 @@
>> </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="address">
>> + IP address to monitor for the health check.
>> + </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/ovn-sb.ovsschema b/ovn-sb.ovsschema
>> index cf33933da..dd9e32f06 100644
>> --- a/ovn-sb.ovsschema
>> +++ b/ovn-sb.ovsschema
>> @@ -1,7 +1,7 @@
>> {
>> "name": "OVN_Southbound",
>> - "version": "21.7.0",
>> - "cksum": "1383351379 36646",
>> + "version": "21.8.0",
>> + "cksum": "3491605797 36713",
>> "tables": {
>> "SB_Global": {
>> "columns": {
>> @@ -516,7 +516,8 @@
>> "type": {"type": {"key": {
>> "type": "string",
>> "enum": ["set", ["load-balancer",
>> - "network-function"]]},
>> + "network-function",
>> + "logical-switch-port"]]},
>> "min": 0, "max": 1}},
>> "ip": {"type": "string"},
>> "mac": {"type": "string"},
>> diff --git a/ovn-sb.xml b/ovn-sb.xml
>> index 00bae26bf..0ec7d6094 100644
>> --- a/ovn-sb.xml
>> +++ b/ovn-sb.xml
>> @@ -5042,6 +5042,8 @@ tcp.flags = RST;
>> icmp, and the health probe is done by injecting an icmp echo
>> request
>> packet into the <code>inport</code> of the Network_Function and
>> then
>> montoring the same packet coming out of the <code>outport</code>.
>> + For <code>type</code> "logical-switch-port", supported protocols are
>> + <code>tcp</code>, <code>udp</code> and <code>icmp</code>.
>> </column>
>>
>> <column name="port">
>> @@ -5124,6 +5126,15 @@ tcp.flags = RST;
>> the service and doesn't expect any reply. If it receives an ICMP
>> reply, then it considers the service to be <code>offline</code>.
>> </p>
>> +
>> + <p>
>> + For ICMP service, <code>ovn-controller</code> sends an ICMP
>> + Echo Request and expects an ICMP Echo Reply to consider service
>> + to be <code>online</code>. If no reply is received or if an ICMP
>> + error message is received, the service is considered
>> + <code>offline</code>.
>> + </p>
>> +
>> </column>
>> </group>
>>
>> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
>> index dccf30758..177e097fb 100644
>> --- a/tests/ovn-nbctl.at
>> +++ b/tests/ovn-nbctl.at
>> @@ -3499,3 +3499,199 @@ 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
> Nit: missing period at end of sentence (this applies to most of the
> comments in this patch).
>
>> +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 IPV4 ICMP health check
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 192.168.0.255 192.168.1.10])
>> +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
>> +])
>> +
>> +# 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 IPv4 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
>> +])
>> +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
>> +])
>> +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])
>> +check_row_count nb:Logical_Switch_Port_Health_Check 0
>> +AT_CHECK([ovn-nbctl lsp-hc-list lport0], [0], [dnl])
>> +
>> +# Check invalid protocol
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 stcp 10.0.0.1 50], [1], [], [stderr])
>> +AT_CHECK([grep "Type must be icmp, tcp or udp" stderr], [0], [ignore])
> Nit: in the docs we used Oxford-comma, I guess we can do that here too
> (and in nbctl.c).
>
>> +
>> +# Check invalid source IP
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp invalid_ip invalid_ip], [1], [],
>> [stderr])
>> +AT_CHECK([grep "Not a valid IPv4 or IPv6 address" stderr], [0], [ignore])
>> +
>> +# Check TCP/UDP non-valid 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])
>> +
>> +AT_CHECK([ovn-nbctl lsp-hc-del lport0])
>> +
>> +# Different source IP, same protocol, port and address - should be allowed
>> +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 tcp 10.0.0.2 80 192.168.1.10], [0],
>> [], [ignore])
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.3 80 192.168.1.10], [0],
>> [], [ignore])
>> +
>> +# Same source IP, different ports, same protocol and address - should be
>> allowed
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 81 192.168.1.10], [0],
>> [], [ignore])
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.1 82 192.168.1.10], [0],
>> [], [ignore])
>> +
>> +# Same source IP and port, different protocols, same address - should be
>> allowed
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 udp 10.0.0.1 80 192.168.1.10], [0],
>> [], [ignore])
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.1 192.168.1.10], [0], [],
>> [ignore])
>> +
>> +# Check duplication
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.5 90 192.168.1.10])
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 tcp 10.0.0.5 90 192.168.1.10], [1],
>> [], [stderr])
>> +AT_CHECK([grep "Health check with address 192.168.1.10 already exists on
>> port lport0" stderr], [0], [ignore])
>> +
>> +# ICMP - same source IP, different addresses - should be allowed
>> +AT_CHECK([ovn-nbctl lsp-hc-del lport0])
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.9 192.168.1.10])
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.9 192.168.1.11], [0], [],
>> [ignore])
>> +
>> +# ICMP - different source IP, same address - should be allowed
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport0 icmp 10.0.0.10 192.168.1.10], [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 10.0.0.2], [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: icmp, tcp, udp
>> +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-add lport3 tcp 2001:db8::2 80 2001:db8::1])
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport3 udp 2001:db8::2 53 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
>> +
>> + Protocol : tcp
>> + Source IP : 2001:db8::2
>> + Port : 80
>> + Addresses : 2001:db8::1
>> +
>> + Protocol : udp
>> + Source IP : 2001:db8::2
>> + Port : 53
>> + Addresses : 2001:db8::1
>> +])
>> +
>> +AT_CHECK([ovn-nbctl lsp-hc-del lport3])
>> +
>> +# Duplicate IPv6 health check - should be disabled
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport3 icmp 2001:db8::2 2001:db8::1])
>> +AT_CHECK([ovn-nbctl lsp-hc-add lport3 icmp 2001:db8::2 2001:db8::1], [1],
>> [], [stderr])
>> +AT_CHECK([grep "Health check with address 2001:db8::1 already exists on
>> port lport3" stderr], [0], [ignore])
>> +
>> +# 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
>> +])
>> +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
>> + 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..c403fcfb6 100644
>> --- a/utilities/ovn-nbctl.8.xml
>> +++ b/utilities/ovn-nbctl.8.xml
>> @@ -1822,6 +1822,58 @@
>> </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 the IP address to monitor.
>> + </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..e58001148 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, uint16_t *port_p)
> I'd call this parse_l4_port().
>
>> +{
>> + 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\
>> @@ -1464,18 +1482,23 @@ nbctl_pre_lsp_set_addresses(struct ctl_context *ctx)
>> &nbrec_logical_switch_port_col_dynamic_addresses);
>> }
>>
>> -static char *
>> +static bool
>> lsp_contains_duplicate_ip(struct lport_addresses *laddrs1,
>> struct lport_addresses *laddrs2,
>> - const struct nbrec_logical_switch_port *lsp_test)
>> + const struct nbrec_logical_switch_port *lsp_test,
>> + char **error_str)
>> {
>> for (size_t i = 0; i < laddrs1->n_ipv4_addrs; i++) {
>> for (size_t j = 0; j < laddrs2->n_ipv4_addrs; j++) {
>> if (laddrs1->ipv4_addrs[i].addr ==
>> laddrs2->ipv4_addrs[j].addr) {
>> - return xasprintf("duplicate IPv4 address '%s' found on "
>> - "logical switch port '%s'",
>> - laddrs1->ipv4_addrs[i].addr_s,
>> - lsp_test->name);
>> + if (error_str) {
>> + *error_str = xasprintf("duplicate IPv4 address '%s' "
>> + "found on logical switch "
>> + "port '%s'",
>> + laddrs1->ipv4_addrs[i].addr_s,
>> + lsp_test->name);
>> + }
>> + return true;
>> }
>> }
>> }
>> @@ -1484,15 +1507,23 @@ lsp_contains_duplicate_ip(struct lport_addresses
>> *laddrs1,
>> for (size_t j = 0; j < laddrs2->n_ipv6_addrs; j++) {
>> if (IN6_ARE_ADDR_EQUAL(&laddrs1->ipv6_addrs[i].addr,
>> &laddrs2->ipv6_addrs[j].addr)) {
>> - return xasprintf("duplicate IPv6 address '%s' found on "
>> - "logical switch port '%s'",
>> - laddrs1->ipv6_addrs[i].addr_s,
>> - lsp_test->name);
>> + if (error_str) {
>> + *error_str = xasprintf("duplicate IPv6 address "
>> + "'%s' found on logical "
>> + "switch port '%s'",
>> + laddrs1->ipv6_addrs[i].addr_s,
>> + lsp_test->name);
>> + }
>> + return true;
>> }
>> }
>> }
>>
>> - return NULL;
>> + if (error_str) {
>> + *error_str = NULL;
>> + }
>> +
>> + return false;
>> }
>>
>> static char *
>> @@ -1500,12 +1531,13 @@ lsp_contains_duplicates(const struct
>> nbrec_logical_switch *ls,
>> const struct nbrec_logical_switch_port *lsp,
>> const char *address)
>> {
>> + char *error = NULL;
>> + char *sub_error = NULL;
>> struct lport_addresses laddrs;
>> if (!extract_lsp_addresses(address, &laddrs)) {
>> return NULL;
>> }
>>
>> - char *sub_error = NULL;
>> for (size_t i = 0; i < ls->n_ports; i++) {
>> struct nbrec_logical_switch_port *lsp_test = ls->ports[i];
>> if (lsp_test == lsp) {
>> @@ -1518,10 +1550,11 @@ lsp_contains_duplicates(const struct
>> nbrec_logical_switch *ls,
>> addr = lsp_test->dynamic_addresses;
>> }
>> if (extract_lsp_addresses(addr, &laddrs_test)) {
>> - sub_error = lsp_contains_duplicate_ip(&laddrs, &laddrs_test,
>> - lsp_test);
>> + bool has_duplicate =
>> + lsp_contains_duplicate_ip(&laddrs, &laddrs_test,
>> + lsp_test, &sub_error);
>> destroy_lport_addresses(&laddrs_test);
>> - if (sub_error) {
>> + if (has_duplicate) {
>> goto err_out;
>> }
>> }
>> @@ -1529,7 +1562,6 @@ lsp_contains_duplicates(const struct
>> nbrec_logical_switch *ls,
>> }
>>
>> err_out: ;
>> - char *error = NULL;
>> if (sub_error) {
>> error = xasprintf("Error on switch %s: %s", ls->name, sub_error);
>> free(sub_error);
>> @@ -8677,6 +8709,448 @@ 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 char *
>> +lsp_health_check_parse_protocol(const char *type,
>> + enum health_check_protocol *protocol)
>> +{
>> + char *error = NULL;
>> +
>> + if (!strcmp(type, "icmp")) {
>> + *protocol = LSP_ICMP_HEALTH_CHECK;
>> + } else if (!strcmp(type, "tcp")) {
>> + *protocol = LSP_TCP_HEALTH_CHECK;
>> + } else if (!strcmp(type, "udp")) {
>> + *protocol = LSP_UDP_HEALTH_CHECK;
>> + } else {
>> + error = xasprintf("%s: Type must be icmp, tcp or udp.", type);
>> + }
>> +
>> + return error;
>> +}
>> +
>> +static char *
>> +lsp_health_check_parse_dst_port(struct ctl_context *ctx,
>> + enum health_check_protocol protocol,
>> + const char *protocol_str,
>> + uint16_t *destination_port,
>> + int *target_ip_index)
>> +{
>> + char *error;
>> +
>> + switch (protocol) {
>> + case LSP_TCP_HEALTH_CHECK:
>> + case LSP_UDP_HEALTH_CHECK:
>> + if (ctx->argc < 5) {
>> + error = xasprintf("Destination port required for "
>> + "%s health check.", protocol_str);
>> + return error;
>> + }
>> +
>> + error = parse_l4_port_range(ctx->argv[4], destination_port);
>> + if (error) {
>> + return error;
>> + }
>> +
>> + *target_ip_index = 5;
>> + break;
>> +
>> + case LSP_ICMP_HEALTH_CHECK:
>> + *target_ip_index = 4;
>> + break;
>> +
>> + default:
>> + OVS_NOT_REACHED();
>> + }
>> +
>> + if (*target_ip_index == ctx->argc) {
>> + error = xasprintf("No addresses specified for health checking.");
>> + return error;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static bool
>> +lsp_health_check_get_duplicate_record(
>> + int64_t destination_port, const char *protocol_str,
>> + const char *source_ip_str, const char *target_ip_str,
>> + const struct nbrec_logical_switch_port *lsp)
>> +{
>> + 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 (!strcmp(lsp_hc_p->src_ip, source_ip_str) &&
>> + !strcmp(lsp_hc_p->protocol, protocol_str)) {
>> +
>> + if (strcmp(protocol_str, "icmp") &&
>> + lsp_hc_p->port != destination_port) {
>> + continue;
>> + }
>> +
>> + if (strcmp(lsp_hc_p->address, target_ip_str) == 0) {
>> + return true;
>> + }
>> + }
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static char *
>> +lsp_health_check_parse_target_address(
>> + const char *target_ip_str, const char *protocol_str,
>> + const char *source_ip_str, const int64_t destination_port,
>> + const struct nbrec_logical_switch_port *lsp)
>> +{
>> + bool ip_found_on_port = false;
>> + char *error = NULL;
>> +
>> + struct lport_addresses target_address;
>> + if (!extract_ip_address(target_ip_str, &target_address)) {
>> + error = xasprintf("Not a valid IPv4 or IPv6 address %s.",
>> + target_ip_str);
>> + return error;
>> + }
>> +
>> + struct lport_addresses lsp_address;
>> + for (size_t i = 0; i < lsp->n_addresses; i++) {
>> + if (!extract_lsp_addresses(lsp->addresses[i], &lsp_address)) {
>> + error = xasprintf("Error extracting logical switch port
>> address.");
>> + goto cleanup;
>> + }
>> +
>> + if (lsp_contains_duplicate_ip(&target_address,
>> + &lsp_address, lsp, NULL)) {
>> + ip_found_on_port = true;
>> + }
>> +
>> + destroy_lport_addresses(&lsp_address);
>> +
>> + if (ip_found_on_port) {
>> + break;
>> + }
>> + }
>> +
>> + if (!ip_found_on_port) {
>> + error = xasprintf("Address %s not configured on port %s.",
>> + target_ip_str, lsp->name);
>> + goto cleanup;
>> + }
>> +
>> + if (lsp_health_check_get_duplicate_record(destination_port,
>> + protocol_str,
>> + source_ip_str,
>> + target_ip_str, lsp)) {
>> + error = xasprintf("Health check with address %s already exists "
>> + "on port %s.", target_ip_str, lsp->name);
>> + goto cleanup;
>> + }
>> +
>> +cleanup:
>> + destroy_lport_addresses(&target_address);
>> + return error;
>> +}
>> +
>> +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_address);
>> +}
>> +
>> +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 *lsp_port_name = ctx->argv[1];
>> + const char *protocol_type = ctx->argv[2];
>> + const char *src_ip = ctx->argv[3];
>> + char *error = NULL;
>> +
>> + error = lsp_by_name_or_uuid(ctx, lsp_port_name, true, &lsp);
>> + if (error) {
>> + ctx->error = error;
>> + return;
>> + }
>> +
>> + if (lsp->type && lsp->type[0]) {
>> + ctl_error(ctx, "%s: Health check monitoring supported only for"
>> + " port with vif type.", lsp->type);
>> + return;
>> + }
>> +
>> + char *validated_src_ip = normalize_addr_str(src_ip);
>> + if (!validated_src_ip) {
>> + ctl_error(ctx, "%s: Not a valid IPv4 or IPv6 address.", src_ip);
>> + goto cleanup;
>> + }
>> +
>> + enum health_check_protocol protocol;
>> + error = lsp_health_check_parse_protocol(protocol_type, &protocol);
>> + if (error) {
>> + ctx->error = error;
>> + goto cleanup;
>> + }
>> +
>> + uint16_t destination_port = 0;
>> + int target_ip_index = 0;
>> + error = lsp_health_check_parse_dst_port(ctx, protocol,
>> + protocol_type,
>> + &destination_port,
>> + &target_ip_index);
>> + if (error) {
>> + ctx->error = error;
>> + goto cleanup;
>> + }
>> +
>> + const char *target_address = ctx->argv[target_ip_index];
>> + error = lsp_health_check_parse_target_address(
>> + target_address, protocol_type,
>> + validated_src_ip, destination_port, lsp);
>> + if (error) {
>> + ctx->error = error;
>> + goto cleanup;
>> + }
>> +
>> + lsp_hc = nbrec_logical_switch_port_health_check_insert(ctx->txn);
>> + nbrec_logical_switch_port_health_check_set_protocol(lsp_hc,
>> + protocol_type);
>> + nbrec_logical_switch_port_health_check_set_src_ip(lsp_hc,
>> + validated_src_ip);
>> + nbrec_logical_switch_port_health_check_set_port(lsp_hc,
>> + destination_port);
>> +
>> + nbrec_logical_switch_port_health_check_set_address(lsp_hc,
>> + target_address);
>> +
> Nit: these two empty lines above are not really needed.
>
>> + nbrec_logical_switch_port_update_health_checks_addvalue(lsp, lsp_hc);
> Do we need a nbrec_logical_switch_port_verify_health_checks(lsp) above?
I haven't encountered this API (_verify_ functions) in code before. I’ll go
ahead and use it for now, but could you help me understand the specific cases
where it’s actually necessary? I want to avoid misusing it or leaving gaps in
the code. It's just not used in all places now in code. Is it for some
cascanding updates , but I’m not entirely clear on how this API fits in. I'm
also trying to figure out a problem in IDL right now, maybe this can help me.
If you have a moment, I’d appreciate a brief explanation ?
>> +
>> +cleanup:
>> + free(validated_src_ip);
>> +}
>> +
>> +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_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 (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);
> Do we need a nbrec_logical_switch_port_verify_health_checks(lsp) above?
>
>> + 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
>> +lsp_health_check_cmp(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) {
>> + return protocol_cmp;
>> + }
>> +
>> + if (strcmp(lsp_hc_1->protocol, "icmp")) {
>> + if (lsp_hc_1->port != lsp_hc_2->port) {
>> + return lsp_hc_1->port < lsp_hc_2->port ? -1 : 1;
>> + }
>> + }
>> +
>> + int ip_cmp = strcmp(lsp_hc_1->address, lsp_hc_2->address);
>> + 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_address);
>> +}
>> +
>> +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, lsp_health_check_cmp);
>> +
>> + 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];
> Nit: this looks a bit weird. Maybe write it on a single line, it seems
> to fit.
>
>> + 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);
>> + }
>> +
>> + ds_put_format(&ctx->output, " Addresses : ");
>> + ds_put_format(&ctx->output, "%s", hc->address);
>> + 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 +9538,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", 4, 5, "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,
Alexandra.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
