Hi, On Thu, Jan 18, 2024 at 06:51:32PM +0400, Sergey Kandaurov wrote: > > > On 9 Jan 2024, at 19:39, Roman Arutyunyan <a...@nginx.com> wrote: > > > > Hi, > > > > On Fri, Dec 15, 2023 at 07:37:47PM +0400, Sergey Kandaurov wrote: > >> # HG changeset patch > >> # User Sergey Kandaurov <pluk...@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 <pluk...@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 <pluk...@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 <pluk...@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
# HG changeset patch # User Roman Arutyunyan <a...@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++) {
_______________________________________________ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel