On Sat, Aug 4, 2018 at 3:20 AM Ben Pfaff <[email protected]> wrote: > On Thu, Jul 12, 2018 at 01:37:33AM +0530, Numan Siddique wrote: > > On Wed, Jul 11, 2018 at 10:26 PM Ben Pfaff <[email protected]> wrote: > > > > > On Wed, Jul 11, 2018 at 12:37:28PM +0530, Numan Siddique wrote: > > > > On Wed, Jul 11, 2018 at 2:08 AM Ben Pfaff <[email protected]> wrote: > > > > > > > > > On Wed, Jul 11, 2018 at 12:56:39AM +0530, [email protected] > wrote: > > > > > > From: Numan Siddique <[email protected]> > > > > > > > > > > > > The python function ovs.socket_util.check_connection_completion() > > > uses > > > > > select() > > > > > > (provided by python) to monitor the socket file descriptor. The > > > select() > > > > > > returns 1 when the file descriptor becomes ready. For error cases > > > like - > > > > > > 111 (Connection refused) and 113 (No route to host) (POLLERR), > > > > > ovs.poller._SelectSelect.poll() > > > > > > expects the exceptfds list to be set by select(). But that is > not the > > > > > case. > > > > > > As per the select() man page, writefds list will be set for > POLLERR. > > > > > > Please see "Correspondence between select() and poll() > notifications" > > > > > section of select(2) > > > > > > man page. > > > > > > > > > > > > Because of this behavior, > > > ovs.socket_util.check_connection_completion() > > > > > returns success > > > > > > even if the remote is unreachable or not listening on the port. > > > > > > > > > > > > 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]> > > > > > > > > > > Can we just code check_connection_completion() like we do in C, by > > > > > directly using select() on Windows and poll() everywhere else? The > > > > > approach in this patch seems a little indirect to me. > > > > > > > > > > > > > > Thanks for the review. As per the comments here - > > > > > https://github.com/openvswitch/ovs/blob/master/python/ovs/poller.py#L51 > > > > ***** > > > > # eventlet/gevent doesn't support select.poll. If select.poll is > used, > > > > # python interpreter is blocked as a whole instead of switching from > the > > > > # current thread that is about to block to other runnable thread. > > > > # So emulate select.poll by select.select because using python means > that > > > > # performance isn't so important. > > > > ******* > > > > > > > > we cannot use poll() in python. I tried with this patch to test it > out - > > > > https://paste.fedoraproject.org/paste/YUhgVte-BOjgid-ojmHZJw > > > > All the ovs python idl tests pass. But it doesn't work with openstack > > > > networking-ovn. The whole neutron-server process just blocks. > > > > > > > > I don't see any other way for python. Once select() returns we have > to > > > use > > > > some mechanism to get the error code. > > > > Does calling sock.connect_ex() bother you ? > > > > > > I don't mean to change what poller.py does. I mean to implement > > > ovs.socket_util.check_connection_completion in terms of select.poll and > > > select.select directly. The comment in python/ovs/poller.py should not > > > be relevant because the use of select.poll in > > > check_connection_completion would never block (it would use a timeout > of > > > 0). > > > > > > > > Hi Ben, > > > > For python applications which use eventlet, select.poll/epoll is not > > available. 'select ' python module > > is monkey patched and poll and few other functions are deleted. > > > > https://github.com/eventlet/eventlet/blob/master/NEWS#L155 > > > https://github.com/eventlet/eventlet/blob/master/eventlet/green/select.py#L9 > > > https://github.com/eventlet/eventlet/blob/master/eventlet/patcher.py#L305 > > It looks like it's possible to get the original function. I think that > it is a reasonable choice in this case: > http://www.gevent.org/api/gevent.monkey.html#gevent.monkey.get_original
Thanks for pointing this out. I have submitted v4 as per your suggestion - https://patchwork.ozlabs.org/project/openvswitch/list/?series=59666 Thanks Numan _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
