Received-SPF: pass (google.com: domain of linux-kernel+bounces-216130-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Sun, 16 Jun 2024 08:07:20 +0200
From: Oleksij Rempel <o.rempel@pengutronix.de>
To: Kory Maincent <kory.maincent@bootlin.com>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Donald Hunter <donald.hunter@gmail.com>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	Dent Project <dentproject@linuxfoundation.org>,
	kernel@pengutronix.de
Subject: Re: [PATCH net-next v3 5/7] net: ethtool: Add new power limit get
 and set features
Message-ID: <Zm6BGJxu4bLVszFD@pengutronix.de>
References: <20240614-feature_poe_power_cap-v3-0-a26784e78311@bootlin.com>
 <20240614-feature_poe_power_cap-v3-5-a26784e78311@bootlin.com>
 <Zm26aJaz7Z7LAXNT@pengutronix.de>
 <Zm3dTuXuVEF9MhDS@pengutronix.de>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <Zm3dTuXuVEF9MhDS@pengutronix.de>

On Sat, Jun 15, 2024 at 08:28:30PM +0200, Oleksij Rempel wrote:
> > > diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
> > > index 7dbf2ef3ac0e..a78b6aea84af 100644
> > > --- a/Documentation/networking/ethtool-netlink.rst
> > > +++ b/Documentation/networking/ethtool-netlink.rst
> > > @@ -1737,6 +1737,7 @@ Kernel response contents:
> > >                                                    PoE PSE.
> > >    ``ETHTOOL_A_C33_PSE_EXT_SUBSTATE``         u32  power extended substatus of
> > >                                                    the PoE PSE.
> > > +  ``ETHTOOL_A_C33_PSE_PW_LIMIT``             u32  power limit of the PoE PSE.
> > >    ======================================  ======  =============================
> > >  
> > >  When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_STATE`` attribute identifies
> > > @@ -1799,6 +1800,9 @@ Possible values are:
> > >  		  ethtool_c33_pse_ext_substate_power_not_available
> > >  		  ethtool_c33_pse_ext_substate_short_detected
> > >  
> > > +When set, the optional ``ETHTOOL_A_C33_PSE_PW_LIMIT`` attribute identifies
> > > +the C33 PSE power limit in mW.

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 

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)   |
 +-+-+--------------------------------------+--------+----------------------------+

The min/max values should provide ranges actually configurable by PSE controller.
If controller works with fixed classes, the min and max values will be equal.

The ethtool output may look like this:

$ ethtool --get-pse eth0

Power Information for eth0:
=====================================
Current Power Limit: 15000 mW
Current Power Consumption: 12000 mW

Supported Power Limit Ranges:
  - Range 1: 0 - 7500 mW
  - Range 2: 7501 - 15000 mW
  - Range 3: 15001 - 30000 mW

Port Power Priority: 3
Supported Priority Range: 1 - 5

Pairs in Use: 4
Pair Configuration Type: Alternative A (MDI-X) and Alternative B(S)
PSE Type: Type 4
Detected PD Class: Class 5 (40000 mW max)

Low-Level Classification:
  - Classification Type: Autodetected
  - Configured PD Class: Class 5 (40000 mW max)

Maintain Power Signature (MPS) State: Present


> > > +
> > >  PSE_SET
> > >  =======
> > >  
> > > @@ -1810,6 +1814,7 @@ Request contents:
> > >    ``ETHTOOL_A_PSE_HEADER``                nested  request header
> > >    ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL``       u32  Control PoDL PSE Admin state
> > >    ``ETHTOOL_A_C33_PSE_ADMIN_CONTROL``        u32  Control PSE Admin state
> > > +  ``ETHTOOL_A_C33_PSE_PW_LIMIT``             u32  Control PoE PSE power limit
> > >    ======================================  ======  =============================
> > >  
> > >  When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL`` attribute is used
> > > @@ -1820,6 +1825,9 @@ to control PoDL PSE Admin functions. This option is implementing
> > >  The same goes for ``ETHTOOL_A_C33_PSE_ADMIN_CONTROL`` implementing
> > >  ``IEEE 802.3-2022`` 30.9.1.2.1 acPSEAdminControl.
> > >  
> > > +When set, the optional ``ETHTOOL_A_C33_PSE_PW_LIMIT`` attribute is used
> > > +to control C33 PSE power limit in mW.
> > 
> > 
> > The corresponding name int the IEEE 802.3-2022 seems to be pse_avail_pwr
> > in 145.2.5.4 Variables and pse_available_power in 33.2.4.4 Variables.
> > 
> > This variable is using classes instead of mW. pd692x0 seems to use
> > classes instead of mW too. May be it is better to use classes for UAPI
> > too? 
> 
> 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.

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