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

[Eric Dumazet]

On Thu, Aug 29, 2024 at 5:28 PM Joe Damato <jdamato@xxxxxxxxxx> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > > Reviewed-by: Martin Karsten <mkarsten@xxxxxxxxxxxx>
> > > Tested-by: Martin Karsten <mkarsten@xxxxxxxxxxxx>
> > > ---
> > >  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.

> 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.