Vladimir Homutov via nginx-devel
October 26, 2023 04:38PM
On Fri, Oct 27, 2023 at 12:27:22AM +0400, Sergey Kandaurov wrote:
> On Thu, Oct 26, 2023 at 05:20:39PM +0300, Vladimir Homutov wrote:
> > On Thu, Oct 26, 2023 at 03:08:55AM +0400, Sergey Kandaurov wrote:
> > > On Wed, Oct 11, 2023 at 04:58:47PM +0300, Vladimir Homutov via nginx-devel wrote:
> > [..]
> >
> > > > 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
> > > > @@ -563,8 +563,6 @@ ngx_quic_output_packet(ngx_connection_t
> > > > pkt.need_ack = 1;
> > > > }
> > > >
> > > > - ngx_quic_log_frame(c->log, f, 1);
> > > > -
> > > > flen = ngx_quic_create_frame(p, f);
> > > > if (flen == -1) {
> > > > return NGX_ERROR;
> > > > @@ -578,6 +576,8 @@ ngx_quic_output_packet(ngx_connection_t
> > > > f->last = now;
> > > > f->plen = 0;
> > > >
> > > > + ngx_quic_log_frame(c->log, f, 1);
> > > > +
> > > > nframes++;
> > >
> > > I'd rather move setting frame fields before calling
> > > ngx_quic_log_frame()/ngx_quic_create_frame()
> > > to preserve consistency with other places, i.e.:
> > > - set fields
> > > - log frame
> > > - create frame
> > >
> > > To look as follows:
> > >
> > > : f->pnum = ctx->pnum;
> > > : f->first = now;
> > > : f->last = now;
> > > : f->plen = 0;
> > > :
> > > : ngx_quic_log_frame(c->log, f, 1);
> > > :
> > > : flen = ngx_quic_create_frame(p, f);
> > > :
> >
> > agreed
> >
> > > > }
> > > >
> > > > @@ -925,6 +925,13 @@ ngx_quic_send_early_cc(ngx_connection_t
> > > >
> > > > res.data = dst;
> > > >
> > > > + ngx_log_debug7(NGX_LOG_DEBUG_EVENT, c->log, 0,
> > > > + "quic packet tx %s bytes:%ui need_ack:%d"
> > > > + " number:%L encoded nl:%d trunc:0x%xD frame:%ui]",
> > >
> > > typo: closing square bracket
> >
> > thanks, removed
> >
> > > Not sure we need logging for a (particular) frame in packet logging,
> > > not to say that it looks like a layering violation.
> > > Anyway, it is logged nearby, for example:
> > >
> > > quic frame tx init:0 CONNECTION_CLOSE err:11 invalid address validation token ft:0
> > > quic packet tx init bytes:36 need_ack:0 number:0 encoded nl:1 trunc:0x0
> > >
> > > So I'd remove this part.
> >
> > agreed, frame logging removed
> >
> > > > + ngx_quic_level_name(pkt.level), pkt.payload.len,
> > > > + pkt.need_ack, pkt.number, pkt.num_len, pkt.trunc,
> > > > + frame->type);
> > > > +
> > >
> > > BTW, it would make sense to get a new macro / inline function
> > > for packet tx logging, similar to ngx_quic_log_frame(),
> > > since we will have three places with identical ngx_log_debug7().
> >
> > actually, four (we have also retry), so having a macro is a good idea
> >
> > updated patch attached
>
> Well, I don't think retry needs logging, because this is not a real
> packet, it carries a token and is used to construct a Retry packet
> (which is also a special packet) later in ngx_quic_encrypt().
> Logging such a construct is bogus, because nearly all fields aren't
> initialized to sensible values, personally I've got the following:
>
> quic packet tx init bytes:0 need_ack:0 number:0 encoded nl:0 trunc:0x0

yes, this makes sense, removed.

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[patch] quic PTO counter fixes

Vladimir Homutov via nginx-devel 436 October 11, 2023 10:00AM

Re: [patch] quic PTO counter fixes

Thorvaldur Thorvaldsson 111 October 15, 2023 03:50PM

Re: [patch] quic PTO counter fixes

Sergey Kandaurov 93 October 25, 2023 07:10PM

Re: [patch] quic PTO counter fixes

Vladimir Homutov via nginx-devel 85 October 26, 2023 10:30AM

Re: [patch] quic PTO counter fixes

Sergey Kandaurov 83 October 26, 2023 04:28PM

Re: [patch] quic PTO counter fixes

Vladimir Homutov via nginx-devel 78 October 26, 2023 04:38PM

Re: [patch] quic PTO counter fixes

Sergey Kandaurov 103 October 26, 2023 05:34PM

Re: [patch] quic PTO counter fixes

Vladimir Homutov via nginx-devel 84 November 09, 2023 10:16AM

Re: [patch] quic PTO counter fixes

Sergey Kandaurov 104 November 14, 2023 08:12AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 330
Record Number of Users: 8 on April 13, 2023
Record Number of Guests: 500 on July 15, 2024
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready