[PATCH] Fix nopush cleanup for SPDY

Yury Kirpichev ykirpichev at gmail.com
Tue Jun 11 09:00:42 UTC 2013


Hi,

Could you please take a look at patch below.
I've tried to fix problem that TCP_CORK option is not cleaned in SPDY case.

# HG changeset patch
# User ykirpichev at gmail.com
# Date 1370939502 -14400
# Branch nopush_fix_1
# Node ID 58d7a76b975ed7afb6a980b8810051a10dfc96f4
# Parent  725fb71ab1a60bd48b0afb8b001b5349f5054cb1
Fix tcp_nopush cleanup for spdy

diff -r 725fb71ab1a6 -r 58d7a76b975e src/http/ngx_http.c
--- a/src/http/ngx_http.c Fri Jun 07 13:16:00 2013 -0700
+++ b/src/http/ngx_http.c Tue Jun 11 12:31:42 2013 +0400
@@ -2106,3 +2106,112 @@

     return NGX_OK;
 }
+
+
+ngx_int_t
+ngx_http_check_and_set_nopush(ngx_connection_t* c)
+{
+    int tcp_nodelay;
+
+    /* the TCP_CORK and TCP_NODELAY are mutually exclusive */
+    if (c->tcp_nodelay == NGX_TCP_NODELAY_SET) {
+
+        tcp_nodelay = 0;
+
+        if (setsockopt(c->fd, IPPROTO_TCP, TCP_NODELAY,
+                       (const void *) &tcp_nodelay, sizeof(int)) == -1)
+        {
+            /*
+             * there is a tiny chance to be interrupted, however,
+             * we continue a processing with the TCP_NODELAY
+             * and without the TCP_CORK
+             */
+
+            if (ngx_errno != NGX_EINTR) {
+                ngx_connection_error(c, ngx_errno,
+                                     "setsockopt(TCP_NODELAY) failed");
+                return NGX_ERROR;
+            }
+
+        } else {
+            c->tcp_nodelay = NGX_TCP_NODELAY_UNSET;
+
+            ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
+                           "no tcp_nodelay");
+        }
+    }
+
+    if (c->tcp_nodelay == NGX_TCP_NODELAY_UNSET) {
+
+        if (ngx_tcp_nopush(c->fd) == NGX_ERROR) {
+
+            /*
+             * there is a tiny chance to be interrupted, however,
+             * we continue a processing without the TCP_CORK
+             */
+
+            if (ngx_errno != NGX_EINTR) {
+                ngx_connection_error(c, ngx_errno,
+                                     ngx_tcp_nopush_n " failed");
+                return NGX_ERROR;
+            }
+
+        } else {
+            c->tcp_nopush = NGX_TCP_NOPUSH_SET;
+
+            ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
+                           "tcp_nopush");
+        }
+    }
+
+    return NGX_OK;
+}
+
+ngx_int_t
+ngx_http_check_and_restore_nopush(ngx_connection_t* c,
+                                  ngx_http_core_loc_conf_t* clcf)
+{
+    int tcp_nodelay;
+
+    if (c->tcp_nopush == NGX_TCP_NOPUSH_SET) {
+        if (ngx_tcp_push(c->fd) == -1) {
+            ngx_connection_error(c, ngx_socket_errno, ngx_tcp_push_n "
failed");
+            return NGX_ERROR;
+        }
+
+        c->tcp_nopush = NGX_TCP_NOPUSH_UNSET;
+        tcp_nodelay = ngx_tcp_nodelay_and_tcp_nopush ? 1 : 0;
+
+    } else {
+        tcp_nodelay = 1;
+    }
+#if 1
+    if (tcp_nodelay
+        && clcf->tcp_nodelay
+        && c->tcp_nodelay == NGX_TCP_NODELAY_UNSET)
+    {
+        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "tcp_nodelay");
+
+        if (setsockopt(c->fd, IPPROTO_TCP, TCP_NODELAY,
+                       (const void *) &tcp_nodelay, sizeof(int))
+            == -1)
+        {
+#if (NGX_SOLARIS)
+            /* Solaris returns EINVAL if a socket has been shut down */
+            c->log_error = NGX_ERROR_IGNORE_EINVAL;
+#endif
+
+            ngx_connection_error(c, ngx_socket_errno,
+                                 "setsockopt(TCP_NODELAY) failed");
+
+            c->log_error = NGX_ERROR_INFO;
+            return NGX_ERROR;
+        }
+
+        c->tcp_nodelay = NGX_TCP_NODELAY_SET;
+    }
+#endif
+
+    return NGX_OK;
+}
+
diff -r 725fb71ab1a6 -r 58d7a76b975e src/http/ngx_http.h
--- a/src/http/ngx_http.h Fri Jun 07 13:16:00 2013 -0700
+++ b/src/http/ngx_http.h Tue Jun 11 12:31:42 2013 +0400
@@ -180,5 +180,9 @@
 extern ngx_http_output_header_filter_pt  ngx_http_top_header_filter;
 extern ngx_http_output_body_filter_pt    ngx_http_top_body_filter;

