Subject: Re: [PATCH net-next v11 2/4] ethtool: provide customized dim profile management

From: "Michael S. Tsirkin" <[email-masked]>
Date: Wed, 1 May 2024 11:48:23 -0400

On Wed, May 01, 2024 at 11:11:47PM +0800, Heng Qi wrote:
> On Wed, 1 May 2024 07:44:20 -0700, Jakub Kicinski <[email-masked]> wrote:
> > On Wed, 1 May 2024 12:45:36 +0800 Heng Qi wrote:
> > > >    net/ethtool/coalesce.c: At top level:  
> > >  [...]  
> > > >      446 | static int ethnl_update_profile(struct net_device *dev,
> > > >          |            ^~~~~~~~~~~~~~~~~~~~  
> > >  [...]  
> > > >      151 | static int coalesce_put_profile(struct sk_buff *skb, u16 attr_type,
> > > >          |            ^~~~~~~~~~~~~~~~~~~~
> > > >   
> > > 
> > > This is a known minor issue, to reduce the use of 'IS_ENABLED(CONFIG_DIMLIB)'
> > > mentioned in v10. Since the calls of ethnl_update_profile() and
> > > coalesce_put_profile() will only occur when IS_ENABLED(CONFIG_DIMLIB) returns
> > > true, the robot's warning can be ignored the code is safe.
> > > 
> > > All NIPA test cases running on my local pass successfully on V11.
> > > 
> > > Alternatively, I remake the series to have IS_ENABLED(CONFIG_DIMLIB) back,
> > > up to Kuba (and others). :)
> > 
> > You should remove the ifdef around the member in struct net_device.
> > It's too much code complication to save one pointer in the struct.
> 
> Makes sense.
> 
> Thanks.

if you really want to you can add a comment
 /* only used if IS_ENABLED(CONFIG_DIMLIB) */