Welcome! Log In Create A New Profile

Advanced

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

Aleksei Bavshin via nginx-devel
May 24, 2022 12:38PM
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
Subject Author Views Posted

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

Aleksei Bavshin via nginx-devel 411 May 24, 2022 12:38PM

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

Roman Arutyunyan 124 May 26, 2022 05:34AM

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

Maxim Dounin 100 June 01, 2022 10:42AM

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

Aleksei Bavshin via nginx-devel 234 June 01, 2022 11:06AM

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

Maxim Dounin 189 June 01, 2022 08:30PM

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

Aleksei Bavshin via nginx-devel 238 June 01, 2022 11:22PM

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

Maxim Dounin 102 June 03, 2022 05:14PM

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

Roman Arutyunyan 154 June 07, 2022 03:04AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 240
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready