[ovs-dev] [PATCH] stream-tcp: Call setsockopt TCP_NODELAY after TCP is connected for Windows.

[Gurucharan Shetty]

On Tue, Oct 7, 2014 at 2:25 PM, Ben Pfaff <blp at nicira.com> wrote:
> On Thu, Oct 02, 2014 at 08:16:56AM -0700, Gurucharan Shetty wrote:
>> On Windows platform, TCP_NODELAY can only be set when TCP is established.
>> The current code does not create any problems while running unit tests
>> (because connections get established immediately) but is observed while
>> connecting to a different machine.
>>
>> commit 8b76839(Move setsockopt TCP_NODELAY to when TCP is connected.)
>> made changes to call setsockopt with TCP_NODELAY after TCP is connected
>> only in lib/stream-ssl.c. We need the same change for stream-tcp too.
>>
>> Signed-off-by: Gurucharan Shetty <gshetty at nicira.com>
>
> I guess we will need a similar change for stream-ssl.c.
As noted in the commit message, stream-ssl already has the change.
stream-tcp was missing it.
>
> I'm not a fan of unnecessary platform-related differences.  That makes
> me think that it might be better to set TCP_NODELAY only after
> connecting on every platform.  After all, there is no benefit, except
> simplicity, to setting it before one can actually send any data, and
> we cannot get the simplicity benefit anyway.
>
> The one complication is that on Windows, stream-fd is only used for
> TCP sockets, whereas on Unix it is used for Unix domain sockets and
> TCP sockets, and one cannot set TCP_NODELAY on a non-TCP socket.  I
> guess that means that we'd need a new parameter to new_fd_stream() to
> indicate whether it is a TCP socket.  That's probably OK though.
>
> Looking at a diff, the differences between the two versions of
> stream-fd are not large, and some of the differences are unnecessary.
> Maybe this change is an argument toward re-unification.
I agree. As a first step, I will send in a patch that merges
stream-fd-windows and stream-fd-unix. Once that looks fine, I will
resping this patch.

Thanks!