[PATCH 6 of 6] QUIC: path revalidation after expansion failure

[Roman Arutyunyan]

Hi,

On Fri, Dec 15, 2023 at 04:40:37PM +0400, Sergey Kandaurov wrote:
> 
> > On 30 Nov 2023, at 15:05, Roman Arutyunyan <arut at nginx.com> wrote:
> > 
> > # HG changeset patch
> > # User Roman Arutyunyan <arut at nginx.com>
> > # Date 1701241101 -14400
> > #      Wed Nov 29 10:58:21 2023 +0400
> > # Node ID 82fa5941af6fecb4fc7f0ac6308ae6c266d5e545
> > # Parent  4b7663d9146ce9baeb78fb57c3fed7368f25dae9
> > QUIC: path revalidation after expansion failure.
> > 
> > As per RFC 9000, Section 8.2.1:
> > 
> >    When an endpoint is unable to expand the datagram size to 1200 bytes due
> >    to the anti-amplification limit, the path MTU will not be validated.
> >    To ensure that the path MTU is large enough, the endpoint MUST perform a
> >    second path validation by sending a PATH_CHALLENGE frame in a datagram of
> >    at least 1200 bytes.
> > 
> > diff --git a/src/event/quic/ngx_event_quic_connection.h b/src/event/quic/ngx_event_quic_connection.h
> > --- a/src/event/quic/ngx_event_quic_connection.h
> > +++ b/src/event/quic/ngx_event_quic_connection.h
> > @@ -111,7 +111,8 @@ struct ngx_quic_path_s {
> >     uint64_t                          mtu_pnum[NGX_QUIC_PATH_RETRIES];
> >     ngx_str_t                         addr_text;
> >     u_char                            text[NGX_SOCKADDR_STRLEN];
> > -    ngx_uint_t                        validated; /* unsigned validated:1; */
> > +    unsigned                          validated:1;
> > +    unsigned                          mtu_unvalidated:1;
> > };
> > 
> > 
> > diff --git a/src/event/quic/ngx_event_quic_migration.c b/src/event/quic/ngx_event_quic_migration.c
> > --- a/src/event/quic/ngx_event_quic_migration.c
> > +++ b/src/event/quic/ngx_event_quic_migration.c
> > @@ -169,6 +169,7 @@ valid:
> > 
> >             path->mtu = prev->mtu;
> >             path->max_mtu = prev->max_mtu;
> > +            path->mtu_unvalidated = 0;
> >         }
> >     }
> > 
> > @@ -182,6 +183,13 @@ valid:
> >         qc->congestion.recovery_start = ngx_current_msec;
> >     }
> > 
> > +    path->validated = 1;
> > +
> > +    if (path->mtu_unvalidated) {
> > +        path->mtu_unvalidated = 0;
> > +        return ngx_quic_validate_path(c, path);
> > +    }
> > +
> >     /*
> >      * RFC 9000, 9.3.  Responding to Connection Migration
> >      *
> > @@ -199,8 +207,6 @@ valid:
> > 
> >     ngx_quic_path_dbg(c, "is validated", path);
> > 
> > -    path->validated = 1;
> > -
> >     ngx_quic_discover_path_mtu(c, path);
> > 
> >     return NGX_OK;
> > @@ -578,7 +584,15 @@ ngx_quic_send_path_challenge(ngx_connect
> >          * sending a datagram of this size.
> >          */
> > 
> > -        min = (ngx_quic_path_limit(c, path, 1200) < 1200) ? 0 : 1200;
> > +        if (path->mtu_unvalidated
> > +            || ngx_quic_path_limit(c, path, 1200) < 1200)
> > +        {
> > +            min = 0;
> > +            path->mtu_unvalidated = 1;
> > +
> > +        } else {
> > +            min = 1200;
> > +        }
> > 
> >         if (ngx_quic_frame_sendto(c, frame, min, path) == NGX_ERROR) {
> >             return NGX_ERROR;
> 
> This needs the following fixup, path->validated is now a bit-mask.
> 
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1702643887 -14400
> #      Fri Dec 15 16:38:07 2023 +0400
> # Node ID 763803589a36e3c67cbe39dd324b4e91fe57ecb7
> # Parent  cbe1a0e8094be744b940fe1b0cc5314f99c94672
> QUIC: fixed format specifier after a6f79f044de5.
> 
> diff --git a/src/event/quic/ngx_event_quic_migration.h b/src/event/quic/ngx_event_quic_migration.h
> --- a/src/event/quic/ngx_event_quic_migration.h
> +++ b/src/event/quic/ngx_event_quic_migration.h
> @@ -19,7 +19,7 @@
>  
>  #define ngx_quic_path_dbg(c, msg, path)                                       \
>      ngx_log_debug7(NGX_LOG_DEBUG_EVENT, c->log, 0,                            \
> -                   "quic path seq:%uL %s tx:%O rx:%O valid:%ui st:%d mtu:%uz",\
> +                   "quic path seq:%uL %s tx:%O rx:%O valid:%d st:%d mtu:%uz", \
>                     path->seqnum, msg, path->sent, path->received,             \
>                     path->validated, path->state, path->mtu);
>  

Looks ok