On 27.12.2018 20:34, Ben Pfaff wrote: > On Wed, Dec 26, 2018 at 06:23:55PM +0300, Ilya Maximets wrote: >> On some systems in case where remote is not responding, socket could >> remain in SYN_SENT state for a really long time without errors waiting >> for connection. This leads to situations where open_blok() hangs for >> a few minutes waiting for connection to the DOWN remote. >> >> For example, our "multiple remotes" idl tests hangs waiting for >> connection to the WRONG_PORT on FreeBSD in CirrusCI environment. >> This leads to test failures because Alarm signal arrives much faster >> than ETIMEDOUT from the socket. >> >> This patch allowes to specify timeout value for 'open_block' function. >> If the connection takes more time, socket will be closed with >> ETIMEDOUT error code. Negative value or None in python could be >> used to wait infinitely. >> >> Signed-off-by: Ilya Maximets <[email protected]> > > Thanks for the improvements! > > Would you mind moving the 'timeout' parameter to stream_open_block() to > the middle of the parameter list? Conventionally we put output > parameters like streamp at the end of a list of parameters, although you > could probably cite a hundred places where we haven't done it right.
I fully agree that output arguments should go last. Thanks for pointing that. > > In stream_open_block(), would you mind using a 'deadline' of LLONG_MAX > to represent "no timeout", instead of -1? It's annoying but some system > might start the monotonic clock out at a negative value (for the > real-time clock, I've seen this on misconfigured systems, and I don't > want to take the risk) and the symptoms of the problem would be really > difficult to debug. Interesting fact. Thanks. > > Similarly, in the Python code there's a risk that 'deadline' would end > up 0 if ovs.timeval.msec() is negative, and then we'd skip the timeout > for that one pathological case, and so I'd prefer to see "if deadline is > not None" instead of "if deadline" there. Sure. > > Thanks for humoring me on these petty details! > > Ben > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
