Re: [PATCH net-next 3/5] net: napi: Make gro_flush_timeout per-NAPI

Joe Damato
2024-08-29T15:39:36Z

On Thu, Aug 29, 2024 at 05:31:30PM +0200, Eric Dumazet wrote:
> On Thu, Aug 29, 2024 at 5:28 PM Joe Damato <jdamato@fastly.com> wrote:
> >
> > On Thu, Aug 29, 2024 at 03:48:05PM +0200, Eric Dumazet wrote:
> > > On Thu, Aug 29, 2024 at 3:13 PM Joe Damato <jdamato@fastly.com> wrote:
> > > >
> > > > Allow per-NAPI gro_flush_timeout setting.
> > > >
> > > > The existing sysfs parameter is respected; writes to sysfs will write to
> > > > all NAPI structs for the device and the net_device gro_flush_timeout
> > > > field. Reads from sysfs will read from the net_device field.
> > > >
> > > > The ability to set gro_flush_timeout on specific NAPI instances will be
> > > > added in a later commit, via netdev-genl.
> > > >
> > > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > > > Reviewed-by: Martin Karsten <mkarsten@uwaterloo.ca>
> > > > Tested-by: Martin Karsten <mkarsten@uwaterloo.ca>
> > > > ---
> > > > include/linux/netdevice.h | 26 ++++++++++++++++++++++++++
> > > > net/core/dev.c | 32 ++++++++++++++++++++++++++++----
> > > > net/core/net-sysfs.c | 2 +-
> > > > 3 files changed, 55 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > > > index 7d53380da4c0..d00024d9f857 100644
> > > > --- a/include/linux/netdevice.h
> > > > +++ b/include/linux/netdevice.h
> > > > @@ -372,6 +372,7 @@ struct napi_struct {
> > > > int rx_count; /* length of rx_list */
> > > > unsigned int napi_id;
> > > > int defer_hard_irqs;
> > > > + unsigned long gro_flush_timeout;
> > > > struct hrtimer timer;
> > > > struct task_struct *thread;
> > > > /* control-path-only fields follow */
> > > > @@ -557,6 +558,31 @@ void napi_set_defer_hard_irqs(struct napi_struct *n, int defer);
> > > > */
> > > > void netdev_set_defer_hard_irqs(struct net_device *netdev, int defer);
> > > >
> > >
> > > Same remark : dev->gro_flush_timeout is no longer read in the fast path.
> > >
> > > Please move gro_flush_timeout out of net_device_read_txrx and update
> > > Documentation/networking/net_cachelines/net_device.rst
> >
> > Is there some tooling I should use to generate this file?
> >
> > I am asking because it seems like the file is missing two fields in
> > net_device at the end of the struct:
> >
> > struct hlist_head page_pools;
> > struct dim_irq_moder * irq_moder;
> >
>
> At first glance this is control path only, no big deal.

My plan was to move both fields you pointed out in my series to the
end of the struct (there is a big enough hole for both) and move
them to the bottom of this doc file (with just Type and Name, of
course).

The two fields (page_pools, irq_moder) being missing made me unsure
if I was planning on doing the right thing for the v2.

> > Both of which seem to have been added just before and long after
> > (respectively) commit 14006f1d8fa2 ("Documentations: Analyze heavily
> > used Networking related structs").
> >
> > If this is a bug, I can submit one patch (with two fixes tags) which
> > adds both fields to the file?
>
> No need for a Fixes: tag for this, just submit to net-next.
>
> This file is really 'needed' for current development, for people
> caring about data locality.

Will do; thanks for the guidance.