Re: [net-next v5 4/9] netdev-genl: Dump gro_flush_timeout

[Joe Damato <jdamato@fastly.com>]

On Thu, Oct 10, 2024 at 06:45:11AM +0200, Eric Dumazet wrote:
> On Thu, Oct 10, 2024 at 6:34 AM Joe Damato <jdamato@fastly.com> wrote:
> >
> > On Wed, Oct 09, 2024 at 08:14:40PM -0700, Jakub Kicinski wrote:
> > > On Wed,  9 Oct 2024 00:54:58 +0000 Joe Damato wrote:
> > > > +        name: gro-flush-timeout
> > > > +        doc: The timeout, in nanoseconds, of when to trigger the NAPI
> > > > +             watchdog timer and schedule NAPI processing.
> > >
> > > You gotta respin because we reformatted the cacheline info.
> >
> > Yea, I figured I'd be racing with that change and would need a
> > respin.
> >
> > I'm not sure how the queue works exactly, but it looks like I might
> > also be racing with another change [1], I think.
> >
> > I think I'm just over 24hr and could respin and resend now, but
> > should I wait longer in case [1] is merged before you see my
> > respin?
> 
> I would avoid the rtnl_lock() addition in "netdev-genl: Support
> setting per-NAPI config values"
> before re-sending ?

OK.

> >
> > Just trying to figure out how to get the fewest number of respins
> > possible ;)
> >
> > > So while at it perhaps throw in a sentence here about the GRO effects?
> > > The initial use of GRO flush timeout was to hold incomplete GRO
> > > super-frames in the GRO engine across NAPI cycles.
> >
> > From my reading of the code, if the timeout is non-zero, then
> > napi_gro_flush will flush only "old" super-frames in
> > napi_complete_done.
> >
> > If that's accurate (and maybe I missed something?), then how about:
> >
> > doc: The timeout, in nanoseconds, of when to trigger the NAPI
> >      watchdog timer which schedules NAPI processing. Additionally, a
> >      non-zero value will also prevent GRO from flushing recent
> >      super-frames at the end of a NAPI cycle. This may add receive
> >      latency in exchange for reducing the number of frames processed
> >      by the network stack.
> 
> Note that linux TCP always has a PSH flag at the end of each TSO packet,
> so the latency increase is only possible in presence of tail drop,
> if the last MSS (with the PSH) was dropped.

Would you like me to note that in the doc, as well?