On some systems in case where remote is not responding, socket could
remain in SYN_SENT state for a really long time without errors waiting
for connection. This leads to situations where open_blok() hangs for
a few minutes waiting for connection to the DOWN remote.

For example, our "multiple remotes" idl tests hangs waiting for
connection to the WRONG_PORT on FreeBSD in CirrusCI environment.
This leads to test failures because Alarm signal arrives much faster
than ETIMEDOUT from the socket.

This patch allowes to specify timeout value for 'open_block' function.
If the connection takes more time, socket will be closed with
ETIMEDOUT error code. Negative value or None in python could be
used to wait infinitely.

Signed-off-by: Ilya Maximets <[email protected]>
---
 lib/stream.c         | 27 ++++++++++++++++++++-------
 lib/stream.h         |  2 +-
 lib/unixctl.c        |  2 +-
 ovsdb/ovsdb-client.c |  2 +-
 python/ovs/stream.py | 16 +++++++++++++---
 tests/test-jsonrpc.c |  4 ++--
 tests/test-ovsdb.c   |  2 +-
 tests/test-ovsdb.py  |  2 +-
 tests/test-stream.c  |  2 +-
 tests/test-stream.py |  2 +-
 10 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/lib/stream.c b/lib/stream.c
index c4dabda39..bba56905e 100644
--- a/lib/stream.c
+++ b/lib/stream.c
@@ -241,27 +241,40 @@ error:
 }
 
 /* Blocks until a previously started stream connection attempt succeeds or
- * fails.  'error' should be the value returned by stream_open() and 'streamp'
- * should point to the stream pointer set by stream_open().  Returns 0 if
- * successful, otherwise a positive errno value other than EAGAIN or
- * EINPROGRESS.  If successful, leaves '*streamp' untouched; on error, closes
- * '*streamp' and sets '*streamp' to null.
+ * fails, but no more than 'timeout' milliseconds.  'error' should be the
+ * value returned by stream_open() and 'streamp' should point to the stream
+ * pointer set by stream_open().  Returns 0 if successful, otherwise a
+ * positive errno value other than EAGAIN or EINPROGRESS.  If successful,
+ * leaves '*streamp' untouched; on error, closes '*streamp' and sets
+ * '*streamp' to null. Negative value of 'timeout' means infinite waiting.
  *
  * Typical usage:
- *   error = stream_open_block(stream_open("tcp:1.2.3.4:5", &stream), &stream);
+ *   error = stream_open_block(stream_open("tcp:1.2.3.4:5", &stream), -1,
+ *                             &stream);
  */
 int
-stream_open_block(int error, struct stream **streamp)
+stream_open_block(int error, long long int timeout, struct stream **streamp)
 {
     struct stream *stream = *streamp;
+    long long int deadline = LLONG_MAX;
 
     fatal_signal_run();
 
     if (!error) {
+        if (timeout >= 0) {
+            deadline = time_msec() + timeout;
+        }
         while ((error = stream_connect(stream)) == EAGAIN) {
+            if (deadline != LLONG_MAX && time_msec() > deadline) {
+                error = ETIMEDOUT;
+                break;
+            }
             stream_run(stream);
             stream_run_wait(stream);
             stream_connect_wait(stream);
+            if (deadline != LLONG_MAX) {
+                poll_timer_wait_until(deadline);
+            }
             poll_block();
         }
         ovs_assert(error != EINPROGRESS);
diff --git a/lib/stream.h b/lib/stream.h
index 88f576155..77bffa498 100644
--- a/lib/stream.h
+++ b/lib/stream.h
@@ -34,7 +34,7 @@ void stream_usage(const char *name, bool active, bool 
passive, bool bootstrap);
 /* Bidirectional byte streams. */
 int stream_verify_name(const char *name);
 int stream_open(const char *name, struct stream **, uint8_t dscp);
-int stream_open_block(int error, struct stream **);
+int stream_open_block(int error, long long int timeout, struct stream **);
 void stream_close(struct stream *);
 const char *stream_get_name(const struct stream *);
 int stream_connect(struct stream *);
diff --git a/lib/unixctl.c b/lib/unixctl.c
index 0bcfada91..c216de3d0 100644
--- a/lib/unixctl.c
+++ b/lib/unixctl.c
@@ -460,7 +460,7 @@ unixctl_client_create(const char *path, struct jsonrpc 
**client)
     *client = NULL;
 
     error = stream_open_block(stream_open(unix_path, &stream, DSCP_DEFAULT),
-                              &stream);
+                              -1, &stream);
     free(unix_path);
     free(abs_path);
 
diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index 7c8a59d0e..83c3c12cc 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -498,7 +498,7 @@ open_jsonrpc(const char *server)
     int error;
 
     error = stream_open_block(jsonrpc_stream_open(server, &stream,
-                              DSCP_DEFAULT), &stream);
+                              DSCP_DEFAULT), -1, &stream);
     if (error == EAFNOSUPPORT) {
         struct pstream *pstream;
 
diff --git a/python/ovs/stream.py b/python/ovs/stream.py
index cdfcc399e..da683afd8 100644
--- a/python/ovs/stream.py
+++ b/python/ovs/stream.py
@@ -206,10 +206,12 @@ class Stream(object):
         raise NotImplementedError("This method must be overrided by subclass")
 
     @staticmethod
-    def open_block(error_stream):
+    def open_block(error_stream, timeout=None):
         """Blocks until a Stream completes its connection attempt, either
-        succeeding or failing.  (error, stream) should be the tuple returned by
-        Stream.open().  Returns a tuple of the same form.
+        succeeding or failing, but no more than 'timeout' milliseconds.
+        (error, stream) should be the tuple returned by Stream.open().
+        Negative value of 'timeout' means infinite waiting.
+        Returns a tuple of the same form.
 
         Typical usage:
         error, stream = Stream.open_block(Stream.open("unix:/tmp/socket"))"""
@@ -217,6 +219,9 @@ class Stream(object):
         # Py3 doesn't support tuple parameter unpacking - PEP 3113
         error, stream = error_stream
         if not error:
+            deadline = None
+            if timeout is not None and timeout >= 0:
+                deadline = ovs.timeval.msec() + timeout
             while True:
                 error = stream.connect()
                 if sys.platform == 'win32' and error == errno.WSAEWOULDBLOCK:
@@ -225,10 +230,15 @@ class Stream(object):
                     error = errno.EAGAIN
                 if error != errno.EAGAIN:
                     break
+                if deadline is not None and ovs.timeval.msec() > deadline:
+                    error = errno.ETIMEDOUT
+                    break
                 stream.run()
                 poller = ovs.poller.Poller()
                 stream.run_wait(poller)
                 stream.connect_wait(poller)
+                if deadline is not None:
+                    poller.timer_wait_until(deadline)
                 poller.block()
             if stream.socket is not None:
                 assert error != errno.EINPROGRESS
diff --git a/tests/test-jsonrpc.c b/tests/test-jsonrpc.c
index 49d2b91bd..04e941b14 100644
--- a/tests/test-jsonrpc.c
+++ b/tests/test-jsonrpc.c
@@ -272,7 +272,7 @@ do_request(struct ovs_cmdl_context *ctx)
     }
 
     error = stream_open_block(jsonrpc_stream_open(ctx->argv[1], &stream,
-                              DSCP_DEFAULT), &stream);
+                              DSCP_DEFAULT), -1, &stream);
     if (error) {
         ovs_fatal(error, "could not open \"%s\"", ctx->argv[1]);
     }
