2024-06-14 14:34:57

by Kory Maincent

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

From: Kory Maincent (Dent Project) <[email protected]>

This patch expands the status information provided by ethtool for PSE c33
with power limit. It also adds a call to pse_ethtool_set_pw_limit() to
configure the PSE control power limit.

Signed-off-by: Kory Maincent <[email protected]>
---

Change in v3:
- Add ethtool netlink documentation.
---
Documentation/networking/ethtool-netlink.rst | 8 ++++++
include/uapi/linux/ethtool_netlink.h | 1 +
net/ethtool/pse-pd.c | 42 +++++++++++++++++++++++-----
3 files changed, 44 insertions(+), 7 deletions(-)

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.
+
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.
+
RSS_GET
=======

diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 398a0aa8daad..62ac9e1001bf 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -919,6 +919,7 @@ enum {
ETHTOOL_A_C33_PSE_ACTUAL_PW, /* u32 */
ETHTOOL_A_C33_PSE_EXT_STATE, /* u32 */
ETHTOOL_A_C33_PSE_EXT_SUBSTATE, /* u32 */
+ ETHTOOL_A_C33_PSE_PW_LIMIT, /* u32 */

/* add new constants above here */
__ETHTOOL_A_PSE_CNT,
diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index fec56db557d3..5c5eccd5b32b 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -94,6 +94,9 @@ static int pse_reply_size(const struct ethnl_req_info *req_base,
len += nla_total_size(sizeof(u32)); /* _C33_PSE_EXT_STATE */
if (st->c33_ext_state_info.__c33_pse_ext_substate > 0)
len += nla_total_size(sizeof(u32)); /* _C33_PSE_EXT_SUBSTATE */
+ if (st->c33_pw_limit > 0)
+ len += nla_total_size(sizeof(u32)); /* _C33_PSE_PW_LIMIT */
+
return len;
}

@@ -144,6 +147,11 @@ static int pse_fill_reply(struct sk_buff *skb,
st->c33_ext_state_info.__c33_pse_ext_substate))
return -EMSGSIZE;

+ if (st->c33_pw_limit > 0 &&
+ nla_put_u32(skb, ETHTOOL_A_C33_PSE_PW_LIMIT,
+ st->c33_pw_limit))
+ return -EMSGSIZE;
+
return 0;
}

@@ -157,6 +165,7 @@ const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1] = {
[ETHTOOL_A_C33_PSE_ADMIN_CONTROL] =
NLA_POLICY_RANGE(NLA_U32, ETHTOOL_C33_PSE_ADMIN_STATE_DISABLED,
ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED),
+ [ETHTOOL_A_C33_PSE_PW_LIMIT] = { .type = NLA_U32 },
};

static int
@@ -199,19 +208,38 @@ static int
ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
{
struct net_device *dev = req_info->dev;
- struct pse_control_config config = {};
struct nlattr **tb = info->attrs;
struct phy_device *phydev;
+ int ret = 0;

phydev = dev->phydev;
+
+ if (tb[ETHTOOL_A_C33_PSE_PW_LIMIT]) {
+ unsigned int pw_limit = nla_get_u32(tb[ETHTOOL_A_C33_PSE_PW_LIMIT]);
+
+ ret = pse_ethtool_set_pw_limit(phydev->psec, info->extack,
+ pw_limit);
+ if (ret)
+ return ret;
+ }
+
/* These values are already validated by the ethnl_pse_set_policy */
- if (pse_has_podl(phydev->psec))
- config.podl_admin_control = nla_get_u32(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
- if (pse_has_c33(phydev->psec))
- config.c33_admin_control = nla_get_u32(tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL]);
+ if (tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] ||
+ tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL]) {
+ struct pse_control_config config = {};
+
+ if (pse_has_podl(phydev->psec))
+ config.podl_admin_control = nla_get_u32(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
+ if (pse_has_c33(phydev->psec))
+ config.c33_admin_control = nla_get_u32(tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL]);
+
+ ret = pse_ethtool_set_config(phydev->psec, info->extack,
+ &config);
+ if (ret)
+ return ret;
+ }

- /* Return errno directly - PSE has no notification */
- return pse_ethtool_set_config(phydev->psec, info->extack, &config);
+ return ret;
}

const struct ethnl_request_ops ethnl_pse_request_ops = {

--
2.34.1



2024-06-15 16:00:04

by Oleksij Rempel

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

Hi Köry,

On Fri, Jun 14, 2024 at 04:33:21PM +0200, Kory Maincent wrote:
> From: Kory Maincent (Dent Project) <[email protected]>
>
> This patch expands the status information provided by ethtool for PSE c33
> with power limit. It also adds a call to pse_ethtool_set_pw_limit() to
> configure the PSE control power limit.
>
> Signed-off-by: Kory Maincent <[email protected]>
> ---
>
> Change in v3:
> - Add ethtool netlink documentation.
> ---
> Documentation/networking/ethtool-netlink.rst | 8 ++++++
> include/uapi/linux/ethtool_netlink.h | 1 +
> net/ethtool/pse-pd.c | 42 +++++++++++++++++++++++-----
> 3 files changed, 44 insertions(+), 7 deletions(-)
>
> 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.
> +
> 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?

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 |

2024-06-15 18:29:30

by Oleksij Rempel

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

On Sat, Jun 15, 2024 at 05:59:36PM +0200, Oleksij Rempel wrote:
> Hi Köry,
>
> On Fri, Jun 14, 2024 at 04:33:21PM +0200, Kory Maincent wrote:
> > From: Kory Maincent (Dent Project) <[email protected]>
> >
> > This patch expands the status information provided by ethtool for PSE c33
> > with power limit. It also adds a call to pse_ethtool_set_pw_limit() to
> > configure the PSE control power limit.
> >
> > Signed-off-by: Kory Maincent <[email protected]>
> > ---
> >
> > Change in v3:
> > - Add ethtool netlink documentation.
> > ---
> > Documentation/networking/ethtool-netlink.rst | 8 ++++++
> > include/uapi/linux/ethtool_netlink.h | 1 +
> > net/ethtool/pse-pd.c | 42 +++++++++++++++++++++++-----
> > 3 files changed, 44 insertions(+), 7 deletions(-)
> >
> > 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.
> > +
> > 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.

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

2024-06-16 06:08:44

by Oleksij Rempel

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

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 |