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] [day] [month] [year] [list]

Message-ID: <20241126165228.4b113abb@kmaincent-XPS-13-7390>
Date: Tue, 26 Nov 2024 16:52:28 +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

On Tue, 26 Nov 2024 16:31:55 +0100
Kory Maincent <kory.maincent@...tlin.com> wrote:

> 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.

We already talked about it but a policies per port seems irrelevant to me.
https://lore.kernel.org/netdev/ZySR75i3BEzNbjnv@pengutronix.de/
How do we compare the priority value of ports that use different budget
strategy? How do we manage in the same power domain two ports with
different budget strategies or disconnection policies?

We indeed may need a separate interface to configure the PSE power domain
budget strategies and disconnection policies.

I think not being able to set the budget evaluation strategy is not relevant
for now as we don't have PSE which could support both, but being able to
set the disconnection policies may be relevant.
If we don't add this support to this series how do we decide which is the
default disconnection policy supported?

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