On Fri, Jan 10, 2014 at 11:43:11AM -0800, Joe Stringer wrote: > This patch modifies 'struct nl_dump' and nl_dump_next() to allow > multiple threads to share the same nl_dump. These changes are targeted > around synchronizing buffer status between multiple callers, and > allowing callers to fully process their existing buffers before > determining whether to stop fetching flows. > > The lifecycle of 'dump->status' is as follows:-
Spurious trailing '-' just above. I have two concerns about nl_dump_next(). The first is that any nonzero status always overwrites the existing status. This means that EOF in one thread overwrites EPROTO (or some other error) from another thread, effectively dropping the error. I can't think, off-hand, of a good way to avoid this without two variables, so maybe we should use two variables. The second is that ignoring EAGAIN in the loop makes it possible for threads (other than the one thread that receives NLMSG_DONE) to spin in essentially a no-op loop waiting for the thread that receives NLMSG_DONE to set dump->status. I think we can avoid that by treating EAGAIN as a reason to return false; I'm pretty sure that the kernel netlink code will never return EAGAIN while a dump is in progress. Thanks, Ben. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