+ngx_int_t ngx_http_check_and_set_nopush(ngx_connection_t* c);
+
+ngx_int_t ngx_http_check_and_restore_nopush(ngx_connection_t* c,
+                                            ngx_http_core_loc_conf_t*
clcf);

 #endif /* _NGX_HTTP_H_INCLUDED_ */
diff -r 725fb71ab1a6 -r 58d7a76b975e src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c Fri Jun 07 13:16:00 2013 -0700
+++ b/src/http/ngx_http_request.c Tue Jun 11 12:31:42 2013 +0400
@@ -2740,7 +2740,6 @@
 static void
 ngx_http_set_keepalive(ngx_http_request_t *r)
 {
-    int                        tcp_nodelay;
     ngx_int_t                  i;
     ngx_buf_t                 *b, *f;
     ngx_event_t               *rev, *wev;
@@ -2913,44 +2912,9 @@

     c->log->action = "keepalive";

-    if (c->tcp_nopush == NGX_TCP_NOPUSH_SET) {
-        if (ngx_tcp_push(c->fd) == -1) {
-            ngx_connection_error(c, ngx_socket_errno, ngx_tcp_push_n "
failed");
-            ngx_http_close_connection(c);
-            return;
-        }
-
-        c->tcp_nopush = NGX_TCP_NOPUSH_UNSET;
-        tcp_nodelay = ngx_tcp_nodelay_and_tcp_nopush ? 1 : 0;
-
-    } else {
-        tcp_nodelay = 1;
-    }
-
-    if (tcp_nodelay
-        && clcf->tcp_nodelay
-        && c->tcp_nodelay == NGX_TCP_NODELAY_UNSET)
-    {
-        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "tcp_nodelay");
-
-        if (setsockopt(c->fd, IPPROTO_TCP, TCP_NODELAY,
-                       (const void *) &tcp_nodelay, sizeof(int))
-            == -1)
-        {
-#if (NGX_SOLARIS)
-            /* Solaris returns EINVAL if a socket has been shut down */
-            c->log_error = NGX_ERROR_IGNORE_EINVAL;
-#endif
-
-            ngx_connection_error(c, ngx_socket_errno,
-                                 "setsockopt(TCP_NODELAY) failed");
-
-            c->log_error = NGX_ERROR_INFO;
-            ngx_http_close_connection(c);
-            return;
-        }
-
-        c->tcp_nodelay = NGX_TCP_NODELAY_SET;
+    if (ngx_http_check_and_restore_nopush(c, clcf) != NGX_OK) {
+        ngx_http_close_connection(c);
+        return;
     }

 #if 0
diff -r 725fb71ab1a6 -r 58d7a76b975e src/http/ngx_http_spdy.c
--- a/src/http/ngx_http_spdy.c Fri Jun 07 13:16:00 2013 -0700
+++ b/src/http/ngx_http_spdy.c Tue Jun 11 12:31:42 2013 +0400
@@ -450,7 +450,6 @@
     ngx_http_spdy_handle_connection(sc);
 }

-
 ngx_int_t
 ngx_http_spdy_send_output_queue(ngx_http_spdy_connection_t *sc)
 {
@@ -490,6 +489,13 @@
     }

     cl = c->send_chain(c, cl, 0);
+    clcf = ngx_http_get_module_loc_conf(sc->http_connection->conf_ctx,
+                                        ngx_http_core_module);
+
+    if (ngx_http_check_and_restore_nopush(c, clcf) != NGX_OK) {
+        c->error = 1;
+        return NGX_ERROR;
+    }

     if (cl == NGX_CHAIN_ERROR) {
         c->error = 1;
@@ -501,8 +507,6 @@
         return NGX_ERROR;
     }

-    clcf = ngx_http_get_module_loc_conf(sc->http_connection->conf_ctx,
-                                        ngx_http_core_module);

     if (ngx_handle_write_event(wev, clcf->send_lowat) != NGX_OK) {
         return NGX_ERROR; /* FIXME */
diff -r 725fb71ab1a6 -r 58d7a76b975e src/os/unix/ngx_linux_sendfile_chain.c
--- a/src/os/unix/ngx_linux_sendfile_chain.c Fri Jun 07 13:16:00 2013 -0700
+++ b/src/os/unix/ngx_linux_sendfile_chain.c Tue Jun 11 12:31:42 2013 +0400
@@ -33,11 +33,13 @@
 #define NGX_HEADERS  IOV_MAX
 #endif

+ngx_int_t
+ngx_http_check_and_set_nopush(ngx_connection_t* c);

 ngx_chain_t *
 ngx_linux_sendfile_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit)
 {
-    int            rc, tcp_nodelay;
+    int            rc;
     off_t          size, send, prev_send, aligned, sent, fprev;
     u_char        *prev;
     size_t         file_size;
@@ -154,61 +156,9 @@
             && cl
             && cl->buf->in_file)
         {
-            /* the TCP_CORK and TCP_NODELAY are mutually exclusive */
-
-            if (c->tcp_nodelay == NGX_TCP_NODELAY_SET) {
-
-                tcp_nodelay = 0;
-
-                if (setsockopt(c->fd, IPPROTO_TCP, TCP_NODELAY,
-                               (const void *) &tcp_nodelay, sizeof(int))
== -1)
-                {
-                    err = ngx_errno;
-
-                    /*
-                     * there is a tiny chance to be interrupted, however,
-                     * we continue a processing with the TCP_NODELAY
-                     * and without the TCP_CORK
-                     */
-
-                    if (err != NGX_EINTR) {
-                        wev->error = 1;
-                        ngx_connection_error(c, err,
-                                             "setsockopt(TCP_NODELAY)
failed");
-                        return NGX_CHAIN_ERROR;
-                    }
-
-                } else {
-                    c->tcp_nodelay = NGX_TCP_NODELAY_UNSET;
-
-                    ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
-                                   "no tcp_nodelay");
-                }
-            }
-
-            if (c->tcp_nodelay == NGX_TCP_NODELAY_UNSET) {
-
-                if (ngx_tcp_nopush(c->fd) == NGX_ERROR) {
-                    err = ngx_errno;
-
-                    /*
-                     * there is a tiny chance to be interrupted, however,
-                     * we continue a processing without the TCP_CORK
-                     */
-
-                    if (err != NGX_EINTR) {
-                        wev->error = 1;
-                        ngx_connection_error(c, err,
-                                             ngx_tcp_nopush_n " failed");
-                        return NGX_CHAIN_ERROR;
-                    }
-
-                } else {
-                    c->tcp_nopush = NGX_TCP_NOPUSH_SET;
-
-                    ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
-                                   "tcp_nopush");
-                }
+            if (ngx_http_check_and_set_nopush(c) != NGX_OK) {
+                wev->error = 1;
+                return NGX_CHAIN_ERROR;
             }
         }
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20130611/8e959770/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: spdy_fix_nopush
Type: application/octet-stream
Size: 9844 bytes
Desc: not available
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20130611/8e959770/attachment-0001.obj>


More information about the nginx-devel mailing list