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

Joe Damato
2024-08-29T15:28:06Z

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;

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?

- Joe