@@ -312,7 +312,7 @@ do_notify(struct ovs_cmdl_context *ctx)
     }
 
     error = stream_open_block(jsonrpc_stream_open(ctx->argv[1], &stream,
-                              DSCP_DEFAULT), &stream);
+                              DSCP_DEFAULT), -1, &stream);
     if (error) {
         ovs_fatal(error, "could not open \"%s\"", ctx->argv[1]);
     }
diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c
index 453d88eab..187eb2867 100644
--- a/tests/test-ovsdb.c
+++ b/tests/test-ovsdb.c
@@ -2416,7 +2416,7 @@ do_idl(struct ovs_cmdl_context *ctx)
         struct stream *stream;
 
         error = stream_open_block(jsonrpc_stream_open(ctx->argv[1], &stream,
-                                  DSCP_DEFAULT), &stream);
+                                  DSCP_DEFAULT), -1, &stream);
         if (error) {
             ovs_fatal(error, "failed to connect to \"%s\"", ctx->argv[1]);
         }
diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py
index c03476c7f..2d1112ddd 100644
--- a/tests/test-ovsdb.py
+++ b/tests/test-ovsdb.py
@@ -604,7 +604,7 @@ def do_idl(schema_file, remote, *commands):
         stream = None
         for r in remotes:
             error, stream = ovs.stream.Stream.open_block(
-                ovs.stream.Stream.open(r))
+                ovs.stream.Stream.open(r), 2000)
             if not error and stream:
                 break
             stream = None
diff --git a/tests/test-stream.c b/tests/test-stream.c
index 4816de02d..4af44200e 100644
--- a/tests/test-stream.c
+++ b/tests/test-stream.c
@@ -37,7 +37,7 @@ main(int argc, char *argv[])
     }
 
     error = stream_open_block(stream_open(argv[1], &stream, DSCP_DEFAULT),
-                              &stream);
+                              10000, &stream);
     if (error) {
         VLOG_ERR("stream_open_block(%s) failure: %s",
                  argv[1], ovs_strerror(error));
diff --git a/tests/test-stream.py b/tests/test-stream.py
index 4a5117501..93d63c019 100644
--- a/tests/test-stream.py
+++ b/tests/test-stream.py
@@ -20,7 +20,7 @@ import ovs.stream
 def main(argv):
     remote = argv[1]
     err, stream = ovs.stream.Stream.open_block(
-            ovs.stream.Stream.open(remote))
+            ovs.stream.Stream.open(remote), 10000)
 
     if err or stream is None:
         sys.exit(1)
-- 
2.17.1

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

Reply via email to