[ovs-dev] [PATCH 2/8] stream: Allow timeout configuration for open_block.

[Ben Pfaff]

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 <i.maximets at samsung.com>

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.

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.

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.

Thanks for humoring me on these petty details!

Ben