[ovs-dev] [PATCH 1/2] ovs python: ovs.stream.open_block() returns success even if the remote is unreachable

nusiddiq at redhat.com nusiddiq at redhat.com
Sun Jul 8 16:04:02 UTC 2018


From: Numan Siddique <nusiddiq at redhat.com>

Calling ovs.stream.open_block(ovs.stream.open("tcp:127.0.0.1:6641")) returns
success even if there is no server listening on 6641. To check if the connection
is established or not, Stream class makes use of ovs.socket_util.check_connection_completion().
This function returns zero if the select for the socket fd signals. It doesn't
really check if the connection was established or not.

This patch fixes this issue by adding a wrapper function - check_connection_completion_status()
which calls sock.connect_ex() to get the status of the connection if
ovs.socket_util.check_connection_completion() returns success.

The test cases added fails without the fix in this patch.

Signed-off-by: Numan Siddique <nusiddiq at redhat.com>
---
 python/ovs/socket_util.py | 34 ++++++++++++++++++++++++++++++++++
 python/ovs/stream.py      | 16 +++++++++++++---
 tests/automake.mk         |  1 +
 tests/ovsdb-idl.at        | 16 ++++++++++++++++
 tests/test-stream.py      | 32 ++++++++++++++++++++++++++++++++
 5 files changed, 96 insertions(+), 3 deletions(-)
 create mode 100644 tests/test-stream.py

diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
index 403104936..91f4532ea 100644
--- a/python/ovs/socket_util.py
+++ b/python/ovs/socket_util.py
@@ -256,6 +256,40 @@ def inet_open_active(style, target, default_port, dscp):
         return get_exception_errno(e), None
 
 
+def check_connection_completion_status(sock, target, default_port):
+    status = check_connection_completion(sock)
+    if status:
+        return status
+
+    try:
+        address = inet_parse_active(target, default_port)
+    except ValueError:
+        # It's not a valid tcp target.
+        return status
+
+    # For tcp connections, check_connection_completion function returns 0
+    # when the select for the socket fd signals. But it doesn't really
+    # verify the connection was established or not. So call connect again on
+    # the socket to get the status.
+    try:
+        err = sock.connect_ex(address)
+    except socket.error as e:
+        err = get_exception_errno(e)
+        if sys.platform == 'win32' and error == errno.WSAEWOULDBLOCK:
+            # WSAEWOULDBLOCK would be the equivalent on Windows
+            # for EINPROGRESS on Unix.
+            err = err.EINPROGRESS
+
+    if err == errno.EINPROGRESS or err == errno.EALREADY:
+        return errno.EINPROGRESS
+
+    if err == 0 or err == errno.EISCONN:
+        return 0
+
+    sock.close()
+    return err
+
+
 def get_exception_errno(e):
     """A lot of methods on Python socket objects raise socket.error, but that
     exception is documented as having two completely different forms of
diff --git a/python/ovs/stream.py b/python/ovs/stream.py
index 5996497a5..7d5227469 100644
--- a/python/ovs/stream.py
+++ b/python/ovs/stream.py
@@ -119,6 +119,7 @@ class Stream(object):
                                                       bInitialState=False)
 
         self.name = name
+        self.suffix = name.split(":", 1)[1]
         if status == errno.EAGAIN:
             self.state = Stream.__S_CONNECTING
         elif status == 0:
@@ -191,8 +192,16 @@ class Stream(object):
         if error:
             return error, None
         else:
-            status = ovs.socket_util.check_connection_completion(sock)
-            return 0, cls(sock, name, status)
+            err = ovs.socket_util.check_connection_completion_status(
+                sock, suffix, 0)
+            if err == errno.EAGAIN or err == errno.EINPROGRESS:
+                status = errno.EAGAIN
+                err = 0
+            elif err == 0:
+                status = 0
+            else:
+                status = err
+            return err, cls(sock, name, status)
 
     @staticmethod
     def _open(suffix, dscp):
@@ -246,7 +255,8 @@ class Stream(object):
 
     def __scs_connecting(self):
         if self.socket is not None:
-            retval = ovs.socket_util.check_connection_completion(self.socket)
+            retval = ovs.socket_util.check_connection_completion_status(
+                self.socket, self.suffix, 0)
             assert retval != errno.EINPROGRESS
         elif sys.platform == 'win32':
             if self.retry_connect:
diff --git a/tests/automake.mk b/tests/automake.mk
index 8224e5a4a..0abf29d47 100644
--- a/tests/automake.mk
+++ b/tests/automake.mk
@@ -421,6 +421,7 @@ CHECK_PYFILES = \
 	tests/test-l7.py \
 	tests/test-ovsdb.py \
 	tests/test-reconnect.py \
+	tests/test-stream.py \
 	tests/MockXenAPI.py \
 	tests/test-unix-socket.py \
 	tests/test-unixctl.py \
diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 58935faf3..d4d283db4 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -1720,3 +1720,19 @@ OVSDB_CHECK_IDL_COMPOUND_INDEX_WITH_REF([set, simple3 idl-compound-index-with-re
 007: check simple4: empty
 008: End test
 ]])
+
+m4_define([CHECK_STREAM_OPEN_BLOCK_PY],
+  [AT_SETUP([$1])
+   AT_SKIP_IF([test $2 = no])
+   AT_KEYWORDS([Check PY Stream open block - $3])
+   AT_CHECK([ovsdb_start_idltest "ptcp:0:127.0.0.1"])
+   PARSE_LISTENING_PORT([ovsdb-server.log], [TCP_PORT])
+   WRONG_PORT=$(($TCP_PORT+1))
+   AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$TCP_PORT], [0], [ignore])
+   AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$WRONG_PORT], [1], [ignore])
+   OVSDB_SERVER_SHUTDOWN
+   AT_CHECK([$3 $srcdir/test-stream.py tcp:127.0.0.1:$TCP_PORT], [1], [ignore])
+   AT_CLEANUP])
+
+CHECK_STREAM_OPEN_BLOCK_PY([Check PY2 Stream open block], [$HAVE_PYTHON], [$PYTHON])
+CHECK_STREAM_OPEN_BLOCK_PY([Check PY3 Stream open block], [$HAVE_PYTHON], [$PYTHON3])
diff --git a/tests/test-stream.py b/tests/test-stream.py
new file mode 100644
index 000000000..4a5117501
--- /dev/null
+++ b/tests/test-stream.py
@@ -0,0 +1,32 @@
+# Copyright (c) 2018, Red Hat Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at:
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import sys
+
+import ovs.stream
+
+
+def main(argv):
+    remote = argv[1]
+    err, stream = ovs.stream.Stream.open_block(
+            ovs.stream.Stream.open(remote))
+
+    if err or stream is None:
+        sys.exit(1)
+
+    sys.exit(0)
+
+
+if __name__ == '__main__':
+    main(sys.argv)
-- 
2.17.1



More information about the dev mailing list