Bug at getsockopt when TCP_NODELAY is used as parameter

Bug at getsockopt when TCP_NODELAY is used as parameter

[jdzstz]

While compiling varnish 3.0.5, I have detected a possible bug at 
getsockopt when TCP_NODELAY is used as parameter. The issue is caused 
because at cygwin it returns a BOOL, instead returning INT value like at 
Linux.

At varnish 3.0.5, the following code at "sock_test(int fd)" uses getsockopt:
    - 
https://www.varnish-cache.org/trac/browser/bin/varnishd/cache_acceptor.c?rev=dec9098a2ac760d7ba4765a9a1a1091103b6eb05

     l = sizeof tcp_nodelay;
     i = getsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &tcp_nodelay, &l);
     if (i) {
         VTCP_Assert(i);
         return;
     }
     assert(l == sizeof tcp_nodelay);
     if (!tcp_nodelay)
         need_tcpnodelay = 1;


The problem is that at cygwin, this ASSERT fails:
     -     assert(l == sizeof tcp_nodelay);

Because before function call, the initial tcp_nodelay variable size is 
INT, but after funtion call, it returns a pointer to a BOOL variable, so 
assert (sizeof INT == sizeof BOOL) fails.

After some investigations I have seen that at windows, its getsockopt 
function, returns BOOL for TCP_NODELAY:
     - 
http://msdn.microsoft.com/en-us/library/windows/desktop/ms738544%28v=vs.85%29.aspx

But at Unix, the correct behaviour is to return INT:
     - http://www.yolinux.com/TUTORIALS/Sockets.html

I suppose that the cygwin getsockopt internally calls to Windows 
getsockopt, and no type conversion of variables is done. I think the 
solution would be to wrap getsockopt response and change it from bool 
size variable to a int size variable.

As a temporary workaround for varnish 3.0.5 I have changed the variable 
types to bool, but I think It would be better to directly fix it at 
cygwin layer, to avoid modifications to original Linux code:

--- origsrc/varnish-3.0.5/bin/varnishd/cache_acceptor.c 2013-12-02 
08:48:15.000000000 +0100
+++ src/varnish-3.0.5/bin/varnishd/cache_acceptor.c 2014-04-17 
21:31:12.555512600 +0200
@@ -36,6 +36,9 @@
  #include <string.h>
  #include <stdlib.h>
  #include <unistd.h>
+#if defined(__CYGWIN__)
+#include <stdbool.h>
+#endif

  #include <sys/uio.h>
  #include <sys/types.h>
@@ -105,7 +108,12 @@ sock_test(int fd)
      struct linger lin;
      struct timeval tv;
      socklen_t l;
-    int i, tcp_nodelay;
+    int i;
+    #if defined(__CYGWIN__)
+    bool tcp_nodelay;
+    #else
+    int tcp_nodelay;
+    #endif

      l = sizeof lin;
      i = getsockopt(fd, SOL_SOCKET, SO_LINGER, &lin, &l);
@@ -172,7 +180,11 @@ VCA_Prep(struct sess *sp)
  {
      char addr[VTCP_ADDRBUFSIZE];
      char port[VTCP_PORTBUFSIZE];
+    #if defined(__CYGWIN__)
+    bool tcp_nodelay = true;
+    #else
      int tcp_nodelay = 1;
+    #endif

      VTCP_name(sp->sockaddr, sp->sockaddrlen,
          addr, sizeof addr, port, sizeof port);


Thank you
Jorge








---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Bug at getsockopt when TCP_NODELAY is used as parameter

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1534 bytes --]

Hi Jorge,

On Apr 27 11:05, jdzstz wrote:
> 
> While compiling varnish 3.0.5, I have detected a possible bug at
> getsockopt when TCP_NODELAY is used as parameter. The issue is
> caused because at cygwin it returns a BOOL, instead returning INT
> value like at Linux.

It's worse.  Officially these socket options return with an optlen ==
sizeof(BOOL) == sizeof(int).  If you look into all MSDN manual pages
related to getsockopt/setsockopt and their supported socket options,
they claim that the size of the options returning a boolean value are
sizeof(BOOL) == 4, or sizeof(DWORD) == 4.  And that's what they did
up to and including Windows XP and Windows 2003.

Only, starting with Windows Vista, getsockopt suddenly returned with
optlen set to 1, which is sizeof(BOOLEAN), the boolean type used by the
underlying kernel.  And this has never been fixed again.

Cygwin already handles this problem by converting the value to a
sizeof(int) value and changing optlen accordingly, but only for the
socket options SOL_SOCKET/SO_KEEPALIVE and SOL_SOCKET/SO_DONTROUTE.

I will fix that at one point, but I have to dig into that a bit more to
see which socket options have to be special-cased, or if this
special-casing can be radically simplified somehow.  Special-casing
dozens of socket options just to tweak the return type isn't exactly
fun.


Corinna

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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: Bug at getsockopt when TCP_NODELAY is used as parameter

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1294 bytes --]

On May  5 14:34, Corinna Vinschen wrote:
> Only, starting with Windows Vista, getsockopt suddenly returned with
> optlen set to 1, which is sizeof(BOOLEAN), the boolean type used by the
> underlying kernel.  And this has never been fixed again.
> 
> Cygwin already handles this problem by converting the value to a
> sizeof(int) value and changing optlen accordingly, but only for the
> socket options SOL_SOCKET/SO_KEEPALIVE and SOL_SOCKET/SO_DONTROUTE.
> 
> I will fix that at one point, but I have to dig into that a bit more to
> see which socket options have to be special-cased, or if this
> special-casing can be radically simplified somehow.  Special-casing
> dozens of socket options just to tweak the return type isn't exactly
> fun.

Funny enough, of all BSD/Linux-compatible options, only three seem to be
affected at all, SOL_SOCKET/SO_KEEPALIVE, SOL_SOCKET/SO_DONTROUTE, and
IPPROTO_TCP/TCP_NODELAY.  I applied a patch which is supposed to fix
this problem, and I just generated a new developer snapshot which is,
as usual, available on http://cygwin.com/snapshots/ Please give it a try.


Thanks,
Corinna

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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: Bug at getsockopt when TCP_NODELAY is used as parameter

[jdzstz]

Hi Corinna,

I have recompiled and tested Varnish Cache 3.0.5 against cygwin snapshot 
2014-05-05 (64 bits), removing my workaround, and now TCP_NODELA​Y 
functionality works fine.

In order to planning the release of fixed varnish 3.0.5, Do you have any 
estimated release date for future Cygwin 1.7.30?

Thank you


---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Bug at getsockopt when TCP_NODELAY is used as parameter

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 830 bytes --]

On May  7 00:44, jdzstz wrote:
> Hi Corinna,
> 
> I have recompiled and tested Varnish Cache 3.0.5 against cygwin
> snapshot 2014-05-05 (64 bits), removing my workaround, and now
> TCP_NODELAY functionality works fine.
> 
> In order to planning the release of fixed varnish 3.0.5, Do you have
> any estimated release date for future Cygwin 1.7.30?

Not yet, no.  The new SAM/AD code(1) needs a bit more testing and at
least one more change(2) and, worst of all, I still have to change the
User's Guide and FAQ to reflect this big change.


Corinna

(1) http://cygwin.com/ml/cygwin/2014-05/txtClll38W8u9.txt
(2) http://cygwin.com/ml/cygwin/2014-05/msg00098.html

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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: Bug at getsockopt when TCP_NODELAY is used as parameter

[jdzstz]

Hi,

Thank you about the information.

Regards,
Jorge Díaz

El 07/05/2014 9:51, Corinna Vinschen escribió:
> On May  7 00:44, jdzstz wrote:
>> Hi Corinna,
>>
>> I have recompiled and tested Varnish Cache 3.0.5 against cygwin
>> snapshot 2014-05-05 (64 bits), removing my workaround, and now
>> TCP_NODELAY functionality works fine.
>>
>> In order to planning the release of fixed varnish 3.0.5, Do you have
>> any estimated release date for future Cygwin 1.7.30?
> Not yet, no.  The new SAM/AD code(1) needs a bit more testing and at
> least one more change(2) and, worst of all, I still have to change the
> User's Guide and FAQ to reflect this big change.
>
>
> Corinna
>
> (1) http://cygwin.com/ml/cygwin/2014-05/txtClll38W8u9.txt
> (2) http://cygwin.com/ml/cygwin/2014-05/msg00098.html
>


---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Bug executing "write" at Cygwin 1.7.31-2

[jdzstz]

Hello,

I am preparing the new version 4.0.1 of Varnish.
I am using a Windows 8.1 machine, with all hotfix installed.

I have updated cygwin to 1.7.31-2 and launch Varnish package tests and 
some failed at "write" function.

After some investigations downloading snapshots, I have verified that:

  * Varnish tests works fine using 20140625 snapshot, downloaded from
    http://cygwin.com/snapshots/x86_64/cygwin1-20140625.dll.xz
  * Varnish tests fails using the next snapshot: 20140707, downloaded
    from http://cygwin.com/snapshots/x86_64/cygwin1-20140707.dll.xz

When installing 20140707 snapshot or 1.7.31-2 release, all the 
problematic tests fails with similar messages:

 1. ---- s1    1.5 Write failed: (212992 vs 1048234) No error
 2. *    s1    1.7 Write failed: (212992 vs 1572908) No error
 3. *    s1    1.7 Write failed: (212992 vs 500009) No error


