[PATCH 4 of 6] Stream: the "deferred" parameter of the "listen" directive

[Sergey Kandaurov]

On Thu, Jan 18, 2024 at 07:24:21PM +0400, Roman Arutyunyan wrote:
> 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:
> > > 
> > > 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.
> > 

[..]

> 
> 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.
> 
> # 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++) {

Makes sense, agreed.