On Tue, Jul 10, 2018 at 11:28:16PM +0530, Numan Siddique wrote: > On Mon, Jul 9, 2018 at 11:36 PM Ben Pfaff <[email protected]> wrote: > > > On Sun, Jul 08, 2018 at 10:05:41PM +0530, [email protected] wrote: > > > From: Numan Siddique <[email protected]> > > > > > > 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 <[email protected]> > > > > 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.
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)? _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
