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 <a...@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 <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++) {
Makes sense, agreed.
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