This message is written by following code, at vtc_http.c file:

    /**********************************************************************
      * Finish and write the vsb to the fd
      */

    static void
    http_write(const struct http *hp, int lvl, const char *pfx)
    {
         ssize_t l;

         AZ(VSB_finish(hp->vsb));
         vtc_dump(hp->vl, lvl, pfx, VSB_data(hp->vsb), VSB_len(hp->vsb));
         l = write(hp->fd, VSB_data(hp->vsb), VSB_len(hp->vsb));
         if (l != VSB_len(hp->vsb))
             vtc_log(hp->vl, hp->fatal, "Write failed: (%zd vs %zd) %s",
                 l, VSB_len(hp->vsb), strerror(errno));
    }



hp->fd is a TCP socket and it seems that "write" function is only 
writing 212992 bytes for some strange reason

I will continue doing some investigations, but it seems that the problem 
is caused by some modification of that 20140707 snapshot: 
https://cygwin.com/snapshots/x86_64/winsup-diffs-20140625-20140707

Thank you,
Jorge




---
This email is free from viruses and malware because avast! Antivirus protection is active.
http://www.avast.com


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Bug executing "write" at Cygwin 1.7.31-2

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1404 bytes --]

On Jul 24 14:22, jdzstz wrote:
> Hello,
> 
> I am preparing the new version 4.0.1 of Varnish.
> I am using a Windows 8.1 machine, with all hotfix installed.
> 
> I have updated cygwin to 1.7.31-2 and launch Varnish package tests and some
> failed at "write" function.
> 
> After some investigations downloading snapshots, I have verified that:
> 
>  * Varnish tests works fine using 20140625 snapshot, downloaded from
>    http://cygwin.com/snapshots/x86_64/cygwin1-20140625.dll.xz
>  * Varnish tests fails using the next snapshot: 20140707, downloaded
>    from http://cygwin.com/snapshots/x86_64/cygwin1-20140707.dll.xz
> 
> When installing 20140707 snapshot or 1.7.31-2 release, all the problematic
> tests fails with similar messages:
> 
> 1. ---- s1    1.5 Write failed: (212992 vs 1048234) No error
> 2. *    s1    1.7 Write failed: (212992 vs 1572908) No error
> 3. *    s1    1.7 Write failed: (212992 vs 500009) No error

Argh.  Sorry about that.  I missed to set a variable under a certain
condition so a subsequent test failed too early.  I prepared a bugfix
and a new snapshot on http://cygwin.com/snapshots/  If it works for
you, I create a bugfix'ed 1.7.31-3 release.

Please report back.


Thanks,
Corinna

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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]

Re: Bug executing "write" at Cygwin 1.7.31-2

[jdzstz]

Hello,

I have executed all tests using varnish snapshot 
(http://cygwin.com/snapshots/x86_64/cygwin1-20140724.dll.xz) and now all 
works fine.

Thank you for providing this quick fix. You can release the 1.7.31-3 to 
solve the issue.

Regards,
Jorge

El 24/07/2014 15:41, Corinna Vinschen escribió:
> On Jul 24 14:22, jdzstz wrote:
>> Hello,
>>
>> I am preparing the new version 4.0.1 of Varnish.
>> I am using a Windows 8.1 machine, with all hotfix installed.
>>
>> I have updated cygwin to 1.7.31-2 and launch Varnish package tests and some
>> failed at "write" function.
>>
>> After some investigations downloading snapshots, I have verified that:
>>
>>   * Varnish tests works fine using 20140625 snapshot, downloaded from
>>     http://cygwin.com/snapshots/x86_64/cygwin1-20140625.dll.xz
>>   * Varnish tests fails using the next snapshot: 20140707, downloaded
>>     from http://cygwin.com/snapshots/x86_64/cygwin1-20140707.dll.xz
>>
>> When installing 20140707 snapshot or 1.7.31-2 release, all the problematic
>> tests fails with similar messages:
>>
>> 1. ---- s1    1.5 Write failed: (212992 vs 1048234) No error
>> 2. *    s1    1.7 Write failed: (212992 vs 1572908) No error
>> 3. *    s1    1.7 Write failed: (212992 vs 500009) No error
> Argh.  Sorry about that.  I missed to set a variable under a certain
> condition so a subsequent test failed too early.  I prepared a bugfix
> and a new snapshot on http://cygwin.com/snapshots/  If it works for
> you, I create a bugfix'ed 1.7.31-3 release.
>
> Please report back.
>
>
> Thanks,
> Corinna
>


--
Problem reports:       http://cygwin.com/problems.html
FAQ:                   http://cygwin.com/faq/
Documentation:         http://cygwin.com/docs.html
Unsubscribe info:      http://cygwin.com/ml/#unsubscribe-simple

Re: Bug executing "write" at Cygwin 1.7.31-2

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 514 bytes --]

On Jul 25 10:46, jdzstz wrote:
> 
> Hello,
> 
> I have executed all tests using varnish snapshot
> (http://cygwin.com/snapshots/x86_64/cygwin1-20140724.dll.xz) and now all
> works fine.
> 
> Thank you for providing this quick fix. You can release the 1.7.31-3 to
> solve the issue.

Thanks for your feedback.  I'm just uploading 1.7.31-3.


Corinna

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

[-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --]