linux-kernel - Re: [PATCH RFC net-next v3 21/27] net: pse-pd: Add support for getting and setting port priority

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241126163155.4b7a444f@kmaincent-XPS-13-7390>
Date: Tue, 26 Nov 2024 16:31:55 +0100
From: Kory Maincent <kory.maincent@...tlin.com>
To: Oleksij Rempel <o.rempel@...gutronix.de>
Cc: Andrew Lunn <andrew@...n.ch>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo
 Abeni <pabeni@...hat.com>, Jonathan Corbet <corbet@....net>, Donald Hunter
 <donald.hunter@...il.com>, Rob Herring <robh@...nel.org>, Andrew Lunn
 <andrew+netdev@...n.ch>, Simon Horman <horms@...nel.org>, Heiner Kallweit
 <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>, Liam Girdwood
 <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>, linux-kernel@...r.kernel.org,
 netdev@...r.kernel.org, linux-doc@...r.kernel.org, Kyle Swenson
 <kyle.swenson@....tech>, Dent Project <dentproject@...uxfoundation.org>,
 kernel@...gutronix.de, Maxime Chevallier <maxime.chevallier@...tlin.com>
Subject: Re: [PATCH RFC net-next v3 21/27] net: pse-pd: Add support for
 getting and setting port priority

Hello Oleksij,

Thanks for your quick reviews!

On Tue, 26 Nov 2024 09:38:27 +0100
Oleksij Rempel <o.rempel@...gutronix.de> wrote:

> > +int pse_ethtool_set_prio_mode(struct pse_control *psec,
> > +			      struct netlink_ext_ack *extack,
> > +			      u32 prio_mode)
> > +{
> > +	struct pse_controller_dev *pcdev = psec->pcdev;
> > +	const struct pse_controller_ops *ops;
> > +	int ret = 0, i;
> > +
> > +	if (!(prio_mode & pcdev->port_prio_supp_modes)) {
> > +		NL_SET_ERR_MSG(extack, "priority mode not supported");
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	if (!pcdev->pi[psec->id].pw_d) {
> > +		NL_SET_ERR_MSG(extack, "no power domain attached");
> > +		return -EOPNOTSUPP;
> > +	}
> > +
> > +	/* ETHTOOL_PSE_PORT_PRIO_DISABLED can't be ORed with another mode
> > */
> > +	if (prio_mode & ETHTOOL_PSE_PORT_PRIO_DISABLED &&
> > +	    prio_mode & ~ETHTOOL_PSE_PORT_PRIO_DISABLED) {
> > +		NL_SET_ERR_MSG(extack,
> > +			       "port priority can't be enabled and
> > disabled simultaneously");
> > +		return -EINVAL;
> > +	}
> > +
> > +	ops = psec->pcdev->ops;
> > +
> > +	/* We don't want priority mode change in the middle of an
> > +	 * enable/disable call
> > +	 */
> > +	mutex_lock(&pcdev->lock);
> > +	pcdev->pi[psec->id].pw_d->port_prio_mode = prio_mode;  
> 
> In proposed implementation we have can set policies per port, but it
> will affect complete domain. This is not good. It feels like a separate
> challenge with extra discussion and work. I would recommend not to
> implement policy setting right now.
> 
> If you will decide to implement setting of policies anyway, then we need
> to discuss the interface.
> - If the policy should be done per domain, then we will need a separate
>   interface to interact with domains.
>   Pro: seems to be easier to implement.
> - If we will go with policy per port, wich would make sense too, then
>   some rework of this patch is needed.
>   Pro: can combine best of both strategies: set ports with wide load
>   range to static strategy and use dynamic strategy on other ports.
> 
> Right now we do not have software implementation for dynamic mode,
> implementing configuration of the policies from user space can be
> implemented later. It is enough to provide information about what
> hard coded policy is currently used.

There is no PSE that support static and dynamic mode indeed but the aim was to
be able to disable the budget evaluation strategy.

In fact we could have static strategy with a disconnection policy that do not
power a newly connected PD if we become over budget. This behavior would be
something similar to no budget evaluation strategy.

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