1) Add LSP service monitor type and handle ICMP health checks for
   logical switch ports in addition to network functions.

2) Added functions for handling valid types and monitoring protocols
   to remove all checks below.

Signed-off-by: Alexandra Rukomoinikova <[email protected]>
---
v2 --> v3: fixed Dumitru's comments
           squashes this patch from 6 patch in v2
---
 controller/pinctrl.c | 141 +++++++++++++++++++++++++++++--------------
 tests/system-ovn.at  |  97 +++++++++++++++++++++++++++++
 2 files changed, 192 insertions(+), 46 deletions(-)

diff --git a/controller/pinctrl.c b/controller/pinctrl.c
index 08940cd8b..f39ffc166 100644
--- a/controller/pinctrl.c
+++ b/controller/pinctrl.c
@@ -6912,6 +6912,8 @@ enum svc_monitor_type {
     SVC_MON_TYPE_LB,
     /* network function */
     SVC_MON_TYPE_NF,
+    /* logical switch port */
+    SVC_MON_TYPE_LSP,
 };
 
 /* Service monitor health checks. */
@@ -7019,6 +7021,58 @@ pinctrl_find_svc_monitor(uint32_t dp_key, uint32_t 
port_key,
     return NULL;
 }
 
+static enum svc_monitor_type
+svc_monitor_type_from_sb(const struct sbrec_service_monitor *sb)
+{
+    if (sb->type &&
+        !strcmp(sb->type, "network-function")) {
+        return SVC_MON_TYPE_NF;
+    }
+
+    if (sb->type &&
+        !strcmp(sb->type, "logical-switch-port")) {
+        return SVC_MON_TYPE_LSP;
+    }
+
+    return SVC_MON_TYPE_LB;
+}
+
+static enum svc_monitor_protocol
+svc_monitor_proto_from_sb(const struct sbrec_service_monitor *sb)
+{
+    if (!strcmp(sb->protocol, "udp")) {
+        return SVC_MON_PROTO_UDP;
+    }
+
+    if (!strcmp(sb->protocol, "icmp")) {
+        return SVC_MON_PROTO_ICMP;
+    }
+
+    return SVC_MON_PROTO_TCP;
+}
+
+static bool
+svc_monitor_type_allows_proto(enum svc_monitor_type type,
+                              enum svc_monitor_protocol proto)
+{
+    switch (type) {
+    case SVC_MON_TYPE_NF:
+        return proto == SVC_MON_PROTO_ICMP;
+
+    case SVC_MON_TYPE_LB:
+        return proto == SVC_MON_PROTO_TCP ||
+               proto == SVC_MON_PROTO_UDP;
+
+    case SVC_MON_TYPE_LSP:
+        return true;
+
+    default:
+        OVS_NOT_REACHED();
+    }
+
+    return false;
+}
+
 static void
 sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
                   const struct sbrec_service_monitor_table *svc_mon_table,
