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

Numan Siddique nusiddiq at redhat.com
Tue Jul 10 17:58:16 UTC 2018


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:
127.0.0.1:6641
and there is no server listening on 6641, poll() returns 1 and POLLERR bit
is
set in struct pollfd's revents (
https://github.com/openvswitch/ovs/blob/master/lib/socket-util.c#L291)

In python implementation, select() is used. For the same remote "tcp:
127.0.0.1:6641",
select() is returning 1, but 'ovs.poller.POLLERR' is not set in revents
(
https://github.com/openvswitch/ovs/blob/master/python/ovs/socket_util.py#L180
).
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)
     else:
         p.register(sock, ovs.poller.POLLOUT)
+        p.register(sock, ovs.poller.POLLERR)
+
     pfds = p.poll(0)
     if len(pfds) == 1:
         revents = pfds[0][1]
*****

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 |
                               POLLERR)
                              /* 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
POLLPRI.

Thanks
Numan

Thanks,
>
> Ben.
>


More information about the dev mailing list