[PATCH 4 of 6] Stream: the "deferred" parameter of the "listen" directive
Roman Arutyunyan
arut at nginx.com
Thu Jan 18 15:24:21 UTC 2024
Hi,
On Thu, Jan 18, 2024 at 06:51:32PM +0400, Sergey Kandaurov wrote:
>
> > On 9 Jan 2024, at 19:39, Roman Arutyunyan <arut at nginx.com> wrote:
> >
> > Hi,
> >
> > On Fri, Dec 15, 2023 at 07:37:47PM +0400, Sergey Kandaurov wrote:
> >> # HG changeset patch
> >> # User Sergey Kandaurov <pluknet at nginx.com>
> >> # Date 1702650289 -14400
> >> # Fri Dec 15 18:24:49 2023 +0400
> >> # Node ID cca722e447f8beaaa6b41a620c8b4239a5d1aa7d
> >> # Parent 4d90cb223fdb9e3e6c148726e36cec7835b2f0f8
> >> Stream: the "deferred" parameter of the "listen" directive.
> >>
> >> The Linux TCP_DEFER_ACCEPT support.
> >>
> >> diff --git a/src/stream/ngx_stream.c b/src/stream/ngx_stream.c
> >> --- a/src/stream/ngx_stream.c
> >> +++ b/src/stream/ngx_stream.c
> >> @@ -1021,6 +1021,10 @@ ngx_stream_add_listening(ngx_conf_t *cf,
> >> ls->keepcnt = addr->opt.tcp_keepcnt;
> >> #endif
> >>
> >> +#if (NGX_HAVE_DEFERRED_ACCEPT && defined TCP_DEFER_ACCEPT)
> >> + ls->deferred_accept = addr->opt.deferred_accept;
> >> +#endif
> >> +
> >> #if (NGX_HAVE_INET6)
> >> ls->ipv6only = addr->opt.ipv6only;
> >> #endif
> >> diff --git a/src/stream/ngx_stream.h b/src/stream/ngx_stream.h
> >> --- a/src/stream/ngx_stream.h
> >> +++ b/src/stream/ngx_stream.h
> >> @@ -53,6 +53,7 @@ typedef struct {
> >> #if (NGX_HAVE_INET6)
> >> unsigned ipv6only:1;
> >> #endif
> >> + unsigned deferred_accept:1;
> >> unsigned reuseport:1;
> >> unsigned so_keepalive:2;
> >> unsigned proxy_protocol:1;
> >> diff --git a/src/stream/ngx_stream_core_module.c b/src/stream/ngx_stream_core_module.c
> >> --- a/src/stream/ngx_stream_core_module.c
> >> +++ b/src/stream/ngx_stream_core_module.c
> >> @@ -987,6 +987,19 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
> >> continue;
> >> }
> >>
> >> + if (ngx_strcmp(value[i].data, "deferred") == 0) {
> >> +#if (NGX_HAVE_DEFERRED_ACCEPT && defined TCP_DEFER_ACCEPT)
> >> + lsopt.deferred_accept = 1;
> >> + lsopt.set = 1;
> >> + lsopt.bind = 1;
> >> +#else
> >> + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
> >> + "the deferred accept is not supported "
> >> + "on this platform, ignored");
> >> +#endif
> >> + continue;
> >> + }
> >> +
> >> if (ngx_strncmp(value[i].data, "ipv6only=o", 10) == 0) {
> >> #if (NGX_HAVE_INET6 && defined IPV6_V6ONLY)
> >> if (ngx_strcmp(&value[i].data[10], "n") == 0) {
> >
> > We should trigger an error if this option (TCP_DEFER_ACCEPT) is set for UDP.
> > We have a block "if (lsopt.type == SOCK_DGRAM) {}" later in this function.
> >
>
> Sure, this and the next change needs appropriate checks.
> SO_SETFIB used to set the routing table (next hop in ip_output)
> doesn't impose restriction on the socket type, so it is ok.
>
> Note that such checks are also missing for HTTP/3
> (see the relevant discussion in nginx-ru@ in December).
>
> Below is an updated patch series (reviewed changes skipped for brevity).
> It now includes an updated patch for HTTP/3 as reported by Izorkin.
>
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1705588165 -14400
> # Thu Jan 18 18:29:25 2024 +0400
> # Node ID df9c52b48971bc0b3d17b27ea261e8df7abb8f00
> # Parent dcca80118ff37f4ffc86ccf3693a098fb1fa9ffc
> HTTP/3: added more compatibility checks for "listen ... quic".
>
> Now "fastopen", "backlog", "accept_filter", "deferred", and "so_keepalive"
> parameters are not allowed with "quic" in the "listen" directive.
>
> Reported by Izorkin.
>
> diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
> --- a/src/http/ngx_http_core_module.c
> +++ b/src/http/ngx_http_core_module.c
> @@ -3961,7 +3961,7 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
>
> ngx_str_t *value, size;
> ngx_url_t u;
> - ngx_uint_t n, i;
> + ngx_uint_t n, i, backlog;
> ngx_http_listen_opt_t lsopt;
>
> cscf->listen = 1;
> @@ -4000,6 +4000,8 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
> lsopt.ipv6only = 1;
> #endif
>
> + backlog = 0;
> +
> for (n = 2; n < cf->args->nelts; n++) {
>
> if (ngx_strcmp(value[n].data, "default_server") == 0
> @@ -4058,6 +4060,8 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
> return NGX_CONF_ERROR;
> }
>
> + backlog = 1;
> +
> continue;
> }
>
> @@ -4305,9 +4309,29 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
> return NGX_CONF_ERROR;
> }
>
> -#if (NGX_HTTP_V3)
> -
> if (lsopt.quic) {
> +#if (NGX_HAVE_TCP_FASTOPEN)
> + if (lsopt.fastopen != -1) {
> + return "\"fastopen\" parameter is incompatible with \"quic\"";
> + }
> +#endif
> +
> + if (backlog) {
> + return "\"backlog\" parameter is incompatible with \"quic\"";
> + }
> +
> +#if (NGX_HAVE_DEFERRED_ACCEPT && defined SO_ACCEPTFILTER)
> + if (lsopt.accept_filter) {
> + return "\"accept_filter\" parameter is incompatible with \"quic\"";
> + }
> +#endif
> +
> +#if (NGX_HAVE_DEFERRED_ACCEPT && defined TCP_DEFER_ACCEPT)
> + if (lsopt.deferred_accept) {
> + return "\"deferred\" parameter is incompatible with \"quic\"";
> + }
> +#endif
> +
> #if (NGX_HTTP_SSL)
> if (lsopt.ssl) {
> return "\"ssl\" parameter is incompatible with \"quic\"";
> @@ -4320,13 +4344,15 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx
> }
> #endif
>
> + if (lsopt.so_keepalive) {
> + return "\"so_keepalive\" parameter is incompatible with \"quic\"";
> + }
> +
> if (lsopt.proxy_protocol) {
> return "\"proxy_protocol\" parameter is incompatible with \"quic\"";
> }
> }
>
> -#endif
> -
> for (n = 0; n < u.naddrs; n++) {
>
> for (i = 0; i < n; i++) {
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1705589071 -14400
> # Thu Jan 18 18:44:31 2024 +0400
> # Node ID b20e6b93489fda0778700b68cf3f85514c7e2547
> # Parent df9c52b48971bc0b3d17b27ea261e8df7abb8f00
> Stream: the "deferred" parameter of the "listen" directive.
>
> The Linux TCP_DEFER_ACCEPT support.
>
> diff --git a/src/stream/ngx_stream.c b/src/stream/ngx_stream.c
> --- a/src/stream/ngx_stream.c
> +++ b/src/stream/ngx_stream.c
> @@ -1021,6 +1021,10 @@ ngx_stream_add_listening(ngx_conf_t *cf,
> ls->keepcnt = addr->opt.tcp_keepcnt;
> #endif
>
> +#if (NGX_HAVE_DEFERRED_ACCEPT && defined TCP_DEFER_ACCEPT)
> + ls->deferred_accept = addr->opt.deferred_accept;
> +#endif
> +
> #if (NGX_HAVE_INET6)
> ls->ipv6only = addr->opt.ipv6only;
> #endif
> diff --git a/src/stream/ngx_stream.h b/src/stream/ngx_stream.h
> --- a/src/stream/ngx_stream.h
> +++ b/src/stream/ngx_stream.h
> @@ -53,6 +53,7 @@ typedef struct {
> #if (NGX_HAVE_INET6)
> unsigned ipv6only:1;
> #endif
> + unsigned deferred_accept:1;
> unsigned reuseport:1;
> unsigned so_keepalive:2;
> unsigned proxy_protocol:1;
> diff --git a/src/stream/ngx_stream_core_module.c b/src/stream/ngx_stream_core_module.c
> --- a/src/stream/ngx_stream_core_module.c
> +++ b/src/stream/ngx_stream_core_module.c
> @@ -1015,6 +1015,19 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
> continue;
> }
>
> + if (ngx_strcmp(value[i].data, "deferred") == 0) {
> +#if (NGX_HAVE_DEFERRED_ACCEPT && defined TCP_DEFER_ACCEPT)
> + lsopt.deferred_accept = 1;
> + lsopt.set = 1;
> + lsopt.bind = 1;
> +#else
> + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
> + "the deferred accept is not supported "
> + "on this platform, ignored");
> +#endif
> + continue;
> + }
> +
> if (ngx_strncmp(value[i].data, "ipv6only=o", 10) == 0) {
> #if (NGX_HAVE_INET6 && defined IPV6_V6ONLY)
> if (ngx_strcmp(&value[i].data[10], "n") == 0) {
> @@ -1173,6 +1186,12 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
> return "\"backlog\" parameter is incompatible with \"udp\"";
> }
>
> +#if (NGX_HAVE_DEFERRED_ACCEPT && defined TCP_DEFER_ACCEPT)
> + if (lsopt.deferred_accept) {
> + return "\"deferred\" parameter is incompatible with \"udp\"";
> + }
> +#endif
> +
> #if (NGX_STREAM_SSL)
> if (lsopt.ssl) {
> return "\"ssl\" parameter is incompatible with \"udp\"";
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1705589072 -14400
> # Thu Jan 18 18:44:32 2024 +0400
> # Node ID af5b23845d81168b8839512fd34fa5d39d316af2
> # Parent b20e6b93489fda0778700b68cf3f85514c7e2547
> Stream: the "accept_filter" parameter of the "listen" directive.
>
> The FreeBSD accept filters support.
>
> diff --git a/src/stream/ngx_stream.c b/src/stream/ngx_stream.c
> --- a/src/stream/ngx_stream.c
> +++ b/src/stream/ngx_stream.c
> @@ -1021,6 +1021,10 @@ ngx_stream_add_listening(ngx_conf_t *cf,
> ls->keepcnt = addr->opt.tcp_keepcnt;
> #endif
>
> +#if (NGX_HAVE_DEFERRED_ACCEPT && defined SO_ACCEPTFILTER)
> + ls->accept_filter = addr->opt.accept_filter;
> +#endif
> +
> #if (NGX_HAVE_DEFERRED_ACCEPT && defined TCP_DEFER_ACCEPT)
> ls->deferred_accept = addr->opt.deferred_accept;
> #endif
> diff --git a/src/stream/ngx_stream.h b/src/stream/ngx_stream.h
> --- a/src/stream/ngx_stream.h
> +++ b/src/stream/ngx_stream.h
> @@ -70,6 +70,10 @@ typedef struct {
> int tcp_keepintvl;
> int tcp_keepcnt;
> #endif
> +
> +#if (NGX_HAVE_DEFERRED_ACCEPT && defined SO_ACCEPTFILTER)
> + char *accept_filter;
> +#endif
> } ngx_stream_listen_opt_t;
>
>
> diff --git a/src/stream/ngx_stream_core_module.c b/src/stream/ngx_stream_core_module.c
> --- a/src/stream/ngx_stream_core_module.c
> +++ b/src/stream/ngx_stream_core_module.c
> @@ -1015,6 +1015,20 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
> continue;
> }
>
> + if (ngx_strncmp(value[i].data, "accept_filter=", 14) == 0) {
> +#if (NGX_HAVE_DEFERRED_ACCEPT && defined SO_ACCEPTFILTER)
> + lsopt.accept_filter = (char *) &value[i].data[14];
> + lsopt.set = 1;
> + lsopt.bind = 1;
> +#else
> + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
> + "accept filters \"%V\" are not supported "
> + "on this platform, ignored",
> + &value[i]);
> +#endif
> + continue;
> + }
> +
> if (ngx_strcmp(value[i].data, "deferred") == 0) {
> #if (NGX_HAVE_DEFERRED_ACCEPT && defined TCP_DEFER_ACCEPT)
> lsopt.deferred_accept = 1;
> @@ -1186,6 +1200,12 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
> return "\"backlog\" parameter is incompatible with \"udp\"";
> }
>
> +#if (NGX_HAVE_DEFERRED_ACCEPT && defined SO_ACCEPTFILTER)
> + if (lsopt.accept_filter) {
> + return "\"accept_filter\" parameter is incompatible with \"udp\"";
> + }
> +#endif
> +
> #if (NGX_HAVE_DEFERRED_ACCEPT && defined TCP_DEFER_ACCEPT)
> if (lsopt.deferred_accept) {
> return "\"deferred\" parameter is incompatible with \"udp\"";
>
The patches look ok.
However for even better visual compatibility between Stream and HTTP I suggest
a small patch on top of everything which moves the fastopen check up.
--
Roman Arutyunyan
-------------- next part --------------
# HG changeset patch
# User Roman Arutyunyan <arut at nginx.com>
# Date 1705590758 -14400
# Thu Jan 18 19:12:38 2024 +0400
# Node ID c8c4fe87c61c39ced688ad66655f40951cde6bcc
# Parent 0257dc20b29f2a897f90e78dc356d384c8d7f66d
Stream: moved fastopen compatibility check.
The move makes the code look similar to the corresponding code in http module.
diff --git a/src/stream/ngx_stream_core_module.c b/src/stream/ngx_stream_core_module.c
--- a/src/stream/ngx_stream_core_module.c
+++ b/src/stream/ngx_stream_core_module.c
@@ -1215,6 +1215,12 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
}
if (lsopt.type == SOCK_DGRAM) {
+#if (NGX_HAVE_TCP_FASTOPEN)
+ if (lsopt.fastopen != -1) {
+ return "\"fastopen\" parameter is incompatible with \"udp\"";
+ }
+#endif
+
if (backlog) {
return "\"backlog\" parameter is incompatible with \"udp\"";
}
@@ -1244,12 +1250,6 @@ ngx_stream_core_listen(ngx_conf_t *cf, n
if (lsopt.proxy_protocol) {
return "\"proxy_protocol\" parameter is incompatible with \"udp\"";
}
-
-#if (NGX_HAVE_TCP_FASTOPEN)
- if (lsopt.fastopen != -1) {
- return "\"fastopen\" parameter is incompatible with \"udp\"";
- }
-#endif
}
for (n = 0; n < u.naddrs; n++) {
More information about the nginx-devel
mailing list