2023-12-01 17:12:12

by Kory Maincent

[permalink] [raw]
Subject: [PATCH net-next v2 4/8] net: ethtool: pse-pd: Expand pse commands with the PSE PoE interface

Add PSE PoE interface support in the ethtool pse command.

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

Changes in v2:
- Follow the "c33" PoE prefix naming change.
---
Documentation/networking/ethtool-netlink.rst | 20 +++++++++
net/ethtool/pse-pd.c | 64 +++++++++++++++++++++++-----
2 files changed, 74 insertions(+), 10 deletions(-)

diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 2540c70952ff..e02a7dabc673 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1711,6 +1711,10 @@ Kernel response contents:
PSE functions
``ETHTOOL_A_PODL_PSE_PW_D_STATUS`` u32 power detection status of the
PoDL PSE.
+ ``ETHTOOL_A_C33_PSE_ADMIN_STATE`` u32 Operational state of the PoE
+ PSE functions.
+ ``ETHTOOL_A_C33_PSE_PW_D_STATUS`` u32 power detection status of the
+ PoE PSE.
====================================== ====== =============================

When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_STATE`` attribute identifies
@@ -1722,6 +1726,12 @@ aPoDLPSEAdminState. Possible values are:
.. kernel-doc:: include/uapi/linux/ethtool.h
:identifiers: ethtool_podl_pse_admin_state

+The same goes for ``ETHTOOL_A_C33_PSE_ADMIN_STATE`` implementing
+``IEEE 802.3-2022`` 30.9.1.1.2 aPSEAdminState.
+
+.. kernel-doc:: include/uapi/linux/ethtool.h
+ :identifiers: ethtool_c33_pse_admin_state
+
When set, the optional ``ETHTOOL_A_PODL_PSE_PW_D_STATUS`` attribute identifies
the power detection status of the PoDL PSE. The status depend on internal PSE
state machine and automatic PD classification support. This option is
@@ -1731,6 +1741,12 @@ Possible values are:
.. kernel-doc:: include/uapi/linux/ethtool.h
:identifiers: ethtool_podl_pse_pw_d_status

+The same goes for ``ETHTOOL_A_C33_PSE_ADMIN_PW_D_STATUS`` implementing
+``IEEE 802.3-2022`` 30.9.1.1.5 aPSEPowerDetectionStatus.
+
+.. kernel-doc:: include/uapi/linux/ethtool.h
+ :identifiers: ethtool_c33_pse_pw_d_status
+
PSE_SET
=======

@@ -1741,6 +1757,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
====================================== ====== =============================

When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL`` attribute is used
@@ -1748,6 +1765,9 @@ to control PoDL PSE Admin functions. This option is implementing
``IEEE 802.3-2018`` 30.15.1.2.1 acPoDLPSEAdminControl. See
``ETHTOOL_A_PODL_PSE_ADMIN_STATE`` for supported values.

+The same goes for ``ETHTOOL_A_C33_PSE_ADMIN_CONTROL`` implementing
+``IEEE 802.3-2022`` 30.9.1.2.1 acPSEAdminControl.
+
RSS_GET
=======

diff --git a/net/ethtool/pse-pd.c b/net/ethtool/pse-pd.c
index aef57a058f0d..7a98dc029c4f 100644
--- a/net/ethtool/pse-pd.c
+++ b/net/ethtool/pse-pd.c
@@ -82,6 +82,10 @@ static int pse_reply_size(const struct ethnl_req_info *req_base,
len += nla_total_size(sizeof(u32)); /* _PODL_PSE_ADMIN_STATE */
if (st->podl_pw_status > 0)
len += nla_total_size(sizeof(u32)); /* _PODL_PSE_PW_D_STATUS */
+ if (st->c33_admin_state > 0)
+ len += nla_total_size(sizeof(u32)); /* _C33_PSE_ADMIN_STATE */
+ if (st->c33_pw_status > 0)
+ len += nla_total_size(sizeof(u32)); /* _C33_PSE_PW_D_STATUS */

return len;
}
@@ -103,6 +107,16 @@ static int pse_fill_reply(struct sk_buff *skb,
st->podl_pw_status))
return -EMSGSIZE;

+ if (st->c33_admin_state > 0 &&
+ nla_put_u32(skb, ETHTOOL_A_C33_PSE_ADMIN_STATE,
+ st->c33_admin_state))
+ return -EMSGSIZE;
+
+ if (st->c33_pw_status > 0 &&
+ nla_put_u32(skb, ETHTOOL_A_C33_PSE_PW_D_STATUS,
+ st->c33_pw_status))
+ return -EMSGSIZE;
+
return 0;
}

@@ -113,25 +127,18 @@ const struct nla_policy ethnl_pse_set_policy[ETHTOOL_A_PSE_MAX + 1] = {
[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] =
NLA_POLICY_RANGE(NLA_U32, ETHTOOL_PODL_PSE_ADMIN_STATE_DISABLED,
ETHTOOL_PODL_PSE_ADMIN_STATE_ENABLED),
+ [ETHTOOL_A_C33_PSE_ADMIN_CONTROL] =
+ NLA_POLICY_RANGE(NLA_U32, ETHTOOL_C33_PSE_ADMIN_STATE_DISABLED,
+ ETHTOOL_C33_PSE_ADMIN_STATE_ENABLED),
};

static int
ethnl_set_pse_validate(struct ethnl_req_info *req_info, struct genl_info *info)
-{
- return !!info->attrs[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL];
-}
-
-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;

- /* this values are already validated by the ethnl_pse_set_policy */
- config.podl_admin_control = nla_get_u32(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
-
phydev = dev->phydev;
if (!phydev) {
NL_SET_ERR_MSG(info->extack, "No PHY is attached");
@@ -143,6 +150,43 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
return -EOPNOTSUPP;
}

+ if (!tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] &&
+ !tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL])
+ return 0;
+
+ if (tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] &&
+ !(pse_get_types(phydev->psec) & PSE_PODL)) {
+ NL_SET_ERR_MSG_ATTR(info->extack,
+ tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL],
+ "setting PSE PoDL admin control not supported");
+ return -EOPNOTSUPP;
+ }
+ if (tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL] &&
+ !(pse_get_types(phydev->psec) & PSE_C33)) {
+ NL_SET_ERR_MSG_ATTR(info->extack,
+ tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL],
+ "setting PSE PoE admin control not supported");
+ return -EOPNOTSUPP;
+ }
+
+ return 1;
+}
+
+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;
+
+ phydev = dev->phydev;
+ /* These values are already validated by the ethnl_pse_set_policy */
+ if (pse_get_types(phydev->psec) & PSE_PODL)
+ config.podl_admin_control = nla_get_u32(tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL]);
+ if (pse_get_types(phydev->psec) & PSE_C33)
+ config.c33_admin_control = nla_get_u32(tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL]);
+
/* Return errno directly - PSE has no notification */
return pse_ethtool_set_config(phydev->psec, info->extack, &config);
}

--
2.25.1


2023-12-03 18:46:27

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 4/8] net: ethtool: pse-pd: Expand pse commands with the PSE PoE interface

> @@ -143,6 +150,43 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
> return -EOPNOTSUPP;
> }
>
> + if (!tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] &&
> + !tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL])
> + return 0;

-EINVAL? Is there a real use case for not passing either of them?

> +
> + if (tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] &&
> + !(pse_get_types(phydev->psec) & PSE_PODL)) {
> + NL_SET_ERR_MSG_ATTR(info->extack,
> + tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL],
> + "setting PSE PoDL admin control not supported");
> + return -EOPNOTSUPP;
> + }
> + if (tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL] &&
> + !(pse_get_types(phydev->psec) & PSE_C33)) {
> + NL_SET_ERR_MSG_ATTR(info->extack,
> + tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL],
> + "setting PSE PoE admin control not supported");

This probably should be C33, not PoE?

I guess it depends on what the user space tools are using.

Andrew

2023-12-04 15:28:51

by Kory Maincent

