Re: [PATCH net-next 2/5] netdev-genl: Dump napi_defer_hard_irqs

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 30 Aug 2024 10:10:47 +0100 Joe Damato wrote:
> > > +        name: defer-hard-irqs
> > > +        doc: The number of consecutive empty polls before IRQ deferral ends
> > > +             and hardware IRQs are re-enabled.
> > > +        type: s32  
> > 
> > Why is this a signed value? 🤔️  
> 
> In commit 6f8b12d661d0 ("net: napi: add hard irqs deferral
> feature"), napi_defer_hard_irqs was added to struct net_device as an
> int. I was trying to match that and thus made the field a signed int
> in the napi struct, as well.

It's probably because int is the default type in C.
The choice of types in netlink feels more deliberate.

> It looks like there was a possibility of overflow introduced in that
> commit in change_napi_defer_hard_irqs maybe ?
> 
> If you'd prefer I could:
>   - submit a Fixes to change the net_device field to a u32 and then
>     change the netlink code to also be u32
>   - add an overflow check (val > U32_MAX) in
>     change_napi_defer_hard_irqs
> 
> Which would mean for the v2 of this series:
>   - drop the overflow check I added in Patch 1
>   - Change netlink to use u32 in this patch 
> 
> What do you think?

Whether we want to clean things up internally is up to you, the overflow
check you're adding in sysfs seems good. We can use u32 in netlink, with
a check: max: s32-max and lift this requirement later if we ever need
the 32nd bit?