From patchwork Tue Aug 7 11:37:58 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Numan Siddique X-Patchwork-Id: 954486 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41lCFG1Mpdz9ryt for ; Tue, 7 Aug 2018 21:38:10 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 0BDA4BD4; Tue, 7 Aug 2018 11:38:08 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 472019D3 for ; Tue, 7 Aug 2018 11:38:07 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 68B307A4 for ; Tue, 7 Aug 2018 11:38:06 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 952BC40216E4 for ; Tue, 7 Aug 2018 11:38:05 +0000 (UTC) Received: from nusiddiq.redhat (ovpn-117-27.sin2.redhat.com [10.67.117.27]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0DC692026D66; Tue, 7 Aug 2018 11:38:03 +0000 (UTC) From: nusiddiq@redhat.com To: dev@openvswitch.org Date: Tue, 7 Aug 2018 17:07:58 +0530 Message-Id: <20180807113758.3159-1-nusiddiq@redhat.com> In-Reply-To: <20180807113719.2959-1-nusiddiq@redhat.com> References: <20180807113719.2959-1-nusiddiq@redhat.com> X-Scanned-By: MIMEDefang 2.78 on 10.11.54.4 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 07 Aug 2018 11:38:05 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Tue, 07 Aug 2018 11:38:05 +0000 (UTC) for IP:'10.11.54.4' DOMAIN:'int-mx04.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'nusiddiq@redhat.com' RCPT:'' X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH v4 1/2] ovs python: ovs.stream.open_block() returns success even if the remote is unreachable X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org From: Numan Siddique The python function ovs.socket_util.check_connection_completion() uses select() (provided by python) to monitor the socket file descriptor. The select() returns 1 when the file descriptor becomes ready. For error cases like - 111 (Connection refused) and 113 (No route to host) (POLLERR), ovs.poller._SelectSelect.poll() expects the exceptfds list to be set by select(). But that is not the case. As per the select() man page, writefds list will be set for POLLERR. Please see "Correspondence between select() and poll() notifications" section of select(2) man page. Because of this behavior, ovs.socket_util.check_connection_completion() returns success even if the remote is unreachable or not listening on the port. This patch fixes this issue by using poll() to check the connection status similar to the C implementation of check_connection_completion(). A new function 'get_system_poll() is added in ovs/poller.py which returns the select.poll() object. If select.poll is monkey patched by eventlet/gevent, it gets the original select.poll() and returns it. The test cases added in this patch fails without the fix. Suggested-by: Ben Pfaff Signed-off-by: Numan Siddique --- python/ovs/poller.py | 34 ++++++++++++++++++++++++++++++++-- python/ovs/socket_util.py | 5 +++-- python/ovs/stream.py | 11 +++++++++-- tests/automake.mk | 1 + tests/ovsdb-idl.at | 16 ++++++++++++++++ tests/test-stream.py | 32 ++++++++++++++++++++++++++++++++ 6 files changed, 93 insertions(+), 6 deletions(-) create mode 100644 tests/test-stream.py diff --git a/python/ovs/poller.py b/python/ovs/poller.py index 2f3fcf9b6..ef67e6763 100644 --- a/python/ovs/poller.py +++ b/python/ovs/poller.py @@ -31,14 +31,21 @@ except ImportError: SSL = None try: - import eventlet.patcher + from eventlet import patcher as eventlet_patcher def _using_eventlet_green_select(): - return eventlet.patcher.is_monkey_patched(select) + return eventlet_patcher.is_monkey_patched(select) except: + eventlet_patcher = None def _using_eventlet_green_select(): return False +try: + from gevent import monkey as gevent_monkey +except: + gevent_monkey = None + + vlog = ovs.vlog.Vlog("poller") POLLIN = 0x001 @@ -257,3 +264,26 @@ class Poller(object): def __reset(self): self.poll = SelectPoll() self.timeout = -1 + + +""" +Returns the original select.poll() object. If select.poll is monkey patched +by eventlet or gevent library, it gets the original select.poll and returns +an object of it using the eventlet.patcher.original/gevent.monkey.get_original +functions. + +As a last resort, if there is any exception it returns the SelectPoll() object. +""" +def get_system_poll(): + try: + if _using_eventlet_green_select(): + _system_poll = eventlet_patcher.original("select").poll + elif gevent_monkey and gevent_monkey.is_object_patched( + 'select', 'poll'): + _system_poll = gevent_monkey.get_original('select', 'poll') + else: + _system_poll = select.poll + except: + _system_poll = SelectPoll + + return _system_poll() diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py index 403104936..8e582fe91 100644 --- a/python/ovs/socket_util.py +++ b/python/ovs/socket_util.py @@ -162,8 +162,8 @@ def make_unix_socket(style, nonblock, bind_path, connect_path, short=False): def check_connection_completion(sock): - p = ovs.poller.SelectPoll() if sys.platform == "win32": + p = ovs.poller.SelectPoll() event = winutils.get_new_event(None, False, True, None) # Receive notification of readiness for writing, of completed # connection or multipoint join operation, and of socket closure. @@ -173,6 +173,7 @@ def check_connection_completion(sock): win32file.FD_CLOSE) p.register(event, ovs.poller.POLLOUT) else: + p = ovs.poller.get_system_poll() p.register(sock, ovs.poller.POLLOUT) pfds = p.poll(0) if len(pfds) == 1: @@ -180,7 +181,7 @@ def check_connection_completion(sock): if revents & ovs.poller.POLLERR: try: # The following should raise an exception. - socket.send("\0", socket.MSG_DONTWAIT) + sock.send("\0".encode(), socket.MSG_DONTWAIT) # (Here's where we end up if it didn't.) # XXX rate-limit diff --git a/python/ovs/stream.py b/python/ovs/stream.py index 5996497a5..ca0d84425 100644 --- a/python/ovs/stream.py +++ b/python/ovs/stream.py @@ -191,8 +191,15 @@ 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(sock) + 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): 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 014382850..e8a26e971 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)