[permalink] [raw]
Subject: Re: [PATCH net-next v2 4/8] net: ethtool: pse-pd: Expand pse commands with the PSE PoE interface

On Sun, 3 Dec 2023 19:45:18 +0100
Andrew Lunn <[email protected]> wrote:

> > @@ -143,6 +150,43 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct
> > genl_info *info) return -EOPNOTSUPP;
> > }
> >
> > + if (!tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] &&
> > + !tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL])
> > + return 0;
>
> -EINVAL? Is there a real use case for not passing either of them?

No indeed.

> > +
> > + if (tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] &&
> > + !(pse_get_types(phydev->psec) & PSE_PODL)) {
> > + NL_SET_ERR_MSG_ATTR(info->extack,
> > + tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL],
> > + "setting PSE PoDL admin control not
> > supported");
> > + return -EOPNOTSUPP;
> > + }
> > + if (tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL] &&
> > + !(pse_get_types(phydev->psec) & PSE_C33)) {
> > + NL_SET_ERR_MSG_ATTR(info->extack,
> > + tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL],
> > + "setting PSE PoE admin control not
> > supported");
>
> This probably should be C33, not PoE?
>
> I guess it depends on what the user space tools are using.

Yes, I have hesitated on replacing that one.
If you prefer c33 in the log, I will change it in next version

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

2023-12-04 15:29:57

by Oleksij Rempel

[permalink] [raw]
Subject: Re: [PATCH net-next v2 4/8] net: ethtool: pse-pd: Expand pse commands with the PSE PoE interface

On Sun, Dec 03, 2023 at 07:45:18PM +0100, Andrew Lunn wrote:
> > @@ -143,6 +150,43 @@ ethnl_set_pse(struct ethnl_req_info *req_info, struct genl_info *info)
> > return -EOPNOTSUPP;
> > }
> >
> > + if (!tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] &&
> > + !tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL])
> > + return 0;
>
> -EINVAL? Is there a real use case for not passing either of them?
>
> > +
> > + if (tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL] &&
> > + !(pse_get_types(phydev->psec) & PSE_PODL)) {
> > + NL_SET_ERR_MSG_ATTR(info->extack,
> > + tb[ETHTOOL_A_PODL_PSE_ADMIN_CONTROL],
> > + "setting PSE PoDL admin control not supported");
> > + return -EOPNOTSUPP;
> > + }
> > + if (tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL] &&
> > + !(pse_get_types(phydev->psec) & PSE_C33)) {
> > + NL_SET_ERR_MSG_ATTR(info->extack,
> > + tb[ETHTOOL_A_C33_PSE_ADMIN_CONTROL],
> > + "setting PSE PoE admin control not supported");
>
> This probably should be C33, not PoE?
>
> I guess it depends on what the user space tools are using.

The same problem is in the documentation. Mixing different naming
schemes is problematic. Even unmixed this "PoE" is not really suitable for most
use cases. Expanding this abbreviations make it probably more clear:
- PSE PoE - Power Source Equipment Power over Ethernet
- C33 PSE - Clause 33 Power Source Equipment

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

2023-12-06 04:24:25

by Bagas Sanjaya

[permalink] [raw]
Subject: Re: [PATCH net-next v2 4/8] net: ethtool: pse-pd: Expand pse commands with the PSE PoE interface

On Fri, Dec 01, 2023 at 06:10:26PM +0100, Kory Maincent wrote:
> @@ -1741,6 +1757,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
> ====================================== ====== =============================

I get htmldocs warning:

```
Documentation/networking/ethtool-netlink.rst:1760: WARNING: Malformed table.
Text in column margin in table line 4.

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

I have to fix it up:

---- >8 ----
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index e02a7dabc673e2..8da5068105e3e9 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -1757,7 +1757,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_ADMIN_CONTROL`` u32 Control PSE Admin state
====================================== ====== =============================

When set, the optional ``ETHTOOL_A_PODL_PSE_ADMIN_CONTROL`` attribute is used

Thanks.

--
An old man doll... just what I always wanted! - Clara


Attachments:
(No filename) (1.96 kB)
signature.asc (235.00 B)
Download all attachments