Re: [PATCH net-next v3 5/7] net: ethtool: Add new power limit get and set features

[Kory Maincent]

On Sun, 16 Jun 2024 08:07:20 +0200
Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx> wrote:

> On Sat, Jun 15, 2024 at 08:28:30PM +0200, Oleksij Rempel wrote:
>  [...]  
> 
> Except of current value, we need an interface to return list of supported
> ranges. For example a controller with flexible configuration will have
> one entry 

Yes, good idea.
 
> Proposed interface may look like this:
> 
>   ``ETHTOOL_A_C33_PSE_AVAIL_PWR_VAL_LIMIT``  u32  Get PoE PSE currently
> configured power value limit ``ETHTOOL_A_C33_PSE_PWR_LIMIT_RANGES``
> nested  Supported power limit configuration ranges
> ======================================  ======  =============================
> 
>  +------------------------------------------+--------+----------------------------+
>  | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_RANGES``   | nested | array of power
> limit ranges|
> +-+----------------------------------------+--------+----------------------------+
> | | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_RANGE_ENTRY`` | nested | one power
> limit range  |
> +-+-+--------------------------------------+--------+----------------------------+
> | | | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_MIN``  | u32    | minimum power value
> (mW)   |
> +-+-+--------------------------------------+--------+----------------------------+
> | | | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_MAX``  | u32    | maximum power value
> (mW)   |
> +-+-+--------------------------------------+--------+----------------------------+

Not sure the ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_RANGE_ENTRY bring anything
interesting.

 +--------------------------------------------+--------+----------------------------+
 | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_RANGES`` | nested | array of power limit ranges|
 +-+------------------------------------------+--------+----------------------------+
 | | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_MIN``  | u32    | minimum power value (mW)   |
 +-+------------------------------------------+--------+----------------------------+
 | | ``ETHTOOL_A_C33_PSE_PWR_VAL_LIMIT_MAX``  | u32    | maximum power value (mW)   |
 +-+------------------------------------------+--------+----------------------------+

> > Huh... i took some more time to investigate it. Looks like there is no
> > simple answer. Some devices seems to write power class on the box. Other
> > client devices write power consumption in watts. IEEE 802.3-2022
> > provides LLDP specification with PowerValue for watts and PowerClass for
> > classes. Different product user interfaces provide class and/or watts.
> > So, let's go with watts then. Please update the name to something like
> > pse_available_power_value or pse_available_power_value_limit and
> > document how it is related to State diagrams in the IEEE spec.  
> 
> Here is proposal for documentation:
> 
>   ``ETHTOOL_A_C33_PSE_AVAIL_PWR_VAL_LIMIT``  u32  Control PoE PSE available
> power value limit
> 
> When set, the optional ``ETHTOOL_A_C33_PSE_AVAIL_PWR_VAL_LIMIT`` attribute is
> used  to control the available power value limit for C33 PSE in milliwatts.
> This attribute corresponds  to the `pse_available_power` variable described in
> ``IEEE 802.3-2022`` 33.2.4.4 Variables  and `pse_avail_pwr` in 145.2.5.4
> Variables, which are described in power classes. 
> 
> It was decided to use milliwatts for this interface to unify it with other
> power monitoring interfaces, which also use milliwatts, and to align with
> 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.

Thanks for the rephrasing!

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