On Mon, Aug 22, 2005 at 03:19:58PM +0100, Joe Orton wrote:
> On Mon, Aug 22, 2005 at 02:52:38PM +0100, Joe Orton wrote:
> > With the FC4 2.6.12-ish kernel I am seeing:
> >
> > - stuff sent by httpd fails to cork properly; partial frames are sent
> > ... i.e. what Greg reported.
> >
> > - stuff sent by your test case *does* get corked properly; the partial
> > header frame is suppressed; using "--cork --nonblock", which *should* be
> > equivalent to what httpd does.
> >
> > so I think there is something subtle here.
>
> It looks like the fact that TCP_NODELAY was ever set to 1 on the socket
> is triggering the problem, even if it is set to 0 at the time TCP_CORK
> is set.
Dave Miller confirmed this and there's a fix working its way through.
Meanwhile...
Since 2.6 can enable both TCP_CORK and TCP_NODELAY at the same time, and
TCP_CORK will take precedence, APR can save two syscalls in the critical
path. Objections?
Index: network_io/unix/sockopt.c
===================================================================
--- network_io/unix/sockopt.c (revision 239389)
+++ network_io/unix/sockopt.c (working copy)
@@ -242,7 +242,13 @@
break;
case APR_TCP_NOPUSH:
#if APR_TCP_NOPUSH_FLAG
+ /* TCP_NODELAY and TCP_CORK are mutually exclusive on Linux
+ * kernels < 2.6; on newer kernels they can be used together
+ * and TCP_CORK takes preference, which is the desired
+ * behaviour. On older kernels, TCP_NODELAY must be toggled
+ * to "off" whilst TCP_CORK is in effect. */
if (apr_is_option_set(sock, APR_TCP_NOPUSH) != on) {
+#ifndef HAVE_TCP_NODELAY_WITH_CORK
int optlevel = IPPROTO_TCP;
int optname = TCP_NODELAY;
@@ -253,11 +259,9 @@
}
#endif
/* OK we're going to change some settings here... */
- /* TCP_NODELAY is mutually exclusive, so do we have it set? */
if (apr_is_option_set(sock, APR_TCP_NODELAY) == 1 && on) {
- /* If we want to set NOPUSH then if we have the TCP_NODELAY
- * flag set we need to switch it off...
- */
+ /* Now toggle TCP_NODELAY to off, if TCP_CORK is being
+ * turned on: */
int tmpflag = 0;
if (setsockopt(sock->socketdes, optlevel, optname,
(void*)&tmpflag, sizeof(int)) == -1) {
@@ -268,13 +272,19 @@
} else if (on) {
apr_set_option(sock, APR_RESET_NODELAY, 0);
}
+#endif /* HAVE_TCP_NODELAY_WITH_CORK */
+
/* OK, now we can just set the TCP_NOPUSH flag accordingly...*/
if (setsockopt(sock->socketdes, IPPROTO_TCP, APR_TCP_NOPUSH_FLAG,
(void*)&on, sizeof(int)) == -1) {
return errno;
}
apr_set_option(sock, APR_TCP_NOPUSH, on);
+#ifndef HAVE_TCP_NODELAY_WITH_CORK
if (!on && apr_is_option_set(sock, APR_RESET_NODELAY)) {
+ /* Now, if turning TCP_CORK was just turned off, turn
+ * TCP_NODELAY back on again if it was earlier toggled
+ * to off: */
int tmpflag = 1;
if (setsockopt(sock->socketdes, optlevel, optname,
(void*)&tmpflag, sizeof(int)) == -1) {
@@ -283,6 +293,7 @@
apr_set_option(sock, APR_RESET_NODELAY,0);
apr_set_option(sock, APR_TCP_NODELAY, 1);
}
+#endif /* HAVE_TCP_NODELAY_WITH_CORK */
}
#else
return APR_ENOTIMPL;
Index: configure.in
===================================================================
--- configure.in (revision 239389)
+++ configure.in (working copy)
@@ -1875,6 +1875,7 @@
APR_CHECK_TCP_NODELAY_INHERITED
APR_CHECK_O_NONBLOCK_INHERITED
+APR_CHECK_TCP_NODELAY_WITH_CORK
# Look for a way of corking TCP...
APR_CHECK_DEFINE(TCP_CORK, netinet/tcp.h)
Index: build/apr_network.m4
===================================================================
--- build/apr_network.m4 (revision 239389)
+++ build/apr_network.m4 (working copy)
@@ -368,6 +368,60 @@
])
dnl
+dnl Determine whether TCP_NODELAY and TCP_CORK can both be set
+dnl on a TCP socket.
+dnl
+AC_DEFUN([APR_CHECK_TCP_NODELAY_WITH_CORK], [
+AC_CACHE_CHECK([whether TCP_NODELAY and TCP_CORK can both be enabled],
+[apr_cv_tcp_nodelay_with_cork],
+[AC_RUN_IFELSE([AC_LANG_PROGRAM([[
+#ifdef HAVE_SYS_TYPES_H
+#include <sys/types.h>
+#endif
+#ifdef HAVE_SYS_SOCKET_H
+#include <sys/socket.h>
+#endif
+#ifdef HAVE_NETINET_IN_H
+#include <netinet/in.h>
+#endif
+#ifdef HAVE_NETINET_TCP_H
+#include <netinet/tcp.h>
+#endif
+#include <stdio.h>
+#include <stdlib.h>
+]], [[
+ int fd, flag, rc;
+
+ fd = socket(AF_INET, SOCK_STREAM, 0);
+ if (fd < 0) {
+ exit(1);
+ }
+
+ flag = 1;
+ rc = setsockopt(fd, IPPROTO_TCP, TCP_NODELAY, &flag, sizeof flag);
+ if (rc < 0) {
+ perror("setsockopt TCP_NODELAY");
+ exit(2);
+ }
+
+ flag = 1;
+ rc = setsockopt(fd, IPPROTO_TCP, TCP_CORK, &flag, sizeof flag);
+ if (rc < 0) {
+ perror("setsockopt TCP_CORK");
+ exit(3);
+ }
+
+ exit(0);
+]])], [apr_cv_tcp_nodelay_with_cork=yes], [apr_cv_tcp_nodelay_with_cork=no])])
+
+if test "$apr_cv_tcp_nodelay_with_cork" = "yes"; then
+ AC_DEFINE([HAVE_TCP_NODELAY_WITH_CORK], 1,
+ [Define if TCP_NODELAY and TCP_CORK can be enabled at the same
time])
+fi
+])
+
+
+dnl
dnl see if O_NONBLOCK setting is inherited from listening sockets
dnl
AC_DEFUN(APR_CHECK_O_NONBLOCK_INHERITED,[
