This is the mail archive of the cygwin-patches mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Limited Internet speeds caused by inappropriate socket buffering in function fdsock (winsup/net.cc)


Hi Daniel,

On Jan  9 15:49, Daniel Havey wrote:
> At Windows we love what you are doing with Cygwin.  However, we have

Windows?

> been getting reports from our hardware vendors that iperf is slow on
> Windows.  Iperf is of course compiled against the cygwin1.dll and we
> believe we have traced the problem down to the function fdsock in
> net.cc.  SO_RCVBUF and SO_SNDBUF are being manually set.  The comments
> indicate that the idea was to increase the buffer size, but, this code
> must have been written long ago because Windows has used autotuning
> for a very long time now.  Please do not manually set SO_RCVBUF or
> SO_SNDBUF as this will limit your internet speed.

Yes, the code has quite a history.

> I am providing a patch, an STC and my cygcheck -svr output.  Hope we
> can fix this.  Please let me know if I can help further.

Yes, perhaps.  Your patch disables setting SO_SNDBUF/SO_RCVBUF, but it
keeps the values for wmem/rmem intact.

rmem is basically unused, but wmem is used in fhandler_socket::send_internal
to account for KB 823764 and another weird behaviour we observed long ago:

If an application sends data in chunks > SO_SNDBUF, the OS doesn't just
fill up the send buffer, but instead it will allocate a temporary buffer
big enough to copy over the application buffer.  So if the application
sends data inb 2 Meg chunks, every send will allocate another 2 Megs and
copy the data, which wastes time and space.

As far as I remember, this is still a problem in Vista and later.  Can
you confirm or deny this by any chance?

And, do we have something like an ideal splitting point?  Given that
SO_SNDBUF/SO_RCVBUF isn't set anymore, we could set wmem to some
arbitrary, yet useful value on both targets, 32 and 64 bit.  I think,
keeping wmem < 64K on 32 bit should be better,

> If you want to duplicate the STC you will have to find an iperf server
> (I found an extreme case) that has a large enough RTT distance from
> you and try a few times.  I get varying results depending on Internet
> traffic but without the patch never exceed the limit caused by the
> buffering.

I can confirm the observation.  I have an iperf server with an avg
RTT of 155ms, and without your patch I hit the upper ceiling at
10.4 Mbit/s, while with your patch I get up to 23 Mbit/s.

> Here is the patch:

As for your patch, a few minor points:

- Can you please send a BSD sign-off per https://cygwin.com/contrib.html?
  For the text see
  https://cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/CONTRIBUTORS

- Please keep the line length <= 80 chars and remove unnecessary changes
  (e. g. adding empty lines).

- The now unused code should be put into `#if 0' bracketing, rather than
  in a comment. Move the `int size;' declaration down so it will be inside
  the same `#if 0' bracket.

- The preceeding comment shows that the code has a lot of history.  I
  think it would be prudent to add your comment as NOTE 4 into the same
  comment, so the history is in one place.

- Not a requirement, but it would be nice to get the patch in the
  typical `git format-patch' fashion.


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: signature.asc
Description: PGP signature


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]