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
> 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?
> +
> +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},
> };
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
