Re: [PATCH RFC net-next v2 15/18] net: pse-pd: Add support for getting and setting port priority

[Oleksij Rempel <o.rempel@pengutronix.de>]

On Thu, Oct 31, 2024 at 12:11:04PM +0100, Kory Maincent wrote:
> On Thu, 31 Oct 2024 07:54:08 +0100
> Oleksij Rempel <o.rempel@pengutronix.de> wrote:
> 
> > > diff --git a/include/uapi/linux/ethtool.h b/include/uapi/linux/ethtool.h
> > > index a1ad257b1ec1..22664b1ea4a2 100644
> > > --- a/include/uapi/linux/ethtool.h
> > > +++ b/include/uapi/linux/ethtool.h
> > > @@ -1002,11 +1002,35 @@ enum ethtool_c33_pse_pw_d_status {
> > >   * enum ethtool_c33_pse_events - event list of the C33 PSE controller.
> > >   * @ETHTOOL_C33_PSE_EVENT_OVER_CURRENT: PSE output current is too high.
> > >   * @ETHTOOL_C33_PSE_EVENT_OVER_TEMP: PSE in over temperature state.
> > > + * @ETHTOOL_C33_PSE_EVENT_CONNECTED: PD detected on the PSE.
> > > + * @ETHTOOL_C33_PSE_EVENT_DISCONNECTED: PD has been disconnected on the
> > > PSE.
> > > + * @ETHTOOL_C33_PSE_EVENT_PORT_PRIO_STATIC_ERROR: PSE faced an error in
> > > static
> > > + *	port priority management mode.
> > >   */
> > >  
> > >  enum ethtool_c33_pse_events {
> > > -	ETHTOOL_C33_PSE_EVENT_OVER_CURRENT =	1 << 0,
> > > -	ETHTOOL_C33_PSE_EVENT_OVER_TEMP =	1 << 1,
> > > +	ETHTOOL_C33_PSE_EVENT_OVER_CURRENT =		1 << 0,
> > > +	ETHTOOL_C33_PSE_EVENT_OVER_TEMP =		1 << 1,
> > > +	ETHTOOL_C33_PSE_EVENT_CONNECTED =		1 << 2,
> > > +	ETHTOOL_C33_PSE_EVENT_DISCONNECTED =		1 << 3,
> > > +	ETHTOOL_C33_PSE_EVENT_PORT_PRIO_STATIC_ERROR =	1 << 4,
> > > +};  
> > 
> > Same here, priority concept is not part of the spec, so the C33 prefix
> > should be removed.
> 
> Ack. So we assume PoDL could have the same interruption events.
> 
> > > +/**
> > > + * enum pse_port_prio_modes - PSE port priority modes.
> > > + * @ETHTOOL_PSE_PORT_PRIO_DISABLED: Port priority disabled.
> > > + * @ETHTOOL_PSE_PORT_PRIO_STATIC: PSE static port priority. Port priority
> > > + *	based on the power requested during PD classification. This mode
> > > + *	is managed by the PSE core.
> > > + * @ETHTOOL_PSE_PORT_PRIO_DYNAMIC: PSE dynamic port priority. Port priority
> > > + *	based on the current consumption per ports compared to the total
> > > + *	power budget. This mode is managed by the PSE controller.
> > > + */  

After thinking about it more overnight, I wanted to revisit the idea of having
a priority strategy per port. Right now, if one port is set to static or
dynamic mode, all disabled ports seem to have to follow it somehow too. This
makes it feel like we should have a strategy for the whole power domain, not
just for each port.

I'm having trouble imagining how a per-port priority strategy would work in
this setup.

Another point that came to mind is that we might have two different components
here, and we need to keep these two parts separate in follow-up discussions:

- **Budget Evaluation Strategy**: The static approach seems straightforward—if
a class requests more than available, appropriate actions are taken. However,
the dynamic approach has more complexity, such as determining the threshold,
how long violations can be tolerated, and whether a safety margin should be
maintained before exceeding maximum load.

- **Disconnection Policy**: Once a budget violation is detected, this decides
how to react, like which ports should be disconnected and in what order.

Would it make more sense to have a unified strategy for power domains, where we
apply the same budget evaluation mode (static or dynamic) and disconnection
policy to all ports in that domain? This could make the configuration simpler
and the power management more predictable.

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |