Re: [PATCH net-next v13 07/13] net: pse-pd: Add support for budget evaluation strategies

[Kory Maincent <kory.maincent@bootlin.com>]

Le Sat, 14 Jun 2025 12:33:11 -0700,
Jakub Kicinski <kuba@kernel.org> a écrit :

> On Tue, 10 Jun 2025 10:11:41 +0200 Kory Maincent wrote:
> > +static bool
> > +pse_pi_is_admin_enable_not_applied(struct pse_controller_dev *pcdev,
> > +				   int id)  
> 
> the only caller of this function seems to negate the return value:
> 
> drivers/net/pse-pd/pse_core.c:369:              if
> (!pse_pi_is_admin_enable_not_applied(pcdev, i))
> 
> let's avoid the double negation ?

I thought it was better for comprehension.
If we inverse the behavior we would have a function name like that:
pse_pi_is_admin_disable_not_detected_or_applied()

Do you have a better proposition?

> > +static int pse_disable_pi_pol(struct pse_controller_dev *pcdev, int id)
> > +{
> > +	unsigned long notifs = ETHTOOL_PSE_EVENT_OVER_BUDGET;
> > +	struct pse_ntf ntf = {};
> > +	int ret;
> > +
> > +	dev_dbg(pcdev->dev, "Disabling PI %d to free power budget\n", id);
> > +
> > +	NL_SET_ERR_MSG_FMT(&ntf.extack,
> > +			   "Disabling PI %d to free power budget", id);  
> 
> You so dutifully fill in this extack but it doesn't go anywhere.
> Extacks can only be attached to NLMSG_ERROR and NLMSG_DONE
> control messages. You can use the extack infra for the formatting,
> but you need to add a string attribute to the notification message
> to actually expose it to the user.

Indeed it seems there are useless extacks.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com