On Mon, Jul 9, 2018 at 11:36 PM Ben Pfaff <b...@ovn.org> wrote:

> On Sun, Jul 08, 2018 at 10:05:41PM +0530, nusid...@redhat.com wrote:
> > From: Numan Siddique <nusid...@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 <nusid...@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.
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to