@@ -7035,23 +7089,6 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
 
     const struct sbrec_service_monitor *sb_svc_mon;
     SBREC_SERVICE_MONITOR_TABLE_FOR_EACH (sb_svc_mon, svc_mon_table) {
-        enum svc_monitor_type mon_type;
-        if (sb_svc_mon->type && !strcmp(sb_svc_mon->type,
-                                        "network-function")) {
-            mon_type = SVC_MON_TYPE_NF;
-        } else {
-            mon_type = SVC_MON_TYPE_LB;
-        }
-
-        enum svc_monitor_protocol protocol;
-        if (!strcmp(sb_svc_mon->protocol, "udp")) {
-            protocol = SVC_MON_PROTO_UDP;
-        } else if (!strcmp(sb_svc_mon->protocol, "icmp")) {
-            protocol = SVC_MON_PROTO_ICMP;
-        } else {
-            protocol = SVC_MON_PROTO_TCP;
-        }
-
         const struct sbrec_port_binding *pb
             = lport_lookup_by_name(sbrec_port_binding_by_name,
                                    sb_svc_mon->logical_port);
@@ -7085,10 +7122,16 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
         struct eth_addr ea;
         bool mac_found = false;
 
+        enum svc_monitor_type mon_type =
+            svc_monitor_type_from_sb(sb_svc_mon);
+        enum svc_monitor_protocol protocol =
+            svc_monitor_proto_from_sb(sb_svc_mon);
+
+        if (!svc_monitor_type_allows_proto(mon_type, protocol)) {
+            continue;
+        }
+
         if (mon_type == SVC_MON_TYPE_NF) {
-            if (protocol != SVC_MON_PROTO_ICMP) {
-                continue;
-            }
             input_pb = lport_lookup_by_name(sbrec_port_binding_by_name,
                                             sb_svc_mon->logical_input_port);
             if (!input_pb) {
@@ -7103,11 +7146,6 @@ sync_svc_monitors(struct ovsdb_idl_txn *ovnsb_idl_txn,
                 }
             }
         } else {
-            if (protocol != SVC_MON_PROTO_TCP &&
-                protocol != SVC_MON_PROTO_UDP) {
-                continue;
-            }
-
             for (size_t i = 0; i < pb->n_mac && !mac_found; i++) {
                 struct lport_addresses laddrs;
 
@@ -8066,6 +8104,7 @@ static void
 svc_monitor_send_icmp_health_check__(struct rconn *swconn,
                                      struct svc_monitor *svc_mon)
 {
+    bool svc_mon_nf = svc_mon->type == SVC_MON_TYPE_NF;
     uint64_t packet_stub[128 / 8];
     struct dp_packet packet;
     dp_packet_use_stub(&packet, packet_stub, sizeof packet_stub);
@@ -8112,7 +8151,8 @@ svc_monitor_send_icmp_health_check__(struct rconn *swconn,
     struct ofpbuf ofpacts = OFPBUF_STUB_INITIALIZER(ofpacts_stub);
     enum ofp_version version = rconn_get_version(swconn);
     put_load(svc_mon->dp_key, MFF_LOG_DATAPATH, 0, 64, &ofpacts);
-    put_load(svc_mon->input_port_key, MFF_LOG_OUTPORT, 0, 32, &ofpacts);
+    put_load(svc_mon_nf ? svc_mon->input_port_key : svc_mon->port_key,
+             MFF_LOG_OUTPORT, 0, 32, &ofpacts);
     put_load(1, MFF_LOG_FLAGS, MLF_LOCAL_ONLY, 1, &ofpacts);
     struct ofpact_resubmit *resubmit = ofpact_put_RESUBMIT(&ofpacts);
     resubmit->in_port = OFPP_CONTROLLER;
@@ -8406,48 +8446,57 @@ pinctrl_handle_svc_check(struct rconn *swconn, const 
struct flow *ip_flow,
             return;
         }
 
-        /* Handle ICMP ECHO REQUEST probes for Network Function services */
+        /* Handle ICMP ECHO REQUEST probes for Network Function and
+         * Logical Switch Port services */
         if (in_eth->eth_type == htons(ETH_TYPE_IP)) {
             struct icmp_header *ih = l4h;
             /* It's ICMP packet. */
-            if (ih->icmp_type == ICMP4_ECHO_REQUEST && ih->icmp_code == 0) {
-                uint32_t hash = hash_bytes(&dst_ip_addr, sizeof dst_ip_addr,
-                                           hash_3words(dp_key, port_key, 0));
+            if ((ih->icmp_type == ICMP4_ECHO_REQUEST ||
+                ih->icmp_type == ICMP4_ECHO_REPLY) && ih->icmp_code == 0) {
+                int is_echo_request = ih->icmp_type == ICMP4_ECHO_REQUEST;
+                struct in6_addr *target_addr = is_echo_request
+                                               ? &dst_ip_addr : &ip_addr;
+                uint32_t hash =
+                    hash_bytes(target_addr, sizeof(*target_addr),
+                               hash_3words(dp_key, port_key, 0));
                 struct svc_monitor *svc_mon =
-                    pinctrl_find_svc_monitor(dp_key, port_key, &dst_ip_addr, 0,
+                    pinctrl_find_svc_monitor(dp_key, port_key, target_addr, 0,
                                              SVC_MON_PROTO_ICMP, hash);
                 if (!svc_mon) {
-                    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(
-                        1, 5);
+                    static struct vlog_rate_limit rl =
+                        VLOG_RATE_LIMIT_INIT(1, 5);
                     VLOG_WARN_RL(&rl, "handle service check: Service monitor "
-                                 "not found for ICMP request");
+                                 "not found for ICMP %s", is_echo_request ?
+                                 "request" : "reply");
                     return;
                 }
-                if (svc_mon->type == SVC_MON_TYPE_NF) {
-                    pinctrl_handle_icmp_svc_check(pkt_in, svc_mon);
-                }
+                pinctrl_handle_icmp_svc_check(pkt_in, svc_mon);
                 return;
             }
         } else if (in_eth->eth_type == htons(ETH_TYPE_IPV6)) {
             struct icmp6_data_header *ih6 = l4h;
             /* It's ICMPv6 packet. */
-            if (ih6->icmp6_base.icmp6_type == ICMP6_ECHO_REQUEST &&
+            if ((ih6->icmp6_base.icmp6_type == ICMP6_ECHO_REQUEST ||
+                ih6->icmp6_base.icmp6_type == ICMP6_ECHO_REPLY) &&
                 ih6->icmp6_base.icmp6_code == 0) {
-                uint32_t hash = hash_bytes(&dst_ip_addr, sizeof dst_ip_addr,
+                int is_echo_request =
+                    (ih6->icmp6_base.icmp6_type == ICMP6_ECHO_REQUEST);
+                struct in6_addr *target_addr = is_echo_request
+                                               ? &dst_ip_addr : &ip_addr;
+                uint32_t hash = hash_bytes(target_addr, sizeof(*target_addr),
                                            hash_3words(dp_key, port_key, 0));
                 struct svc_monitor *svc_mon =
-                    pinctrl_find_svc_monitor(dp_key, port_key, &dst_ip_addr, 0,
+                    pinctrl_find_svc_monitor(dp_key, port_key, target_addr, 0,
                                              SVC_MON_PROTO_ICMP, hash);
                 if (!svc_mon) {
-                    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(
-                        1, 5);
+                    static struct vlog_rate_limit rl =
+                        VLOG_RATE_LIMIT_INIT(1, 5);
                     VLOG_WARN_RL(&rl, "handle service check: Service monitor "
-                                 "not found for ICMPv6 request");
+                                 "not found for ICMPv6 %s", is_echo_request ?
+                                 "request" : "reply");
                     return;
                 }
-                if (svc_mon->type == SVC_MON_TYPE_NF) {
-                    pinctrl_handle_icmp_svc_check(pkt_in, svc_mon);
-                }
+                pinctrl_handle_icmp_svc_check(pkt_in, svc_mon);
                 return;
             }
         }
diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index a478dd709..b88455dea 100644
--- a/tests/system-ovn.at
+++ b/tests/system-ovn.at
@@ -20997,3 +20997,100 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port 
patch-.*/d
 /connection dropped.*/d"])
 AT_CLEANUP
 ])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Logical Switch Port Health Check])
+AT_SKIP_IF([test $HAVE_NC = no])
+
+ovn_start
+OVS_TRAFFIC_VSWITCHD_START()
+ADD_BR([br-int])
+
+# Set external-ids in br-int needed for ovn-controller
+ovs-vsctl \
+        -- set Open_vSwitch . external-ids:system-id=hv1 \
+        -- set Open_vSwitch . 
external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
+        -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
+        -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
+        -- set bridge br-int fail-mode=secure other-config:disable-in-band=true
+
+# Start ovn-controller
+start_daemon ovn-controller
+
+check ovn-nbctl ls-add ls1
+
+ADD_NAMESPACES(lport)
+ADD_VETH(lport, lport, br-int, "2001:db8::10/64", "f0:00:0f:01:02:04", \
+         "2001:db8::11", "nodad", "192.168.0.10/24", "192.168.0.1")
+
+check ovn-nbctl lsp-add ls1 lport -- \
+    lsp-set-addresses lport "f0:00:0f:01:02:04 192.168.0.10 2001:db8::10" -- \
+    lsp-add-router-port ls1 ls1-up lr1-down
+
+check ovn-nbctl lr-add lr1 -- \
+      lrp-add lr1 lr1-down 00:00:00:00:0f:01 192.168.0.1/24 2001:db8::11/64
+
+check ovn-nbctl --wait=hv sync
+
+check ovn-nbctl lsp-hc-add lport icmp 192.168.0.250 192.168.0.10
+check_row_count sb:Service_Monitor 1
+
+# Wait until the services are set to online.
+wait_row_count Service_Monitor 1 status=online
+
+check ovn-nbctl lsp-hc-add lport tcp 192.168.0.250 4041 192.168.0.10
+
+check_row_count sb:Service_Monitor 2
+
+# Create one more health check on logical switch port
+NETNS_DAEMONIZE([lport], [nc -l -k 192.168.0.10 4041], [lport_tcp.pid])
+
+# Wait until the services are set to online.
+wait_row_count Service_Monitor 2 status=online
+
+check ovn-nbctl lsp-hc-add lport udp 192.168.0.250 4042 192.168.0.10
+
+NETNS_DAEMONIZE([lport], [nc -ulp 4042], [lport_udp.pid])
+
+# Wait until the services are set to online.
+wait_row_count Service_Monitor 3 status=online
+
+check ovn-nbctl lsp-hc-del lport
+
+# IPv6 ICMP health check (ping6)
+check ovn-nbctl lsp-hc-add lport icmp 2001:db8::ff 2001:db8::10
+check_row_count sb:Service_Monitor 1
+
+# Wait until the services are set to online.
+wait_row_count Service_Monitor 1 status=online
+
+# IPv6 TCP health check
+check ovn-nbctl lsp-hc-add lport tcp 2001:db8::ff 4043 2001:db8::10
+
+check_row_count sb:Service_Monitor 2
+
+# Start IPv6 TCP server
+NETNS_DAEMONIZE([lport], [nc -6 -l -k 2001:db8::10 4043], [lport_ipv6_tcp.pid])
+
+# Wait until the services are set to online.
+wait_row_count Service_Monitor 2 status=online
+
+# IPv6 UDP health check
+check ovn-nbctl lsp-hc-add lport udp 2001:db8::ff 4044 2001:db8::10
+check_row_count sb:Service_Monitor 3
+
+# Start IPv6 UDP server
+NETNS_DAEMONIZE([lport], [nc -6 -u -l 2001:db8::10 4044], [lport_ipv6_udp.pid])
+
+# Wait until the services are set to online.
+wait_row_count Service_Monitor 3 status=online
+
+OVN_CLEANUP_CONTROLLER([hv1])
+
+OVN_CLEANUP_NORTHD
+
+as
+OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
+/connection dropped.*/d"])
+AT_CLEANUP
+])
-- 
2.48.1

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to