On Tue, May 24, 2022 at 04:37:11PM +0000, Aleksei Bavshin via nginx-devel wrote:
> 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@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;
> _______________________________________________
> nginx-devel mailing list -- nginx-devel@nginx.org
> To unsubscribe send an email to nginx-devel-leave@nginx.org

Looks good
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org