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

Numan Siddique nusiddiq at redhat.com
Sun Jul 8 16:38:20 UTC 2018


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 <nusiddiq at redhat.com> wrote:

> 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
>
> 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
>
>


More information about the dev mailing list