[ovs-dev] [PATCH v3] [python] Don't mix system poll/monkeypatched select

[Numan Siddique]

On Fri, Jun 11, 2021 at 10:30 AM Terry Wilson <twilson at redhat.com> wrote:
>
> This is a partial revert of c1aa16d19, but keeps its test.
>
> Applications that use eventlet cannot use poll() without blocking.
> To fix an issue where the check_connection_completion() code would
> not detect connection errors when using select(), we switched to
> using the system poll() for checking the connection while leaving
> the select-based poller the default. This mixes monkeypatched and
> unpatched code and caused eventlet-based code to block every time
> we connect.
>
> According to the connect(2) man page, you can detect connect()
> errors when using select():
>
>   After select(2) indicates writability, use getsockopt(2) to read
>   the SO_ERROR option at level SOL_SOCKET to determine whether
>   connect() completed successfully (SO_ERROR is zero) or
>   unsuccessfully (SO_ERROR is one of the usual error codes listed
>   here, explaining the reason for the failure)
>
> so this patch does that instead.
>
> Signed-off-by: Terry Wilson <twilson at redhat.com>

Thanks for addressing this.

LGTM

Acked-by: Numan Siddique <numans at ovn.org>

Thanks
Numan



> ---
>  python/ovs/poller.py      | 35 ++---------------------------------
>  python/ovs/socket_util.py |  9 ++++++---
>  2 files changed, 8 insertions(+), 36 deletions(-)
>
> diff --git a/python/ovs/poller.py b/python/ovs/poller.py
> index 3624ec865..7b3238d6d 100644
> --- a/python/ovs/poller.py
> +++ b/python/ovs/poller.py
> @@ -31,22 +31,14 @@ except ImportError:
>      SSL = None
>
>  try:
> -    from eventlet import patcher as eventlet_patcher
> +    import 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
> @@ -265,26 +257,3 @@ class Poller(object):
>      def __reset(self):
>          self.poll = SelectPoll()
>          self.timeout = -1
> -
> -
> -def get_system_poll():
> -    """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.
> -    """
> -    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 3faa64e9d..7dd000916 100644
> --- a/python/ovs/socket_util.py
> +++ b/python/ovs/socket_util.py
> @@ -159,8 +159,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.
> @@ -170,12 +170,15 @@ 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:
>          revents = pfds[0][1]
> -        if revents & ovs.poller.POLLERR or revents & ovs.poller.POLLHUP:
> +        if revents & ovs.poller.POLLOUT:
> +            # This is how select() indicates a connect() error
> +            return sock.getsockopt(socket.SOL_SOCKET, socket.SO_ERROR)
> +        elif revents & ovs.poller.POLLERR or revents & ovs.poller.POLLHUP:
> +            # This is how poll() indicates a connect() error
>              try:
>                  # The following should raise an exception.
>                  sock.send("\0".encode(), socket.MSG_DONTWAIT)
> --
> 2.26.3
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>