This patch is now part of this series - https://patchwork.ozlabs.org/project/openvswitch/list/?series=54336
Numan On Tue, Jul 3, 2018 at 12:21 AM <[email protected]> wrote: > From: Numan Siddique <[email protected]> > > 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 <[email protected]> > --- > 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 > > v1 -> v2 - Fixed the compilation issue > > 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 d6c447a97..617f31966 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 > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
