vconn: Fix tracking of "connected" state.

While I was looking at the rconn code for connection backoff and retry, I
noticed that ovs-vswitchd was logging the following on each connection
attempt:

   Jun 11 15:17:41|00020|vconn_stream|ERR|send: Connection refused

The "send:" part didn't make much sense.  The configured controller was not
actually running, so the vconn code should not have been able to connect
at all, so the message should have been about a connection failing, not
about sending on a completed connection failing.

Investigation showed that different parts of the library have different
ideas about return value semantics.  vconn_open() and stream_open() both
return 0 if a connection succeeded or if one is in progress, but some of
its callers thought that it returned 0 if the connection succeeded and
EAGAIN if the connection was in progress.  This commit fixes up the callers
that had the wrong idea, by making them instead all vconn_connect() or
stream_connect() to determine whether the connection is complete.

diff --git a/lib/stream.c b/lib/stream.c
index 667a23ff370facf6d8266aa976afc993bd992d6b..acbefc2785a020439aeed89668bb3a6f6188c731 100644 (file)
--- a/lib/stream.c
+++ b/lib/stream.c
@@ -240,14 +240,16 @@ stream_open_block(int error, struct stream **streamp)
 
     fatal_signal_run();
 
-    while (error == EAGAIN) {
-        stream_run(stream);
-        stream_run_wait(stream);
-        stream_connect_wait(stream);
-        poll_block();
-        error = stream_connect(stream);
+    if (!error) {
+        while ((error = stream_connect(stream)) == EAGAIN) {
+            stream_run(stream);
+            stream_run_wait(stream);
+            stream_connect_wait(stream);
+            poll_block();
+        }
         assert(error != EINPROGRESS);
     }
+
     if (error) {
         stream_close(stream);
         *streamp = NULL;

diff --git a/lib/vconn-stream.c b/lib/vconn-stream.c
index 3d0887463ac87d8854a3342e857d74970425a22c..df728d5ccde9870c792981f6d198f4b3b0872114 100644 (file)
--- a/lib/vconn-stream.c
+++ b/lib/vconn-stream.c
@@ -85,13 +85,16 @@ vconn_stream_open(const char *name, char *suffix OVS_UNUSED,
 
     error = stream_open_with_default_ports(name, OFP_TCP_PORT, OFP_SSL_PORT,
                                            &stream);
-
-    if (error && error != EAGAIN) {
-        return error;
+    if (!error) {
+        error = stream_connect(stream);
+        if (!error || error == EAGAIN) {
+            *vconnp = vconn_stream_new(stream, error);
+            return 0;
+        }
     }
 
-    *vconnp = vconn_stream_new(stream, error);
-    return 0;
+    stream_close(stream);
+    return error;
 }
 
 static struct vconn_stream *

diff --git a/lib/vconn.c b/lib/vconn.c
index f4b3169e36094ad6be7e50ef8e24667cc2cc490a..b558f8069fd085f3c58eb3e6aca926d2b9b15984 100644 (file)
--- a/lib/vconn.c
+++ b/lib/vconn.c
@@ -278,14 +278,16 @@ vconn_open_block(const char *name, int min_version, struct vconn **vconnp)
     fatal_signal_run();
 
     error = vconn_open(name, min_version, &vconn);
-    while (error == EAGAIN) {
-        vconn_run(vconn);
-        vconn_run_wait(vconn);
-        vconn_connect_wait(vconn);
-        poll_block();
-        error = vconn_connect(vconn);
+    if (!error) {
+        while ((error == vconn_connect(vconn)) == EAGAIN) {
+            vconn_run(vconn);
+            vconn_run_wait(vconn);
+            vconn_connect_wait(vconn);
+            poll_block();
+        }
         assert(error != EINPROGRESS);
     }
+
     if (error) {
         vconn_close(vconn);
         *vconnp = NULL;

diff --git a/tests/test-vconn.c b/tests/test-vconn.c
index 265bdecb5e7e6acd2116ba390ab3969a95cdeadd..adddc682d1770af602901f386e2a618f5e0658a4 100644 (file)
--- a/tests/test-vconn.c
+++ b/tests/test-vconn.c
@@ -141,7 +141,9 @@ test_refuse_connection(int argc OVS_UNUSED, char *argv[])
     struct fake_pvconn fpv;
     struct vconn *vconn;
 
-    expected_error = !strcmp(type, "unix") ? EPIPE : ECONNRESET;
+    expected_error = (!strcmp(type, "unix") ? EPIPE
+                      : !strcmp(type, "tcp") ? ECONNRESET
+                      : EPROTO);
 
     fpv_create(type, &fpv);
     CHECK_ERRNO(vconn_open(fpv.vconn_name, OFP_VERSION, &vconn), 0);