[PATCH 4 of 6] QUIC: never disable QUIC socket events

[Maxim Dounin]

Hello!

On Fri, Dec 09, 2022 at 09:38:50AM +0000, Roman Arutyunyan wrote:

> # HG changeset patch
> # User Roman Arutyunyan <arut at nginx.com>
> # Date 1670256830 0
> #      Mon Dec 05 16:13:50 2022 +0000
> # Branch quic
> # Node ID de8bcaea559d151f5945d0a2e06c61b56a26a52b
> # Parent  b5c30f16ec8ba3ace2f58d77d294d9b355bf3267
> QUIC: never disable QUIC socket events.
> 
> Unlike TCP accept(), current QUIC implementation does not require new file
> descriptors for new clients.  Also, it does not work with accept mutex since
> it normally requires reuseport option.
> 
> diff --git a/src/event/ngx_event_accept.c b/src/event/ngx_event_accept.c
> --- a/src/event/ngx_event_accept.c
> +++ b/src/event/ngx_event_accept.c
> @@ -416,6 +416,12 @@ ngx_disable_accept_events(ngx_cycle_t *c
>  
>  #endif
>  
> +#if (NGX_QUIC)
> +        if (ls[i].quic) {
> +            continue;
> +        }
> +#endif
> +

As long as the reuseport option is used, this should happen 
automatically.  If it's not, it might be a bad idea to do not 
disable accepting new QUIC connections, given that QUIC 
connections can still use file descriptors for other tasks, such 
as opening files and connecting to backends.

On the other hand, existing QUIC implementation cannot receive 
packets from previously created connections if socket events are 
disabled.  And this might be a good reason for the change.  
If this is the reason, shouldn't it be in the commit log?

Also, it looks like with client sockets, as introduced later in 
this patch series, it would be appropriate to actually disable 
events on QUIC listening sockets.

>          if (ngx_del_event(c->read, NGX_READ_EVENT, NGX_DISABLE_EVENT)
>              == NGX_ERROR)
>          {
> 
> _______________________________________________
> nginx-devel mailing list -- nginx-devel at nginx.org
> To unsubscribe send an email to nginx-devel-leave at nginx.org

-- 
Maxim Dounin
http://mdounin.ru/