[QUIC] padding of Initial packets

[Vladimir Homutov]

On Tue, Feb 08, 2022 at 02:10:04PM +0300, Andrey Kolyshkin wrote:
> Hello.
>
> This patch is strange.
> 1. ngx_quic_revert_send can set to ctx an uninitialized value from
> preserved_pnum. (example if min > len and i = 0, only 0 element is filled
> in preserved_pnum but restored all)
> 2. ngx_quic_revert_send will restored pnum for ctx that have already called
> ngx_quic_output_packet and the packet with this pnum will be queued.
> (example if min > len and i = 1)

thank you for noticing.
indeed, this needs to be fixed. we don't want to restore contexts we
didn't yet touch.


>
>
> On Wed, Feb 2, 2022 at 2:07 PM Sergey Kandaurov <pluknet at nginx.com> wrote:
>
> >
> > > On 2 Feb 2022, at 13:55, Vladimir Homutov <vl at nginx.com> wrote:
> > >
> > > # HG changeset patch
> > > # User Vladimir Homutov <vl at nginx.com>
> > > # Date 1643796973 -10800
> > > #      Wed Feb 02 13:16:13 2022 +0300
> > > # Branch quic
> > > # Node ID fbfbcf66990e8964bcf308f3869f37d1a1acceeb
> > > # Parent  8c6645ecaeb6cbf27976fd9035440bfcab943117
> > > QUIC: fixed padding of initial packets in case of limited path.
> > >
> > > Previously, non-padded initial packet could be sent as a result of the
> > > following situation:
> > >
> > > - initial queue is not empty (so padding to 1200 is required)
> > > - handhsake queue is not empty (so padding is to be added after h/s
> > packet)
> >
> > handshake
> >
> > > - path is limited
> > >
> > > If serializing handshake packet would violate path limit, such packet was
> > > omitted, and the non-padded initial packet was sent.
> > >
> > > The fix is to avoid sending the packet at all in such case.  This
> > follows the
> > > original intention introduced in c5155a0cb12f.
> > >
> > > diff --git a/src/event/quic/ngx_event_quic_output.c
> > b/src/event/quic/ngx_event_quic_output.c
> > > --- a/src/event/quic/ngx_event_quic_output.c
> > > +++ b/src/event/quic/ngx_event_quic_output.c
> > > @@ -158,7 +158,14 @@ ngx_quic_create_datagrams(ngx_connection
> > >                   ? NGX_QUIC_MIN_INITIAL_SIZE - (p - dst) : 0;
> > >
> > >             if (min > len) {
> > > -                continue;
> > > +                /* padding can't be applied - avoid sending the packet
> > */
> > > +
> > > +                for (i = 0; i < NGX_QUIC_SEND_CTX_LAST; i++) {
> > > +                    ctx = &qc->send_ctx[i];
> > > +                    ngx_quic_revert_send(c, ctx, preserved_pnum[i]);
> >
> > this could be simplified to reduce ctx variable:
> >     ngx_quic_revert_send(c, &qc->send_ctx[i], preserved_pnum[i]);
> >
> > but it won't fit into 80 line, so that's good just as well
> >
> > > +                }
> > > +
> > > +                return NGX_OK;
> > >             }
> > >
> > >             n = ngx_quic_output_packet(c, ctx, p, len, min);
> > >
> >
> > --
> > Sergey Kandaurov
> >
> > _______________________________________________
> > nginx-devel mailing list -- nginx-devel at nginx.org
> > To unsubscribe send an email to nginx-devel-leave at nginx.org
> >
>
>
> --
> Best regards, Andrey

> _______________________________________________
> nginx-devel mailing list -- nginx-devel at nginx.org
> To unsubscribe send an email to nginx-devel-leave at nginx.org