[ovs-dev] [PATCH v2 1/2] ovs python: ovs.stream.open_block() returns success even if the remote is unreachable
blp at ovn.org
Tue Jul 10 20:21:26 UTC 2018
On Tue, Jul 10, 2018 at 11:28:16PM +0530, Numan Siddique wrote:
> On Mon, Jul 9, 2018 at 11:36 PM Ben Pfaff <blp at ovn.org> wrote:
> > On Sun, Jul 08, 2018 at 10:05:41PM +0530, 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>
> > I don't understand the problem here. I mean, I believe when you say
> > there is a problem, but the cause doesn't really make sense to me. The
> > code for check_connection_completion in socket_util.py looks correct to
> > me and equivalent to the C implementation in socket-util.c. Do you have
> > an idea of why it doesn't work properly? (Is it somehow specific to
> > Python?)
> > I don't think we have an equivalent test for the C version. Does it
> > pass, or does it need a similar change?
> The C implementation works fine. The function stream_open_block() returns 0
> when connected and a proper errno when it fails. I see the problem only in
> python implementation.
> In C implementation, poll() is used. Suppose if I give remote as tcp:
> and there is no server listening on 6641, poll() returns 1 and POLLERR bit
> set in struct pollfd's revents (
> In python implementation, select() is used. For the same remote "tcp:
> select() is returning 1, but 'ovs.poller.POLLERR' is not set in revents
> So the python function check_connection_completion() returns 0, which is
> false positive.
> I also tested by adding below line in check_connection_completion() but
> still I see the
> same behavior.
> diff --git a/python/ovs/socket_util.py b/python/ovs/socket_util.py
> index 91f4532ea..01dc0af2e 100644
> --- a/python/ovs/socket_util.py
> +++ b/python/ovs/socket_util.py
> @@ -174,6 +174,8 @@ def check_connection_completion(sock):
> p.register(event, ovs.poller.POLLOUT)
> p.register(sock, ovs.poller.POLLOUT)
> + p.register(sock, ovs.poller.POLLERR)
> pfds = p.poll(0)
> if len(pfds) == 1:
> revents = pfds
> As per the select(2) man page,
> Correspondence between select() and poll() notifications
> Within the Linux kernel source, we find the following
> definitions which show the correspondence between the readable, writable,
> and exceptional condition notifications of select() and the event
> notifications provided by poll(2) (and epoll(7)):
> #define POLLIN_SET (POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP |
> /* Ready for reading */
> #define POLLOUT_SET (POLLWRBAND | POLLWRNORM | POLLOUT | POLLERR)
> /* Ready for writing */
> #define POLLEX_SET (POLLPRI)
> /* Exceptional condition */
> To detect POLLERR, the python implementation is looking into the exceptfds
> list returned by select. But looks like exceptfds list will be set only for
OK. I didn't know about that difference between select() and poll().
Should we just make check_connection_completion() use select.poll()
directly (except on Windows)?
More information about the dev