On 13 January 2014 14:55, Ben Pfaff <b...@nicira.com> wrote: > > Spurious trailing '-' just above. >
Will fix. 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. > Right, this was somewhat intentional (although quite possibly flawed). In cases of socket errors, it was expected that these would prevent the final nl_msg from being received, so EOF wouldn't overwrite. In the case of EPROTO, this suggests that a malformed nl_msg is observed in one thread, and a separate thread has successfully received the final, correctly-formatted nl_msgs, and processed through to the end of them. I didn't previously expect that this would occur. By "use two variables", do you mean one for storing the EOF status (error denoting success), and one for all other errors (failures)? 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 > That does sound a bit tidier. There's a few complications with the nl_dump_recv() call: If the socket returns EINTR, nl_dump_recv() reinterprets this as EAGAIN. It will also return EAGAIN if it receives an unexpected nl_msg (wrong seq). As I understand, we should actually retry if we encounter these cases. However, if we receive an EAGAIN from the socket then this is a case to fail out and return. Is that in line with what you're thinking?
_______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
