On 05/23/14 at 08:47am, Ben Pfaff wrote: > On Fri, May 23, 2014 at 01:15:58AM +0200, Thomas Graf wrote: > > Signed-off-by: Thomas Graf <[email protected]> > > nl_sock_recv__() compares its 'events' member against EPOLLERR with ==, > but EPOLL* are bitmasks so should it use & to test for a single bit? > Actually I'm not sure what the 'events' argument means. The comments > don't appear to fully explain it, the code only interprets EPOLLERR, and > the only effect of EPOLLERR is to fall back to linear receive.
The current implementation matches == EPOLLERR because the kernel currently gurantees that EPOLLERR is mutual exclusive with any other bit for mmap netlink on the receive side. It's very fragile though and will need a more generic implementation. However, how to handle EPOLLIN | EPOLLERR? It would mean that data is on the ring but recvmsg() needs to be invoked to retrieve the error condition. Do we discard the data and return the error? Do we ignore the error? This is partly undefined API right now and needs to be clearly defined. > nl_sock_recv() and nl_sock_recv_events() must be thread-safe (given > independent buffers) but it appears to me that nl_sock_recv_mmap() does > not do anything to ensure that a single frame is consumed only once; I > don't see any synchronization around the rx_ring pointer. Absolutely. It currently relies on a individual socket per thread. If that cannot be guaranteed anymore we'll require synchronization. > nl_sock_recv_mmap() delegates to nl_sock_recv_linear() in the > NL_MMAP_STATUS_COPY case, but it treats the nl_sock_recv_linear() return > value as if a negative value indicated error, which it doesn't (it never > returns negative). Also upon error from nl_sock_recv_linear() in that > case one wonders whether that frame should be skipped (could we > otherwise end up looping forever on this frame?). Good catch. The (retval < 0) is dead code right now. I'm not sure that I've encountered a case where a linear receive would return an error. Socket errors are handled via EPOLLERR and would never go through nl_sock_recv_mmap(). I'll need to dig through the kernel code and come up with proper documentation on what to do if a requested linear receive fails. We definitely need to skip the frame at some point but we might be able to retry for some error codes apart from EINTR. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
