Re: [PATCH net] net: napi: Make napi_defer_irqs u32

[Joe Damato <jdamato@fastly.com>]

On Mon, Sep 02, 2024 at 07:00:48PM +0200, Eric Dumazet wrote:
> On Mon, Sep 2, 2024 at 6:29 PM Joe Damato <jdamato@fastly.com> wrote:
> >
> > On Mon, Sep 02, 2024 at 03:01:28PM +0200, Eric Dumazet wrote:
> > > On Sat, Aug 31, 2024 at 1:32 PM Joe Damato <jdamato@fastly.com> wrote:
> > > >
> > > > In commit 6f8b12d661d0 ("net: napi: add hard irqs deferral feature")
> > > > napi_defer_irqs was added to net_device and napi_defer_irqs_count was
> > > > added to napi_struct, both as type int.
> > > >
> > > > This value never goes below zero. Change the type for both from int to
> > > > u32, and add an overflow check to sysfs to limit the value to S32_MAX.
> > > >
> > > > Before this patch:
> > > >
> > > > $ sudo bash -c 'echo 2147483649 > /sys/class/net/eth4/napi_defer_hard_irqs'
> > > > $ cat /sys/class/net/eth4/napi_defer_hard_irqs
> > > > -2147483647
> > > >
> > > > After this patch:
> > > >
> > > > $ sudo bash -c 'echo 2147483649 > /sys/class/net/eth4/napi_defer_hard_irqs'
> > > > bash: line 0: echo: write error: Numerical result out of range
> > > >
> > > > Fixes: 6f8b12d661d0 ("net: napi: add hard irqs deferral feature")
> > > > Cc: stable@kernel.org
> > > > Cc: Eric Dumazet <edumazet@google.com>
> > > > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > > > Signed-off-by: Joe Damato <jdamato@fastly.com>
> > > > ---
> > >
> > > I do not think this deserves a change to stable trees.
> >
> > OK, I can send any other revisions to -next, instead.
> >
> > > Signed or unsigned, what is the issue ?
> > >
> > > Do you really need one extra bit ?
> >
> > I made the maximum S32_MAX because the practical limit has always
> > been S32_MAX. Any larger values overflow. Keeping it at S32_MAX does
> > not change anything about existing behavior, which was my goal.
> >
> > Would you prefer if it was U32_MAX instead?
> >
> > Or are you asking me to leave it the way it is?
> 
> I think this would target net-next at most, please lets avoid hassles
> for stable teams.

Sure, that's fine with me.

I'm just not sure what you meant by your comment about the extra
bit and what you are asking me to make the maximum limit? I have no
preference.

I just want to prevent overflow and then make the per-NAPI stuff
compatible with existing sysfs code as much as possible.