[PATCH] Stream: don't flush empty buffers created for read errors.

Aleksei Bavshin A.Bavshin at F5.com
Tue May 24 16:37:11 UTC 2022


In all honesty, the main motivation for the patch is to address a regression in UDP healthchecks caused by the behavior described below. It's already covered by tests, but the corresponding cases were disabled by default.

I considered
    cl->buf->flush = !src->read->error;
or even
    cl->buf->flush = !src->read->eof; 
but neither of those are exactly equivalent to the code below (with the latter having significantly different behavior), and IMO it would look less obvious. 

# HG changeset patch
# User Aleksei Bavshin <a.bavshin at f5.com>
# Date 1653330584 25200
#      Mon May 23 11:29:44 2022 -0700
# Branch se
# Node ID 5a98e9cb437f7719afa2bde62de68e174fd8e03e
# Parent  8902674cc7fe759cada415c10340f31ae4a90fba
Stream: don't flush empty buffers created for read errors.

When we generate the last_buf buffer for an UDP upstream recv error, it does
not contain any data from the wire. ngx_stream_write_filter attempts to forward
it anyways, which is incorrect (e.g., UDP upstream ECONNREFUSED will be
translated to an empty packet).

Reproduction:

stream {
    upstream unreachable {
        server     127.0.0.1:8880;
    }
    server {
        listen     127.0.0.1:8998 udp;
        proxy_pass unreachable;
    }
}

1 0.000000000    127.0.0.1 → 127.0.0.1    UDP 47 45588 → 8998 Len=5
2 0.000166300    127.0.0.1 → 127.0.0.1    UDP 47 51149 → 8880 Len=5
3 0.000172600    127.0.0.1 → 127.0.0.1    ICMP 75 Destination unreachable (Port
unreachable)
4 0.000202400    127.0.0.1 → 127.0.0.1    UDP 42 8998 → 45588 Len=0

Fixes d127837c714f.

diff --git a/src/stream/ngx_stream_proxy_module.c b/src/stream/ngx_stream_proxy_module.c
--- a/src/stream/ngx_stream_proxy_module.c
+++ b/src/stream/ngx_stream_proxy_module.c
@@ -1676,7 +1676,7 @@ ngx_stream_proxy_process(ngx_stream_sess
     ssize_t                       n;
     ngx_buf_t                    *b;
     ngx_int_t                     rc;
-    ngx_uint_t                    flags, *packets;
+    ngx_uint_t                    flags, flush, *packets;
     ngx_msec_t                    delay;
     ngx_chain_t                  *cl, **ll, **out, **busy;
     ngx_connection_t             *c, *pc, *src, *dst;
@@ -1803,9 +1803,12 @@ ngx_stream_proxy_process(ngx_stream_sess
                 break;
             }
 
+            flush = 1;
+
             if (n == NGX_ERROR) {
                 src->read->eof = 1;
                 n = 0;
+                flush = 0;
             }
 
             if (n >= 0) {
@@ -1846,7 +1849,7 @@ ngx_stream_proxy_process(ngx_stream_sess
 
                 cl->buf->temporary = (n ? 1 : 0);
                 cl->buf->last_buf = src->read->eof;
-                cl->buf->flush = 1;
+                cl->buf->flush = flush;
 
                 (*packets)++;
                 *received += n;



More information about the nginx-devel mailing list