Hello everyone,
As part of the ongoing effort to better describe the ethernet link
topology, this series introduces the first step by allowing to maintain
a list of all the ethernet PHYs that are connected to a given netdevice.
For now, this can happen when using a PHY as a media converter :
MAC - PHYa - SFP - PHYb (in SFP module)
The issue with the above is that from userspace, we can only interact
with PHYa, as it's referenced by netdev->phydev, used in PHY-specific
netlink commands. This series therefore proposes to keep track of all
PHYs, through the struct link_topology, owned by struct
net_device. Phylib is therefore registering all PHYs and information on
their parent device, through direct attachment to a MAC or as part of an
SFP bus. This is done by patches 1 to 3.
Patches 4 to 6 introduce a new netlink command to get/dump the PHYs on a
given interface, with enough information to reconstruct the whole
topology, especially if we have chained PHYs.
Patches 7 to 10 are a proposition to extend the PLCA, PSE-PD, Cabletest
and stats reporting commands to take the PHY index into account, however
I only did minimal testing on these, and I'd like feedback on the idea.
Although more complete, this is still RFC, but I have some followup
series for a better port representation that depends on it, which I can
include for next revisions, but I don't want to make the series too big.
The first RFC was much less compete, but can be found here [1].
The overall topic was presented at Netdev 0x17 [2]
Best regards,
Maxime
[1] : https://lore.kernel.org/netdev/[email protected]/
[2] : https://bootlin.com/pub/conferences/2023/netdev/multi-port-multi-phy-interfaces.pdf
Maxime Chevallier (10):
net: phy: Introduce ethernet link topology representation
net: sfp: pass the phy_device when disconnecting an sfp module's PHY
net: phy: add helpers to handle sfp phy connect/disconnect
net: sfp: Add helper to return the SFP bus name
net: ethtool: Allow passing a phy index for some commands
net: ethtool: Introduce a command to list PHYs on an interface
net: ethtool: plca: Target the command to the requested PHY
net: ethtool: pse-pd: Target the command to the requested PHY
net: ethtool: cable-test: Target the command to the requested PHY
net: ethtool: strset: Allow querying phy stats by index
Documentation/netlink/specs/ethtool.yaml | 69 ++++-
Documentation/networking/ethtool-netlink.rst | 51 ++++
MAINTAINERS | 1 +
drivers/net/phy/Makefile | 2 +-
drivers/net/phy/at803x.c | 2 +
drivers/net/phy/link_topology.c | 78 ++++++
drivers/net/phy/marvell-88x2222.c | 2 +
drivers/net/phy/marvell.c | 2 +
drivers/net/phy/marvell10g.c | 2 +
drivers/net/phy/phy_device.c | 54 ++++
drivers/net/phy/phylink.c | 3 +-
drivers/net/phy/sfp-bus.c | 13 +-
include/linux/ethtool_netlink.h | 5 +
include/linux/link_topology.h | 59 ++++
include/linux/link_topology_core.h | 17 ++
include/linux/netdevice.h | 3 +-
include/linux/phy.h | 5 +
include/linux/sfp.h | 8 +-
include/uapi/linux/ethtool.h | 7 +
include/uapi/linux/ethtool_netlink.h | 30 ++
net/core/dev.c | 4 +
net/ethtool/Makefile | 2 +-
net/ethtool/cabletest.c | 12 +-
net/ethtool/netlink.c | 30 ++
net/ethtool/netlink.h | 11 +-
net/ethtool/phy.c | 279 +++++++++++++++++++
net/ethtool/plca.c | 13 +-
net/ethtool/pse-pd.c | 14 +-
net/ethtool/strset.c | 15 +-
29 files changed, 752 insertions(+), 41 deletions(-)
create mode 100644 drivers/net/phy/link_topology.c
create mode 100644 include/linux/link_topology.h
create mode 100644 include/linux/link_topology_core.h
create mode 100644 net/ethtool/phy.c
--
2.41.0
The ETH_SS_PHY_STATS command gets PHY statistics. Use the phydev pointer
from the ethnl request to allow query phy stats from each PHY on the
link.
Signed-off-by: Maxime Chevallier <[email protected]>
---
net/ethtool/strset.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/net/ethtool/strset.c b/net/ethtool/strset.c
index c678b484a079..70c00631c51f 100644
--- a/net/ethtool/strset.c
+++ b/net/ethtool/strset.c
@@ -233,17 +233,18 @@ static void strset_cleanup_data(struct ethnl_reply_data *reply_base)
}
static int strset_prepare_set(struct strset_info *info, struct net_device *dev,
- unsigned int id, bool counts_only)
+ struct phy_device *phydev, unsigned int id,
+ bool counts_only)
{
const struct ethtool_phy_ops *phy_ops = ethtool_phy_ops;
const struct ethtool_ops *ops = dev->ethtool_ops;
void *strings;
int count, ret;
- if (id == ETH_SS_PHY_STATS && dev->phydev &&
+ if (id == ETH_SS_PHY_STATS && phydev &&
!ops->get_ethtool_phy_stats && phy_ops &&
phy_ops->get_sset_count)
- ret = phy_ops->get_sset_count(dev->phydev);
+ ret = phy_ops->get_sset_count(phydev);
else if (ops->get_sset_count && ops->get_strings)
ret = ops->get_sset_count(dev, id);
else
@@ -258,10 +259,10 @@ static int strset_prepare_set(struct strset_info *info, struct net_device *dev,
strings = kcalloc(count, ETH_GSTRING_LEN, GFP_KERNEL);
if (!strings)
return -ENOMEM;
- if (id == ETH_SS_PHY_STATS && dev->phydev &&
+ if (id == ETH_SS_PHY_STATS && phydev &&
!ops->get_ethtool_phy_stats && phy_ops &&
phy_ops->get_strings)
- phy_ops->get_strings(dev->phydev, strings);
+ phy_ops->get_strings(phydev, strings);
else
ops->get_strings(dev, id, strings);
info->strings = strings;
@@ -305,8 +306,8 @@ static int strset_prepare_data(const struct ethnl_req_info *req_base,
!data->sets[i].per_dev)
continue;
- ret = strset_prepare_set(&data->sets[i], dev, i,
- req_info->counts_only);
+ ret = strset_prepare_set(&data->sets[i], dev, req_base->phydev,
+ i, req_info->counts_only);
if (ret < 0)
goto err_ops;
}
--
2.41.0
As we have the ability to track the PHYs connected to a net_device
through the link_topology, we can expose this list to userspace. This
allows userspace to use these identifiers for phy-specific commands and
take the decision of which PHY to target by knowing the link topology.
Add PHY_GET and PHY_DUMP, which can be a filtered DUMP operation to list
devices on only one interface.
Signed-off-by: Maxime Chevallier <[email protected]>
---
Documentation/netlink/specs/ethtool.yaml | 69 ++++-
Documentation/networking/ethtool-netlink.rst | 51 ++++
include/linux/ethtool_netlink.h | 5 +
include/uapi/linux/ethtool_netlink.h | 29 ++
net/ethtool/Makefile | 2 +-
net/ethtool/netlink.c | 8 +
net/ethtool/netlink.h | 3 +
net/ethtool/phy.c | 279 +++++++++++++++++++
8 files changed, 444 insertions(+), 2 deletions(-)
create mode 100644 net/ethtool/phy.c
diff --git a/Documentation/netlink/specs/ethtool.yaml b/Documentation/netlink/specs/ethtool.yaml
index 5c7a65b009b4..ac9352d8ef57 100644
--- a/Documentation/netlink/specs/ethtool.yaml
+++ b/Documentation/netlink/specs/ethtool.yaml
@@ -16,6 +16,11 @@ definitions:
name: stringset
type: enum
entries: []
+ -
+ name: phy-upstream-type
+ enum-name:
+ type: enum
+ entries: [ mac, sfp, phy ]
attribute-sets:
-
@@ -30,7 +35,9 @@ attribute-sets:
-
name: flags
type: u32
-
+ -
+ name: phy-index
+ type: u32
-
name: bitset-bit
attributes:
@@ -939,6 +946,45 @@ attribute-sets:
-
name: burst-tmr
type: u32
+ -
+ name: phy-upstream
+ attributes:
+ -
+ name: index
+ type: u32
+ -
+ name: sfp-name
+ type: string
+ -
+ name: phy
+ attributes:
+ -
+ name: header
+ type: nest
+ nested-attributes: header
+ -
+ name: phy-index
+ type: u32
+ -
+ name: drvname
+ type: string
+ -
+ name: name
+ type: string
+ -
+ name: upstream-type
+ type: u8
+ enum: phy-upstream-type
+ -
+ name: upstream
+ type: nest
+ nested-attributes: phy-upstream
+ -
+ name: downstream-sfp-name
+ type: string
+ -
+ name: id
+ type: u32
operations:
enum-model: directional
@@ -1689,3 +1735,24 @@ operations:
name: mm-ntf
doc: Notification for change in MAC Merge configuration.
notify: mm-get
+ -
+ name: phy-get
+ doc: Get PHY devices attached to an interface
+
+ attribute-set: phy
+
+ do: &phy-get-op
+ request:
+ attributes:
+ - header
+ reply:
+ attributes:
+ - header
+ - index
+ - drvname
+ - name
+ - upstream-type
+ - upstream
+ - downstream-sfp-name
+ - id
+ dump: *phy-get-op
diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
index 2540c70952ff..29ef675f45c0 100644
--- a/Documentation/networking/ethtool-netlink.rst
+++ b/Documentation/networking/ethtool-netlink.rst
@@ -57,6 +57,7 @@ Structure of this header is
``ETHTOOL_A_HEADER_DEV_INDEX`` u32 device ifindex
``ETHTOOL_A_HEADER_DEV_NAME`` string device name
``ETHTOOL_A_HEADER_FLAGS`` u32 flags common for all requests
+ ``ETHTOOL_A_HEADER_PHY_INDEX`` u32 phy device index
============================== ====== =============================
``ETHTOOL_A_HEADER_DEV_INDEX`` and ``ETHTOOL_A_HEADER_DEV_NAME`` identify the
@@ -81,6 +82,12 @@ the behaviour is backward compatible, i.e. requests from old clients not aware
of the flag should be interpreted the way the client expects. A client must
not set flags it does not understand.
+``ETHTOOL_A_HEADER_PHY_INDEX`` identify the ethernet PHY the message relates to.
+As there are numerous commands that are related to PHY configuration, and because
+we can have more than one PHY on the link, the PHY index can be passed in the
+request for the commands that needs it. It is however not mandatory, and if it
+is not passed for commands that target a PHY, the net_device.phydev pointer
+is used, as a fallback that keeps the legacy behaviour.
Bit sets
========
@@ -1994,6 +2001,49 @@ The attributes are propagated to the driver through the following structure:
.. kernel-doc:: include/linux/ethtool.h
:identifiers: ethtool_mm_cfg
+PHY_GET
+======
+
+Retrieve information about a given Ethernet PHY sitting on the link. As there
+can be more than one PHY, the DUMP operation can be used to list the PHYs
+present on a given interface, by passing an interface index or name in
+the dump request
+
+Request contents:
+
+ ==================================== ====== ==========================
+ ``ETHTOOL_A_PHY_HEADER`` nested request header
+ ==================================== ====== ==========================
+
+Kernel response contents:
+
+ ================================= ====== ==========================
+ ``ETHTOOL_A_PHY_HEADER`` nested request header
+ ``ETHTOOL_A_PHY_INDEX`` u32 the phy's unique index, that can
+ be used for phy-specific requests
+ ``ETHTOOL_A_PHY_DRVNAME`` string the phy driver name
+ ``ETHTOOL_A_PHY_NAME`` string the phy device name
+ ``ETHTOOL_A_PHY_UPSTREAM_TYPE`` u32 the type of device this phy is
+ connected to
+ ``ETHTOOL_A_PHY_UPSTREAM_PHY`` nested if the phy is connected to another
+ phy, this nest contains info on
+ that connection
+ ``ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME`` string if the phy controls an sfp bus,
+ the name of the sfp bus
+ ``ETHTOOL_A_PHY_ID`` u32 the phy id if the phy is C22
+ ================================= ====== ==========================
+
+When ``ETHTOOL_A_PHY_UPSTREAM_TYPE`` is PHY_UPSTREAM_PHY, the PHY's parent is
+another PHY. Information on the parent PHY will be set in the
+``ETHTOOL_A_PHY_UPSTREAM_PHY`` nest, which has the following structure :
+
+ ================================= ====== ==========================
+ ``ETHTOOL_A_PHY_UPSTREAM_INDEX`` u32 the PHY index of the upstream PHY
+ ``ETHTOOL_A_PHY_UPSTREAM_SFP_NAME`` string if this PHY is connected to it's
+ parent PHY through an SFP bus, the
+ name of this sfp bus
+ ================================= ====== ==========================
+
Request translation
===================
@@ -2100,4 +2150,5 @@ are netlink only.
n/a ``ETHTOOL_MSG_PLCA_GET_STATUS``
n/a ``ETHTOOL_MSG_MM_GET``
n/a ``ETHTOOL_MSG_MM_SET``
+ n/a ``ETHTOOL_MSG_PHY_GET``
=================================== =====================================
diff --git a/include/linux/ethtool_netlink.h b/include/linux/ethtool_netlink.h
index fae0dfb9a9c8..b4c40f1cc854 100644
--- a/include/linux/ethtool_netlink.h
+++ b/include/linux/ethtool_netlink.h
@@ -118,5 +118,10 @@ static inline bool ethtool_dev_mm_supported(struct net_device *dev)
return false;
}
+int ethnl_phy_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ return -EOPNOTSUPP;
+}
+
#endif /* IS_ENABLED(CONFIG_ETHTOOL_NETLINK) */
#endif /* _LINUX_ETHTOOL_NETLINK_H_ */
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index e557cf35250e..7d621963698a 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -57,6 +57,7 @@ enum {
ETHTOOL_MSG_PLCA_GET_STATUS,
ETHTOOL_MSG_MM_GET,
ETHTOOL_MSG_MM_SET,
+ ETHTOOL_MSG_PHY_GET,
/* add new constants above here */
__ETHTOOL_MSG_USER_CNT,
@@ -109,6 +110,8 @@ enum {
ETHTOOL_MSG_PLCA_NTF,
ETHTOOL_MSG_MM_GET_REPLY,
ETHTOOL_MSG_MM_NTF,
+ ETHTOOL_MSG_PHY_GET_REPLY,
+ ETHTOOL_MSG_PHY_NTF,
/* add new constants above here */
__ETHTOOL_MSG_KERNEL_CNT,
@@ -976,6 +979,32 @@ enum {
ETHTOOL_A_MM_MAX = (__ETHTOOL_A_MM_CNT - 1)
};
+enum {
+ ETHTOOL_A_PHY_UPSTREAM_UNSPEC,
+ ETHTOOL_A_PHY_UPSTREAM_INDEX, /* u32 */
+ ETHTOOL_A_PHY_UPSTREAM_SFP_NAME, /* string */
+
+ /* add new constants above here */
+ __ETHTOOL_A_PHY_UPSTREAM_CNT,
+ ETHTOOL_A_PHY_UPSTREAM_MAX = (__ETHTOOL_A_PHY_UPSTREAM_CNT - 1)
+};
+
+enum {
+ ETHTOOL_A_PHY_UNSPEC,
+ ETHTOOL_A_PHY_HEADER, /* nest - _A_HEADER_* */
+ ETHTOOL_A_PHY_INDEX, /* u32 */
+ ETHTOOL_A_PHY_DRVNAME, /* string */
+ ETHTOOL_A_PHY_NAME, /* string */
+ ETHTOOL_A_PHY_UPSTREAM_TYPE, /* u8 */
+ ETHTOOL_A_PHY_UPSTREAM, /* nest - _A_PHY_UPSTREAM_* */
+ ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME, /* string */
+ ETHTOOL_A_PHY_ID, /* u32 */
+
+ /* add new constants above here */
+ __ETHTOOL_A_PHY_CNT,
+ ETHTOOL_A_PHY_MAX = (__ETHTOOL_A_PHY_CNT - 1)
+};
+
/* generic netlink info */
#define ETHTOOL_GENL_NAME "ethtool"
#define ETHTOOL_GENL_VERSION 1
diff --git a/net/ethtool/Makefile b/net/ethtool/Makefile
index 504f954a1b28..0ccd0e9afd3f 100644
--- a/net/ethtool/Makefile
+++ b/net/ethtool/Makefile
@@ -8,4 +8,4 @@ ethtool_nl-y := netlink.o bitset.o strset.o linkinfo.o linkmodes.o rss.o \
linkstate.o debug.o wol.o features.o privflags.o rings.o \
channels.o coalesce.o pause.o eee.o tsinfo.o cabletest.o \
tunnels.o fec.o eeprom.o stats.o phc_vclocks.o mm.o \
- module.o pse-pd.o plca.o mm.o
+ module.o pse-pd.o plca.o mm.o phy.o
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index e83ee844b60f..80c8c312a584 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -1150,6 +1150,14 @@ static const struct genl_ops ethtool_genl_ops[] = {
.policy = ethnl_mm_set_policy,
.maxattr = ARRAY_SIZE(ethnl_mm_set_policy) - 1,
},
+ {
+ .cmd = ETHTOOL_MSG_PHY_GET,
+ .doit = ethnl_phy_doit,
+ .start = ethnl_phy_start,
+ .dumpit = ethnl_phy_dumpit,
+ .policy = ethnl_phy_get_policy,
+ .maxattr = ARRAY_SIZE(ethnl_phy_get_policy) - 1,
+ },
};
static const struct genl_multicast_group ethtool_nl_mcgrps[] = {
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 3f5eb60bdf0b..844c83c2ec7f 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -452,6 +452,9 @@ int ethnl_act_cable_test_tdr(struct sk_buff *skb, struct genl_info *info);
int ethnl_tunnel_info_doit(struct sk_buff *skb, struct genl_info *info);
int ethnl_tunnel_info_start(struct netlink_callback *cb);
int ethnl_tunnel_info_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
+int ethnl_phy_start(struct netlink_callback *cb);
+int ethnl_phy_doit(struct sk_buff *skb, struct genl_info *info);
+int ethnl_phy_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
extern const char stats_std_names[__ETHTOOL_STATS_CNT][ETH_GSTRING_LEN];
extern const char stats_eth_phy_names[__ETHTOOL_A_STATS_ETH_PHY_CNT][ETH_GSTRING_LEN];
diff --git a/net/ethtool/phy.c b/net/ethtool/phy.c
new file mode 100644
index 000000000000..ac6b4612301c
--- /dev/null
+++ b/net/ethtool/phy.c
@@ -0,0 +1,279 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright 2023 Bootlin
+ *
+ */
+#include "common.h"
+#include "netlink.h"
+
+#include <linux/link_topology.h>
+#include <linux/phy.h>
+#include <linux/sfp.h>
+
+struct phy_req_info {
+ struct ethnl_req_info base;
+ struct phy_device_node pdn;
+};
+
+#define PHY_REQINFO(__req_base) \
+ container_of(__req_base, struct phy_req_info, base)
+
+const struct nla_policy ethnl_phy_get_policy[ETHTOOL_A_PHY_HEADER + 1] = {
+ [ETHTOOL_A_PHY_HEADER] = NLA_POLICY_NESTED(ethnl_header_policy),
+};
+
+/* Caller holds rtnl */
+static ssize_t
+ethnl_phy_reply_size(const struct ethnl_req_info *req_base,
+ struct netlink_ext_ack *extack)
+{
+ struct phy_device_node *pdn;
+ struct phy_device *phydev;
+ struct link_topology *lt;
+ unsigned long index;
+ size_t size;
+
+ lt = &req_base->dev->link_topo;
+
+ size = nla_total_size(0);
+
+ xa_for_each(<->phys, index, pdn) {
+ phydev = pdn->phy;
+
+ /* ETHTOOL_A_PHY_INDEX */
+ size += nla_total_size(sizeof(u32));
+
+ /* ETHTOOL_A_DRVNAME */
+ size += nla_total_size(strlen(phydev->drv->name));
+
+ /* ETHTOOL_A_NAME */
+ size += nla_total_size(strlen(dev_name(&phydev->mdio.dev)));
+
+ /* ETHTOOL_A_PHY_UPSTREAM_TYPE */
+ size += nla_total_size(sizeof(u8));
+
+ /* ETHTOOL_A_PHY_ID */
+ size += nla_total_size(sizeof(u32));
+
+ if (phy_on_sfp(phydev)) {
+ /* ETHTOOL_A_PHY_UPSTREAM_SFP_NAME */
+ if (sfp_get_name(pdn->parent_sfp_bus))
+ size += nla_total_size(strlen(sfp_get_name(pdn->parent_sfp_bus)));
+
+ /* ETHTOOL_A_PHY_UPSTREAM_INDEX */
+ size += nla_total_size(sizeof(u32));
+ }
+
+ /* ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME */
+ if (phydev->sfp_bus)
+ size += nla_total_size(strlen(sfp_get_name(phydev->sfp_bus)));
+ }
+
+ return size;
+}
+
+static int
+ethnl_phy_fill_reply(const struct ethnl_req_info *req_base, struct sk_buff *skb)
+{
+ struct phy_req_info *req_info = PHY_REQINFO(req_base);
+ struct phy_device_node *pdn = &req_info->pdn;
+ struct phy_device *phydev = pdn->phy;
+ enum phy_upstream ptype;
+ struct nlattr *nest;
+
+ ptype = pdn->upstream_type;
+
+ if (nla_put_u32(skb, ETHTOOL_A_PHY_INDEX, phydev->phyindex) ||
+ nla_put_string(skb, ETHTOOL_A_PHY_DRVNAME, phydev->drv->name) ||
+ nla_put_string(skb, ETHTOOL_A_PHY_NAME, dev_name(&phydev->mdio.dev)) ||
+ nla_put_u8(skb, ETHTOOL_A_PHY_UPSTREAM_TYPE, ptype) ||
+ nla_put_u32(skb, ETHTOOL_A_PHY_ID, phydev->phy_id))
+ return -EMSGSIZE;
+
+ if (ptype == PHY_UPSTREAM_PHY) {
+ struct phy_device *upstream = pdn->upstream.phydev;
+
+ nest = nla_nest_start(skb, ETHTOOL_A_PHY_UPSTREAM);
+ if (!nest)
+ return -EMSGSIZE;
+
+ /* Parent index */
+ if (nla_put_u32(skb, ETHTOOL_A_PHY_UPSTREAM_INDEX, upstream->phyindex))
+ return -EMSGSIZE;
+
+ if (pdn->parent_sfp_bus && sfp_get_name(pdn->parent_sfp_bus) &&
+ nla_put_string(skb, ETHTOOL_A_PHY_UPSTREAM_SFP_NAME,
+ sfp_get_name(pdn->parent_sfp_bus)))
+ return -EMSGSIZE;
+
+ nla_nest_end(skb, nest);
+ }
+
+ if (phydev->sfp_bus &&
+ nla_put_string(skb, ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME,
+ sfp_get_name(phydev->sfp_bus)))
+ return -EMSGSIZE;
+
+ return 0;
+}
+
+static int ethnl_phy_parse_request(struct ethnl_req_info *req_base,
+ struct nlattr **tb)
+{
+ struct phy_req_info *req_info = PHY_REQINFO(req_base);
+ struct link_topology *lt = &req_base->dev->link_topo;
+ struct phy_device_node *pdn;
+
+ if (!req_base->phydev)
+ return 0;
+
+ pdn = xa_load(<->phys, req_base->phydev->phyindex);
+ memcpy(&req_info->pdn, pdn, sizeof(*pdn));
+
+ return 0;
+}
+
+int ethnl_phy_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ struct phy_req_info req_info = {};
+ struct nlattr **tb = info->attrs;
+ struct sk_buff *rskb;
+ void *reply_payload;
+ int reply_len;
+ int ret;
+
+ ret = ethnl_parse_header_dev_get(&req_info.base,
+ tb[ETHTOOL_A_PHY_HEADER],
+ genl_info_net(info), info->extack,
+ true);
+ if (ret < 0)
+ return ret;
+
+ rtnl_lock();
+
+ ret = ethnl_phy_parse_request(&req_info.base, tb);
+ if (ret < 0)
+ goto err_unlock_rtnl;
+
+ /* No PHY, return early */
+ if (!req_info.pdn.phy)
+ goto err_unlock_rtnl;
+
+ ret = ethnl_phy_reply_size(&req_info.base, info->extack);
+ if (ret < 0)
+ goto err_unlock_rtnl;
+ reply_len = ret + ethnl_reply_header_size();
+
+ rskb = ethnl_reply_init(reply_len, req_info.base.dev,
+ ETHTOOL_MSG_PHY_GET_REPLY,
+ ETHTOOL_A_PHY_HEADER,
+ info, &reply_payload);
+ if (!rskb) {
+ ret = -ENOMEM;
+ goto err_unlock_rtnl;
+ }
+
+ ret = ethnl_phy_fill_reply(&req_info.base, rskb);
+ if (ret)
+ goto err_free_msg;
+
+ rtnl_unlock();
+ ethnl_parse_header_dev_put(&req_info.base);
+ genlmsg_end(rskb, reply_payload);
+
+ return genlmsg_reply(rskb, info);
+
+err_free_msg:
+ nlmsg_free(rskb);
+err_unlock_rtnl:
+ rtnl_unlock();
+ ethnl_parse_header_dev_put(&req_info.base);
+ return ret;
+}
+
+struct ethnl_phy_dump_ctx {
+ struct ethnl_req_info req_info;
+};
+
+int ethnl_phy_start(struct netlink_callback *cb)
+{
+ const struct genl_dumpit_info *info = genl_dumpit_info(cb);
+ struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
+ struct nlattr **tb = info->info.attrs;
+ int ret;
+
+ BUILD_BUG_ON(sizeof(*ctx) > sizeof(cb->ctx));
+
+ memset(ctx, 0, sizeof(*ctx));
+
+ ret = ethnl_parse_header_dev_get(&ctx->req_info,
+ tb[ETHTOOL_A_PHY_HEADER],
+ sock_net(cb->skb->sk), cb->extack,
+ false);
+ return ret;
+}
+
+int ethnl_phy_dump_one_dev(struct sk_buff *skb, struct net_device *dev,
+ struct netlink_callback *cb)
+{
+ struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
+ struct phy_req_info *pri = PHY_REQINFO(&ctx->req_info);
+ struct phy_device_node *pdn;
+ unsigned long index = 1;
+ void *ehdr;
+ int ret;
+
+ ctx->req_info.dev = dev;
+
+ xa_for_each(&dev->link_topo.phys, index, pdn) {
+ ehdr = ethnl_dump_put(skb, cb,
+ ETHTOOL_MSG_PHY_GET_REPLY);
+ if (!ehdr) {
+ ret = -EMSGSIZE;
+ break;
+ }
+
+ ret = ethnl_fill_reply_header(skb, dev,
+ ETHTOOL_A_PHY_HEADER);
+ if (ret < 0) {
+ genlmsg_cancel(skb, ehdr);
+ break;
+ }
+
+ memcpy(&pri->pdn, pdn, sizeof(*pdn));
+ ret = ethnl_phy_fill_reply(&ctx->req_info, skb);
+
+ genlmsg_end(skb, ehdr);
+ }
+
+ ctx->req_info.dev = NULL;
+
+ return ret;
+}
+
+int ethnl_phy_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
+{
+ struct ethnl_phy_dump_ctx *ctx = (void *)cb->ctx;
+ struct net *net = sock_net(skb->sk);
+ unsigned long ifindex = 1;
+ struct net_device *dev;
+ int ret = 0;
+
+ rtnl_lock();
+
+ if (ctx->req_info.dev) {
+ ret = ethnl_phy_dump_one_dev(skb, ctx->req_info.dev, cb);
+ } else {
+ for_each_netdev_dump(net, dev, ifindex) {
+ ret = ethnl_phy_dump_one_dev(skb, dev, cb);
+ if (ret)
+ break;
+ }
+ }
+ rtnl_unlock();
+
+ if (ret == -EMSGSIZE && skb->len)
+ return skb->len;
+ return ret;
+}
+
--
2.41.0
Some netlink commands are target towards ethernet PHYs, to control some
of their features. As there's several such commands, add the ability to
pass a PHY index in the ethnl request, which will populate the generic
ethnl_req_info with the relevant phydev when the command targets a PHY.
Signed-off-by: Maxime Chevallier <[email protected]>
---
include/uapi/linux/ethtool_netlink.h | 1 +
net/ethtool/netlink.c | 22 ++++++++++++++++++++++
net/ethtool/netlink.h | 8 ++++++--
3 files changed, 29 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
index 73e2c10dc2cc..e557cf35250e 100644
--- a/include/uapi/linux/ethtool_netlink.h
+++ b/include/uapi/linux/ethtool_netlink.h
@@ -133,6 +133,7 @@ enum {
ETHTOOL_A_HEADER_DEV_INDEX, /* u32 */
ETHTOOL_A_HEADER_DEV_NAME, /* string */
ETHTOOL_A_HEADER_FLAGS, /* u32 - ETHTOOL_FLAG_* */
+ ETHTOOL_A_HEADER_PHY_INDEX, /* u32 */
/* add new constants above here */
__ETHTOOL_A_HEADER_CNT,
diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
index 3bbd5afb7b31..e83ee844b60f 100644
--- a/net/ethtool/netlink.c
+++ b/net/ethtool/netlink.c
@@ -4,6 +4,7 @@
#include <linux/ethtool_netlink.h>
#include <linux/pm_runtime.h>
#include "netlink.h"
+#include <linux/link_topology.h>
static struct genl_family ethtool_genl_family;
@@ -20,6 +21,7 @@ const struct nla_policy ethnl_header_policy[] = {
.len = ALTIFNAMSIZ - 1 },
[ETHTOOL_A_HEADER_FLAGS] = NLA_POLICY_MASK(NLA_U32,
ETHTOOL_FLAGS_BASIC),
+ [ETHTOOL_A_HEADER_PHY_INDEX] = NLA_POLICY_MIN(NLA_U32, 1),
};
const struct nla_policy ethnl_header_policy_stats[] = {
@@ -28,6 +30,7 @@ const struct nla_policy ethnl_header_policy_stats[] = {
.len = ALTIFNAMSIZ - 1 },
[ETHTOOL_A_HEADER_FLAGS] = NLA_POLICY_MASK(NLA_U32,
ETHTOOL_FLAGS_STATS),
+ [ETHTOOL_A_HEADER_PHY_INDEX] = NLA_POLICY_MIN(NLA_U32, 1),
};
int ethnl_ops_begin(struct net_device *dev)
@@ -91,6 +94,7 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
{
struct nlattr *tb[ARRAY_SIZE(ethnl_header_policy)];
const struct nlattr *devname_attr;
+ struct phy_device *phydev = NULL;
struct net_device *dev = NULL;
u32 flags = 0;
int ret;
@@ -145,6 +149,24 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
return -EINVAL;
}
+ if (dev) {
+ if (tb[ETHTOOL_A_HEADER_PHY_INDEX]) {
+ u32 phy_index = nla_get_u32(tb[ETHTOOL_A_HEADER_PHY_INDEX]);
+
+ phydev = link_topo_get_phy(&dev->link_topo, phy_index);
+ if (!phydev) {
+ NL_SET_ERR_MSG_ATTR(extack, header, "no phy matches phy index");
+ return -EINVAL;
+ }
+ } else {
+ /* If we need a PHY but no phy index is specified, fallback
+ * to dev->phydev
+ */
+ phydev = dev->phydev;
+ }
+ }
+
+ req_info->phydev = phydev;
req_info->dev = dev;
req_info->flags = flags;
return 0;
diff --git a/net/ethtool/netlink.h b/net/ethtool/netlink.h
index 9a333a8d04c1..3f5eb60bdf0b 100644
--- a/net/ethtool/netlink.h
+++ b/net/ethtool/netlink.h
@@ -250,6 +250,7 @@ static inline unsigned int ethnl_reply_header_size(void)
* @dev: network device the request is for (may be null)
* @dev_tracker: refcount tracker for @dev reference
* @flags: request flags common for all request types
+ * @phydev: phy_device connected to @dev this request is for (may be null)
*
* This is a common base for request specific structures holding data from
* parsed userspace request. These always embed struct ethnl_req_info at
@@ -259,6 +260,7 @@ struct ethnl_req_info {
struct net_device *dev;
netdevice_tracker dev_tracker;
u32 flags;
+ struct phy_device *phydev;
};
static inline void ethnl_parse_header_dev_put(struct ethnl_req_info *req_info)
@@ -395,9 +397,10 @@ extern const struct ethnl_request_ops ethnl_rss_request_ops;
extern const struct ethnl_request_ops ethnl_plca_cfg_request_ops;
extern const struct ethnl_request_ops ethnl_plca_status_request_ops;
extern const struct ethnl_request_ops ethnl_mm_request_ops;
+extern const struct ethnl_request_ops ethnl_phy_request_ops;
-extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_FLAGS + 1];
-extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_FLAGS + 1];
+extern const struct nla_policy ethnl_header_policy[ETHTOOL_A_HEADER_PHY_INDEX + 1];
+extern const struct nla_policy ethnl_header_policy_stats[ETHTOOL_A_HEADER_PHY_INDEX + 1];
extern const struct nla_policy ethnl_strset_get_policy[ETHTOOL_A_STRSET_COUNTS_ONLY + 1];
extern const struct nla_policy ethnl_linkinfo_get_policy[ETHTOOL_A_LINKINFO_HEADER + 1];
extern const struct nla_policy ethnl_linkinfo_set_policy[ETHTOOL_A_LINKINFO_TP_MDIX_CTRL + 1];
@@ -441,6 +444,7 @@ extern const struct nla_policy ethnl_plca_set_cfg_policy[ETHTOOL_A_PLCA_MAX + 1]
extern const struct nla_policy ethnl_plca_get_status_policy[ETHTOOL_A_PLCA_HEADER + 1];
extern const struct nla_policy ethnl_mm_get_policy[ETHTOOL_A_MM_HEADER + 1];
extern const struct nla_policy ethnl_mm_set_policy[ETHTOOL_A_MM_MAX + 1];
+extern const struct nla_policy ethnl_phy_get_policy[ETHTOOL_A_PHY_HEADER + 1];
int ethnl_set_features(struct sk_buff *skb, struct genl_info *info);
int ethnl_act_cable_test(struct sk_buff *skb, struct genl_info *info);
--
2.41.0
> + if (dev) {
> + if (tb[ETHTOOL_A_HEADER_PHY_INDEX]) {
> + u32 phy_index = nla_get_u32(tb[ETHTOOL_A_HEADER_PHY_INDEX]);
> +
> + phydev = link_topo_get_phy(&dev->link_topo, phy_index);
struct phy_device *link_topo_get_phy(struct link_topology *lt, int phyindex)
We have u32 vs int here for phyindex. It would be good to have the
same type everywhere.
> + if (!phydev) {
> + NL_SET_ERR_MSG_ATTR(extack, header, "no phy matches phy index");
> + return -EINVAL;
> + }
> + } else {
> + /* If we need a PHY but no phy index is specified, fallback
> + * to dev->phydev
> + */
> + phydev = dev->phydev;
> + }
> + }
> +
> + req_info->phydev = phydev;
Don't forget to update Documentation/networking/ethtool-netlink.rst.
Andrew
> diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst
> index 2540c70952ff..29ef675f45c0 100644
> --- a/Documentation/networking/ethtool-netlink.rst
> +++ b/Documentation/networking/ethtool-netlink.rst
> @@ -57,6 +57,7 @@ Structure of this header is
> ``ETHTOOL_A_HEADER_DEV_INDEX`` u32 device ifindex
> ``ETHTOOL_A_HEADER_DEV_NAME`` string device name
> ``ETHTOOL_A_HEADER_FLAGS`` u32 flags common for all requests
> + ``ETHTOOL_A_HEADER_PHY_INDEX`` u32 phy device index
> ============================== ====== =============================
>
> ``ETHTOOL_A_HEADER_DEV_INDEX`` and ``ETHTOOL_A_HEADER_DEV_NAME`` identify the
> @@ -81,6 +82,12 @@ the behaviour is backward compatible, i.e. requests from old clients not aware
> of the flag should be interpreted the way the client expects. A client must
> not set flags it does not understand.
>
> +``ETHTOOL_A_HEADER_PHY_INDEX`` identify the ethernet PHY the message relates to.
> +As there are numerous commands that are related to PHY configuration, and because
> +we can have more than one PHY on the link, the PHY index can be passed in the
> +request for the commands that needs it. It is however not mandatory, and if it
> +is not passed for commands that target a PHY, the net_device.phydev pointer
> +is used, as a fallback that keeps the legacy behaviour.
O.K, you did document it :-)
But i would make this part of the previous patch.
> +Kernel response contents:
> +
> + ================================= ====== ==========================
> + ``ETHTOOL_A_PHY_HEADER`` nested request header
> + ``ETHTOOL_A_PHY_INDEX`` u32 the phy's unique index, that can
> + be used for phy-specific requests
> + ``ETHTOOL_A_PHY_DRVNAME`` string the phy driver name
> + ``ETHTOOL_A_PHY_NAME`` string the phy device name
> + ``ETHTOOL_A_PHY_UPSTREAM_TYPE`` u32 the type of device this phy is
> + connected to
> + ``ETHTOOL_A_PHY_UPSTREAM_PHY`` nested if the phy is connected to another
> + phy, this nest contains info on
> + that connection
> + ``ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME`` string if the phy controls an sfp bus,
> + the name of the sfp bus
> + ``ETHTOOL_A_PHY_ID`` u32 the phy id if the phy is C22
Maybe a future extension. We could make phy_bus_match() set
phydev->phy_id to the ID it matched to the driver when doing C45. We
would then always have a value here.
> --- a/include/linux/ethtool_netlink.h
> +++ b/include/linux/ethtool_netlink.h
> @@ -118,5 +118,10 @@ static inline bool ethtool_dev_mm_supported(struct net_device *dev)
> return false;
> }
>
> +int ethnl_phy_dumpit(struct sk_buff *skb, struct netlink_callback *cb)
> +{
> + return -EOPNOTSUPP;
> +}
This is a header file, so should probably be static inline.
Andrew
> +ethnl_phy_reply_size(const struct ethnl_req_info *req_base,
> + struct netlink_ext_ack *extack)
> +{
> + struct phy_device_node *pdn;
> + struct phy_device *phydev;
> + struct link_topology *lt;
> + unsigned long index;
> + size_t size;
> +
> + lt = &req_base->dev->link_topo;
> +
> + size = nla_total_size(0);
> +
> + xa_for_each(<->phys, index, pdn) {
> + phydev = pdn->phy;
> +
> + /* ETHTOOL_A_PHY_INDEX */
> + size += nla_total_size(sizeof(u32));
> +
> + /* ETHTOOL_A_DRVNAME */
> + size += nla_total_size(strlen(phydev->drv->name));
> +
> + /* ETHTOOL_A_NAME */
> + size += nla_total_size(strlen(dev_name(&phydev->mdio.dev)));
> +
> + /* ETHTOOL_A_PHY_UPSTREAM_TYPE */
> + size += nla_total_size(sizeof(u8));
> +
> + /* ETHTOOL_A_PHY_ID */
> + size += nla_total_size(sizeof(u32));
> +
> + if (phy_on_sfp(phydev)) {
> + /* ETHTOOL_A_PHY_UPSTREAM_SFP_NAME */
> + if (sfp_get_name(pdn->parent_sfp_bus))
> + size += nla_total_size(strlen(sfp_get_name(pdn->parent_sfp_bus)));
Have you tried a modular build?
sfp_get_name() could be in a module, and then you will get linker
errors. It is all a bit messy calling into phylib from ethtool :-(
This might actually be the only function you need? If so, its small
enough you can move it into a header as a static inline function.
Andrew
On Tue, 21 Nov 2023 02:08:37 +0100
Andrew Lunn <[email protected]> wrote:
> > + if (dev) {
> > + if (tb[ETHTOOL_A_HEADER_PHY_INDEX]) {
> > + u32 phy_index = nla_get_u32(tb[ETHTOOL_A_HEADER_PHY_INDEX]);
> > +
> > + phydev = link_topo_get_phy(&dev->link_topo, phy_index);
>
> struct phy_device *link_topo_get_phy(struct link_topology *lt, int phyindex)
>
> We have u32 vs int here for phyindex. It would be good to have the
> same type everywhere.
Indeed I messed-up the typing for that variable, shame as it's the core
of that series :(
I'll get it right for the next version.
>
> > + if (!phydev) {
> > + NL_SET_ERR_MSG_ATTR(extack, header, "no phy matches phy index");
> > + return -EINVAL;
> > + }
> > + } else {
> > + /* If we need a PHY but no phy index is specified, fallback
> > + * to dev->phydev
> > + */
> > + phydev = dev->phydev;
> > + }
> > + }
> > +
> > + req_info->phydev = phydev;
>
> Don't forget to update Documentation/networking/ethtool-netlink.rst.
Yep I'll squeeze the documentation bit here.
Thanks for the review,
Maxime
> Andrew
On Tue, 21 Nov 2023 02:40:27 +0100
Andrew Lunn <[email protected]> wrote:
> > +ethnl_phy_reply_size(const struct ethnl_req_info *req_base,
> > + struct netlink_ext_ack *extack)
> > +{
> > + struct phy_device_node *pdn;
> > + struct phy_device *phydev;
> > + struct link_topology *lt;
> > + unsigned long index;
> > + size_t size;
> > +
> > + lt = &req_base->dev->link_topo;
> > +
> > + size = nla_total_size(0);
> > +
> > + xa_for_each(<->phys, index, pdn) {
> > + phydev = pdn->phy;
> > +
> > + /* ETHTOOL_A_PHY_INDEX */
> > + size += nla_total_size(sizeof(u32));
> > +
> > + /* ETHTOOL_A_DRVNAME */
> > + size += nla_total_size(strlen(phydev->drv->name));
> > +
> > + /* ETHTOOL_A_NAME */
> > + size += nla_total_size(strlen(dev_name(&phydev->mdio.dev)));
> > +
> > + /* ETHTOOL_A_PHY_UPSTREAM_TYPE */
> > + size += nla_total_size(sizeof(u8));
> > +
> > + /* ETHTOOL_A_PHY_ID */
> > + size += nla_total_size(sizeof(u32));
> > +
> > + if (phy_on_sfp(phydev)) {
> > + /* ETHTOOL_A_PHY_UPSTREAM_SFP_NAME */
> > + if (sfp_get_name(pdn->parent_sfp_bus))
> > + size += nla_total_size(strlen(sfp_get_name(pdn->parent_sfp_bus)));
>
> Have you tried a modular build?
Now that you mention it, no. I did try with CONFIG_SFP disabled, but
not as a module, I'll add it to my test suite.
>
> sfp_get_name() could be in a module, and then you will get linker
> errors. It is all a bit messy calling into phylib from ethtool :-(
>
> This might actually be the only function you need? If so, its small
> enough you can move it into a header as a static inline function.
It's the only one indeed, so as add it as a header function then.
Thanks,
Maxime
> Andrew