Re: [PATCH net-next 06/12] net: ethtool: Add PSE new port priority support feature

[Oleksij Rempel]

On Wed, Oct 02, 2024 at 06:28:02PM +0200, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <kory.maincent@xxxxxxxxxxx>
> 
> This patch expands the status information provided by ethtool for PSE c33
> with current port priority and max port priority. It also adds a call to
> pse_ethtool_set_prio() to configure the PSE port priority.
> 
> Signed-off-by: Kory Maincent <kory.maincent@xxxxxxxxxxx>
> ---
>  Documentation/networking/ethtool-netlink.rst | 16 ++++++++++++++++
>  include/uapi/linux/ethtool_netlink.h         |  2 ++
>  net/ethtool/pse-pd.c                         | 18 ++++++++++++++++++
>  3 files changed, 36 insertions(+)
> 
> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
> index 295563e91082..15208429a973 100644
> --- a/Documentation/networking/ethtool-netlink.rst
> +++ b/Documentation/networking/ethtool-netlink.rst
> @@ -1763,6 +1763,10 @@ Kernel response contents:
>                                                        limit of the PoE PSE.
>    ``ETHTOOL_A_C33_PSE_PW_LIMIT_RANGES``       nested  Supported power limit
>                                                        configuration ranges.
> +  ``ETHTOOL_A_C33_PSE_PRIO_MAX``                 u32  priority maximum configurable
> +                                                      on the PoE PSE
> +  ``ETHTOOL_A_C33_PSE_PRIO``                     u32  priority of the PoE PSE
> +                                                      currently configured
>    ==========================================  ======  =============================
>  
>  When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_STATE`` attribute identifies
> @@ -1836,6 +1840,12 @@ identifies the C33 PSE power limit ranges through
>  If the controller works with fixed classes, the min and max values will be
>  equal.
>  
> +When set, the optional ``ETHTOOL_A_C33_PSE_PRIO_MAX`` attribute identifies
> +the C33 PSE maximum priority value.
> +
> +When set, the optional ``ETHTOOL_A_C33_PSE_PRIO`` attributes is used to
> +identifies the currently configured C33 PSE priority.
> +
>  PSE_SET
>  =======
>  
> @@ -1849,6 +1859,8 @@ Request contents:
>    ``ETHTOOL_A_C33_PSE_ADMIN_CONTROL``        u32  Control PSE Admin state
>    ``ETHTOOL_A_C33_PSE_AVAIL_PWR_LIMIT``      u32  Control PoE PSE available
>                                                    power limit
> +  ``ETHTOOL_A_C33_PSE_PRIO``                 u32  Control priority of the
> +                                                  PoE PSE
>    ======================================  ======  =============================
>  
>  When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL`` attribute is used
> @@ -1871,6 +1883,10 @@ various existing products that document power consumption in watts rather than
>  classes. If power limit configuration based on classes is needed, the
>  conversion can be done in user space, for example by ethtool.
>  
> +When set, the optional ``ETHTOOL_A_C33_PSE_PRIO`` attributes is used to
> +control the C33 PSE priority. Allowed priority value are between zero
> +and the value of ``ETHTOOL_A_C33_PSE_PRIO_MAX`` attribute.
 
We need to introduce a new attribute to effectively manage PSE priorities. With
the addition of the `ETHTOOL_A_C33_PSE_PRIO` attribute for setting priorities,
it's important to know which PSE controller or domain each port belongs to.

Initially, we might consider using a PSE controller index, such as
`ETHTOOL_A_PSE_CONTROLLER_ID`, to identify the specific PSE controller
associated with each port.

However, using just the PSE controller index is too limiting. Here's why:

- Typical PSE controllers handle priorities only within themselves. They
usually can't manage prioritization across different controllers unless they
are part of the same power domain. In systems where multiple PSE controllers
cooperate—either directly or through software mechanisms like the regulator
framework—controllers might share power domains or manage priorities together.
This means priorities are not confined to individual controllers but are
relevant within shared power domains.

- As systems become more complex, with controllers that can work together,
relying solely on a controller index won't accommodate these cooperative
scenarios.

To address these issues, we should use a power domain identifier instead. I
suggest introducing a new attribute called `ETHTOOL_A_PSE_POWER_DOMAIN_ID`.

- It specifies the power domain to which each port belongs, ensuring that
priorities are managed correctly within that domain.

- It accommodates systems where controllers cooperate and share power
resources, allowing for proper coordination of priorities across controllers
within the same power domain.

- It provides flexibility for future developments where controllers might work
together in new ways, preventing limitations that would arise from using a
strict controller index.

However, to provide comprehensive information, it would be beneficial to use
both attributes:

- `ETHTOOL_A_PSE_CONTROLLER_ID` to identify the specific PSE controller
associated with each port.

- `ETHTOOL_A_PSE_POWER_DOMAIN_ID` to specify the power domain to which each
port belongs.

Regards,
Oleksij
-- 
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 |