[ovs-dev] [PATCH v2 1/5] Enable kernel probes and map stream probes onto them

anton.ivanov at cambridgegreys.com anton.ivanov at cambridgegreys.com
Tue Jun 2 07:21:48 UTC 2020


From: Anton Ivanov <anton.ivanov at cambridgegreys.com>

1. Fix probe logic. The stream_or_pstream_needs_probes
function returning a mix of integer and boolean. As a
result probes were NOT turned off in a number of cases on
unix domain sockets and other transports where there
should be no probing. It now returns -1 (do not know),
0 (no probe needed), 1 (definitely needs probes).

2. Allow delegating probing to keepalive facilities in the
stream layer if avaialable.

3. Provide TCP KEEPALIVE probing at stream layer on supported
platforms for stream-ssl and stream-tcp.

Signed-off-by: Anton Ivanov <anton.ivanov at cambridgegreys.com>
---
 lib/jsonrpc.c          | 36 ++++++++++++++++++++++++++++++++--
 lib/rconn.c            |  2 +-
 lib/socket-util.c      | 44 ++++++++++++++++++++++++++++++++++++++++++
 lib/socket-util.h      |  8 ++++++++
 lib/stream-fd.c        |  9 +++++++++
 lib/stream-provider.h  |  6 ++++++
 lib/stream-ssl.c       |  8 ++++++++
 lib/stream-tcp.c       |  1 +
 lib/stream-unix.c      |  1 +
 lib/stream-windows.c   |  1 +
 lib/stream.c           | 17 ++++++++++++++--
 lib/stream.h           |  1 +
 ovsdb/jsonrpc-server.c |  2 +-
 13 files changed, 130 insertions(+), 6 deletions(-)

diff --git a/lib/jsonrpc.c b/lib/jsonrpc.c
index ed748dbde..15e8d3527 100644
--- a/lib/jsonrpc.c
+++ b/lib/jsonrpc.c
@@ -787,6 +787,7 @@ struct jsonrpc_session {
     int last_error;
     unsigned int seqno;
     uint8_t dscp;
+    int probe_interval;
 };
 
 static void
@@ -839,6 +840,7 @@ jsonrpc_session_open_multiple(const struct svec *remotes, bool retry)
     s->seqno = 0;
     s->dscp = 0;
     s->last_error = 0;
+    s->probe_interval = reconnect_get_probe_interval(s->reconnect);
 
     const char *name = reconnect_get_name(s->reconnect);
     if (!pstream_verify_name(name)) {
@@ -848,8 +850,9 @@ jsonrpc_session_open_multiple(const struct svec *remotes, bool retry)
         reconnect_set_backoff(s->reconnect, INT_MAX, INT_MAX);
     }
 
-    if (!stream_or_pstream_needs_probes(name)) {
+    if (stream_or_pstream_needs_probes(name) < 1) {
         reconnect_set_probe_interval(s->reconnect, 0);
+        s->probe_interval = 0;
     }
 
     return s;
@@ -879,6 +882,12 @@ jsonrpc_session_open_unreliably(struct jsonrpc *jsonrpc, uint8_t dscp)
     s->stream = NULL;
     s->pstream = NULL;
     s->seqno = 1;
+    s->probe_interval = reconnect_get_probe_interval(s->reconnect);
+
+    if (stream_or_pstream_needs_probes(reconnect_get_name(s->reconnect)) < 1) {
+        reconnect_set_probe_interval(s->reconnect, 0);
+        s->probe_interval = 0;
+    }
 
     return s;
 }
@@ -934,6 +943,12 @@ jsonrpc_session_connect(struct jsonrpc_session *s)
         error = jsonrpc_stream_open(name, &s->stream, s->dscp);
         if (!error) {
             reconnect_connecting(s->reconnect, time_msec());
+            if (stream_set_probe_interval(s->stream, s->probe_interval)) {
+                /* we have delegated probing to the stream layer */
+                reconnect_set_probe_interval(s->reconnect, 0);
+            } else {
+                reconnect_set_probe_interval(s->reconnect, s->probe_interval);
+            }
         } else {
             s->last_error = error;
         }
@@ -967,6 +982,12 @@ jsonrpc_session_run(struct jsonrpc_session *s)
                 jsonrpc_session_disconnect(s);
             }
             reconnect_connected(s->reconnect, time_msec());
+            if (stream_set_probe_interval(stream, s->probe_interval)) {
+                /* we have delegated probing to the stream layer */
+                reconnect_set_probe_interval(s->reconnect, 0);
+            } else {
+                reconnect_set_probe_interval(s->reconnect, s->probe_interval);
+            }
             s->rpc = jsonrpc_open(stream);
             s->seqno++;
         } else if (error != EAGAIN) {
@@ -1008,6 +1029,12 @@ jsonrpc_session_run(struct jsonrpc_session *s)
         if (!error) {
             reconnect_connected(s->reconnect, time_msec());
             s->rpc = jsonrpc_open(s->stream);
+            if (stream_set_probe_interval(s->stream, s->probe_interval)) {
+                /* we have delegated probing to the stream layer */
+                reconnect_set_probe_interval(s->reconnect, 0);
+            } else {
+                reconnect_set_probe_interval(s->reconnect, s->probe_interval);
+            }
             s->stream = NULL;
             s->seqno++;
         } else if (error != EAGAIN) {
@@ -1231,7 +1258,12 @@ void
 jsonrpc_session_set_probe_interval(struct jsonrpc_session *s,
                                    int probe_interval)
 {
-    reconnect_set_probe_interval(s->reconnect, probe_interval);
+    s->probe_interval = probe_interval;
+    if (s->stream && s->probe_interval) {
+        if (!stream_set_probe_interval(s->stream, probe_interval)) {
+           reconnect_set_probe_interval(s->reconnect, probe_interval);
+        }
+    }
 }
 
 /* Sets the DSCP value used for 's''s connection to 'dscp'.  If this is
diff --git a/lib/rconn.c b/lib/rconn.c
index a96b2eb8b..16e7a44d4 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -346,7 +346,7 @@ rconn_connect(struct rconn *rc, const char *target, const char *name)
     rconn_disconnect__(rc);
     rconn_set_target__(rc, target, name);
     rc->reliable = true;
-    if (!stream_or_pstream_needs_probes(target)) {
+    if (stream_or_pstream_needs_probes(target) < 1) {
         rc->probe_interval = 0;
     }
     reconnect(rc);
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 4f1ffecf5..d6d1967f6 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -114,6 +114,50 @@ setsockopt_tcp_nodelay(int fd)
     }
 }
 
+#ifdef HAS_KERNEL_KEEPALIVES
+bool tcp_set_probe_interval(int fd, int probe_interval) {
+    int on = 1;
+    int retval;
+    int value;
+
+    on = 1;
+    retval = setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &on, sizeof on);
+    if (retval) {
+        retval = sock_errno();
+        VLOG_DBG("setsockopt(SO_KEEPALIVE): %s", sock_strerror(retval));
+        return false;
+    }
+    value = 2;
+    retval = setsockopt(fd,
+            IPPROTO_TCP, TCP_KEEPCNT, &value, sizeof value);
+    if (retval) {
+        retval = sock_errno();
+        VLOG_DBG("setsockopt(TCP_KEEPCNT): %s", sock_strerror(retval));
+        return false;
+    }
+    value = probe_interval;
+    retval = setsockopt(fd,
+            IPPROTO_TCP, TCP_KEEPIDLE, &value, sizeof value);
+    if (retval) {
+        retval = sock_errno();
+        VLOG_DBG("setsockopt(TCP_KEEPIDLE): %s", sock_strerror(retval));
+        return false;
+    }
+    value = probe_interval;
+    retval = setsockopt(fd,
+            IPPROTO_TCP, TCP_KEEPINTVL, &value, sizeof value);
+    if (retval) {
+        retval = sock_errno();
+        VLOG_DBG("setsockopt(SO_KEEPALIVE): %s", sock_strerror(retval));
+        return false;
+    }
+    return true;
+#else
+bool tcp_set_probe_interval(int fd OVS_UNUSED, int probe_interval OVS_UNUSED) {
+    return false;
+#endif
+}
+
 /* Sets the DSCP value of socket 'fd' to 'dscp', which must be 63 or less.
  * 'family' must indicate the socket's address family (AF_INET or AF_INET6, to
  * do anything useful). */
diff --git a/lib/socket-util.h b/lib/socket-util.h
index 9ccb7d4cc..2439e2f78 100644
--- a/lib/socket-util.h
+++ b/lib/socket-util.h
@@ -27,6 +27,7 @@
 #include "openvswitch/types.h"
 #include <netinet/in_systm.h>
 #include <netinet/ip.h>
+#include <netinet/tcp.h>
 
 struct ds;
 
@@ -181,4 +182,11 @@ static inline int sock_errno(void)
 #endif
 }
 
+#if defined (SO_KEEPALIVE) && defined (TCP_KEEPCNT) && \
+    defined (TCP_KEEPIDLE) && defined (TCP_KEEPINTVL)
+#define HAS_KERNEL_KEEPALIVES 1
+#endif
+
+bool tcp_set_probe_interval(int fd, int probe_interval);
+
 #endif /* socket-util.h */
diff --git a/lib/stream-fd.c b/lib/stream-fd.c
index 46ee7ae27..30622929b 100644
--- a/lib/stream-fd.c
+++ b/lib/stream-fd.c
@@ -162,6 +162,14 @@ fd_wait(struct stream *stream, enum stream_wait_type wait)
     }
 }
 
+static bool fd_set_probe_interval(struct stream *stream, int probe_interval) {
+    struct stream_fd *sf = stream_fd_cast(stream);
+
+    return tcp_set_probe_interval(sf->fd, probe_interval);
+}
+
+
+
 static const struct stream_class stream_fd_class = {
     "fd",                       /* name */
     false,                      /* needs_probes */
@@ -173,6 +181,7 @@ static const struct stream_class stream_fd_class = {
     NULL,                       /* run */
     NULL,                       /* run_wait */
     fd_wait,                    /* wait */
+    fd_set_probe_interval,      /* set_probe_interval */
 };
 
 /* Passive file descriptor stream. */
diff --git a/lib/stream-provider.h b/lib/stream-provider.h
index 75f4f059b..6c28cb50b 100644
--- a/lib/stream-provider.h
+++ b/lib/stream-provider.h
@@ -124,6 +124,12 @@ struct stream_class {
     /* Arranges for the poll loop to wake up when 'stream' is ready to take an
      * action of the given 'type'. */
     void (*wait)(struct stream *stream, enum stream_wait_type type);
+    /* Sets low level keepalives if supported
+     *
+     *     If successful returns true
+     *
+     */
+    bool (*set_probe_interval)(struct stream *stream, int probe_interval);
 };
 
 /* Passive listener for incoming stream connections.
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 078fcbc3a..575c55f5b 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -789,6 +789,13 @@ ssl_run(struct stream *stream)
     }
 }
 
+static bool ssl_set_probe_interval(struct stream *stream, int probe_interval) {
+    struct ssl_stream *sslv = ssl_stream_cast(stream);
+
+    return tcp_set_probe_interval(sslv->fd, probe_interval);
+}
+
+
 static void
 ssl_run_wait(struct stream *stream)
 {
@@ -861,6 +868,7 @@ const struct stream_class ssl_stream_class = {
     ssl_run,                    /* run */
     ssl_run_wait,               /* run_wait */
     ssl_wait,                   /* wait */
+    ssl_set_probe_interval,     /* set_probe_interval */
 };
 
 /* Passive SSL. */
diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c
index e8dc2bfaa..67c912105 100644
--- a/lib/stream-tcp.c
+++ b/lib/stream-tcp.c
@@ -73,6 +73,7 @@ const struct stream_class tcp_stream_class = {
     NULL,                       /* run */
     NULL,                       /* run_wait */
     NULL,                       /* wait */
+    NULL,
 };
 
 /* Passive TCP. */
diff --git a/lib/stream-unix.c b/lib/stream-unix.c
index d265efb83..2322df955 100644
--- a/lib/stream-unix.c
+++ b/lib/stream-unix.c
@@ -73,6 +73,7 @@ const struct stream_class unix_stream_class = {
     NULL,                       /* run */
     NULL,                       /* run_wait */
     NULL,                       /* wait */
+    NULL,
 };
 
 /* Passive UNIX socket. */
diff --git a/lib/stream-windows.c b/lib/stream-windows.c
index 5c4c55e5d..836112f75 100644
--- a/lib/stream-windows.c
+++ b/lib/stream-windows.c
@@ -374,6 +374,7 @@ const struct stream_class windows_stream_class = {
     NULL,                       /* run */
     NULL,                       /* run_wait */
     windows_wait,               /* wait */
+    NULL,
 };
 
 struct pwindows_pstream
diff --git a/lib/stream.c b/lib/stream.c
index e246b3773..f0b16fb4f 100644
--- a/lib/stream.c
+++ b/lib/stream.c
@@ -430,6 +430,19 @@ stream_wait(struct stream *stream, enum stream_wait_type wait)
     (stream->class->wait)(stream, wait);
 }
 
+
+bool stream_set_probe_interval(struct stream *stream, int probe_interval) {
+    if (! stream->class->needs_probes) {
+        return true;
+    }
+    if (probe_interval && stream->class->set_probe_interval) {
+        return (stream->class->set_probe_interval)(
+                stream, probe_interval / 1000);
+    }
+    return false;
+}
+
+
 void
 stream_connect_wait(struct stream *stream)
 {
@@ -509,9 +522,9 @@ stream_or_pstream_needs_probes(const char *name)
     const struct stream_class *class;
 
     if (!stream_lookup_class(name, &class)) {
-        return class->needs_probes;
+        return class->needs_probes ? 1 : 0;
     } else if (!pstream_lookup_class(name, &pclass)) {
-        return pclass->needs_probes;
+        return pclass->needs_probes ? 1 : 0;
     } else {
         return -1;
     }
diff --git a/lib/stream.h b/lib/stream.h
index 77bffa498..e343f75a5 100644
--- a/lib/stream.h
+++ b/lib/stream.h
@@ -40,6 +40,7 @@ const char *stream_get_name(const struct stream *);
 int stream_connect(struct stream *);
 int stream_recv(struct stream *, void *buffer, size_t n);
 int stream_send(struct stream *, const void *buffer, size_t n);
+bool stream_set_probe_interval(struct stream *, int probe_interval);
 
 void stream_run(struct stream *);
 void stream_run_wait(struct stream *);
diff --git a/ovsdb/jsonrpc-server.c b/ovsdb/jsonrpc-server.c
index 4e2dfc3d7..fe8ffc317 100644
--- a/ovsdb/jsonrpc-server.c
+++ b/ovsdb/jsonrpc-server.c
@@ -212,7 +212,7 @@ ovsdb_jsonrpc_default_options(const char *target)
 {
     struct ovsdb_jsonrpc_options *options = xzalloc(sizeof *options);
     options->max_backoff = RECONNECT_DEFAULT_MAX_BACKOFF;
-    options->probe_interval = (stream_or_pstream_needs_probes(target)
+    options->probe_interval = (stream_or_pstream_needs_probes(target) < 1
                                ? RECONNECT_DEFAULT_PROBE_INTERVAL
                                : 0);
     return options;
-- 
2.20.1



More information about the dev mailing list