[PATCH ethtool-next v2 0/3] Introduce PHY listing and targeting

[PATCH ethtool-next v2 0/3] Introduce PHY listing and targeting

[Maxime Chevallier]

Hello,

This series adds the ethtool-side support to list PHYs associated to a
netdevice, as well as allowing to target PHYs for some commands :
 - PSE-PD commands
 - Cable testing commands
 - PLCA commands

This V2 uses the uAPI that got applied on the associated kernel-side
series [1], cleans a few lose-ends from the first version, and adds the
manpage modifications that was missing from V1.

The PHY-targetting commands look like this:

ethtool --phy 1 --cable-test eth0

Note that the --phy parameter gets passed at the beginning of the
command-line. This allows getting a generic command-line parsing code,
easy to write, but at the expense of maybe being a bit counter intuitive.

Another option could be to add a "phy" parameter to all the supported
commands, let me know if you think this looks too bad.

Patch 1 deals with the ability to pass a PHY index to the relevant
commands.

Patch 2 implements the --show-phys command. This command uses a netlink
DUMP request to list the PHYs, and introduces the ability to perform
filtered DUMP request, where the netdev index gets passed in the DUMP
request header.

Thanks,

Maxime

[1]: https://lore.kernel.org/netdev/20240821151009.1681151-1-maxime.chevallier@bootlin.com/

Link to V1: https://lore.kernel.org/netdev/20240103142950.235888-1-maxime.chevallier@bootlin.com/

Maxime Chevallier (3):
  update UAPI header copies
  ethtool: Allow passing a PHY index for phy-targetting commands
  ethtool: Introduce a command to list PHYs

 Makefile.am                  |   1 +
 ethtool.8.in                 |  56 +++++++++++++++++
 ethtool.c                    |  30 ++++++++-
 internal.h                   |   1 +
 netlink/cable_test.c         |   4 +-
 netlink/extapi.h             |   1 +
 netlink/msgbuff.c            |  52 ++++++++++++----
 netlink/msgbuff.h            |   3 +
 netlink/nlsock.c             |  38 ++++++++++++
 netlink/nlsock.h             |   2 +
 netlink/phy.c                | 116 +++++++++++++++++++++++++++++++++++
 netlink/plca.c               |   4 +-
 netlink/pse-pd.c             |   4 +-
 uapi/linux/ethtool.h         |  16 +++++
 uapi/linux/ethtool_netlink.h |  25 ++++++++
 15 files changed, 334 insertions(+), 19 deletions(-)
 create mode 100644 netlink/phy.c

-- 
2.45.2

[PATCH ethtool-next v2 1/3] update UAPI header copies

[Maxime Chevallier]

Update to kernel commit 7d3aed652d09.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 uapi/linux/ethtool.h         | 16 ++++++++++++++++
 uapi/linux/ethtool_netlink.h | 25 +++++++++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/uapi/linux/ethtool.h b/uapi/linux/ethtool.h
index fef07da..7022fcc 100644
--- a/uapi/linux/ethtool.h
+++ b/uapi/linux/ethtool.h
@@ -2531,4 +2531,20 @@ struct ethtool_link_settings {
 	 * __u32 map_lp_advertising[link_mode_masks_nwords];
 	 */
 };
+
+/**
+ * enum phy_upstream - Represents the upstream component a given PHY device
+ * is connected to, as in what is on the other end of the MII bus. Most PHYs
+ * will be attached to an Ethernet MAC controller, but in some cases, there's
+ * an intermediate PHY used as a media-converter, which will driver another
+ * MII interface as its output.
+ * @PHY_UPSTREAM_MAC: Upstream component is a MAC (a switch port,
+ *		      or ethernet controller)
+ * @PHY_UPSTREAM_PHY: Upstream component is a PHY (likely a media converter)
+ */
+enum phy_upstream {
+	PHY_UPSTREAM_MAC,
+	PHY_UPSTREAM_PHY,
+};
+
 #endif /* _LINUX_ETHTOOL_H */
diff --git a/uapi/linux/ethtool_netlink.h b/uapi/linux/ethtool_netlink.h
index dfc25a0..f865c7c 100644
--- a/uapi/linux/ethtool_netlink.h
+++ b/uapi/linux/ethtool_netlink.h
@@ -58,6 +58,7 @@ enum {
 	ETHTOOL_MSG_MM_GET,
 	ETHTOOL_MSG_MM_SET,
 	ETHTOOL_MSG_MODULE_FW_FLASH_ACT,
+	ETHTOOL_MSG_PHY_GET,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_USER_CNT,
@@ -111,6 +112,8 @@ enum {
 	ETHTOOL_MSG_MM_GET_REPLY,
 	ETHTOOL_MSG_MM_NTF,
 	ETHTOOL_MSG_MODULE_FW_FLASH_NTF,
+	ETHTOOL_MSG_PHY_GET_REPLY,
+	ETHTOOL_MSG_PHY_NTF,
 
 	/* add new constants above here */
 	__ETHTOOL_MSG_KERNEL_CNT,
@@ -134,6 +137,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,
@@ -556,6 +560,10 @@ enum {
 	 * a regular 100 Ohm cable and a part with the abnormal impedance value
 	 */
 	ETHTOOL_A_CABLE_RESULT_CODE_IMPEDANCE_MISMATCH,
+	/* TDR not possible due to high noise level */
+	ETHTOOL_A_CABLE_RESULT_CODE_NOISE,
+	/* TDR resolution not possible / out of distance */
+	ETHTOOL_A_CABLE_RESULT_CODE_RESOLUTION_NOT_POSSIBLE,
 };
 
 enum {
@@ -965,6 +973,7 @@ enum {
 	ETHTOOL_A_RSS_INDIR,		/* binary */
 	ETHTOOL_A_RSS_HKEY,		/* binary */
 	ETHTOOL_A_RSS_INPUT_XFRM,	/* u32 */
+	ETHTOOL_A_RSS_START_CONTEXT,	/* u32 */
 
 	__ETHTOOL_A_RSS_CNT,
 	ETHTOOL_A_RSS_MAX = (__ETHTOOL_A_RSS_CNT - 1),
@@ -1049,6 +1058,22 @@ enum {
 	ETHTOOL_A_MODULE_FW_FLASH_MAX = (__ETHTOOL_A_MODULE_FW_FLASH_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,		/* u32 */
+	ETHTOOL_A_PHY_UPSTREAM_INDEX,		/* u32 */
+	ETHTOOL_A_PHY_UPSTREAM_SFP_NAME,	/* string */
+	ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME,	/* string */
+
+	/* 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
-- 
2.45.2

[PATCH ethtool-next v2 2/3] ethtool: Allow passing a PHY index for phy-targetting commands

[Maxime Chevallier]

With the introduction of PHY topology and the ability to list PHYs, we
can now target some netlink commands to specific PHYs. This is done by
passing a PHY index as a request parameter in the netlink GET command.

This is useful for PSE-PD, PLCA and Cable-testing operations when
multiple PHYs are on the link (e.g. when a PHY is used as an SFP
upstream controller, and when there's another PHY within the SFP
module).

Introduce a new, generic, option "--phy N" that can be used in
conjunction with PHY-targetting commands to pass the PHY index for the
targetted PHY.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 ethtool.8.in         | 20 +++++++++++++++++
 ethtool.c            | 25 ++++++++++++++++++++-
 internal.h           |  1 +
 netlink/cable_test.c |  4 ++--
 netlink/msgbuff.c    | 52 ++++++++++++++++++++++++++++++++++----------
 netlink/msgbuff.h    |  3 +++
 netlink/nlsock.c     |  3 ++-
 netlink/plca.c       |  4 ++--
 netlink/pse-pd.c     |  4 ++--
 9 files changed, 96 insertions(+), 20 deletions(-)

diff --git a/ethtool.8.in b/ethtool.8.in
index 11bb0f9..a455b5d 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -143,6 +143,10 @@ ethtool \- query or control network driver and hardware settings
 .B ethtool [-I | --include-statistics]
 .I args
 .HP
+.B ethtool
+.BN --phy
+.I args
+.HP
 .B ethtool \-\-monitor
 [
 .I command
@@ -588,6 +592,22 @@ plain text in the presence of this option.
 Include command-related statistics in the output. This option allows
 displaying relevant device statistics for selected get commands.
 .TP
+.BI \-\-phy \ N
+Target a PHY within the interface. The PHY index can be retrieved with
+.B \-\-show\-phys.
+The following commands can accept a PHY index:
+.TS
+nokeep;
+lB	l.
+\-\-cable\-test
+\-\-cable\-test\-tdr
+\-\-get\-plca\-cfg
+\-\-set\-plca\-cfg
+\-\-get\-plca\-status
+\-\-show-pse
+\-\-set-pse
+.TE
+.TP
 .B \-a \-\-show\-pause
 Queries the specified Ethernet device for pause parameter information.
 .RS 4
diff --git a/ethtool.c b/ethtool.c
index 7f47407..3bd777e 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -5739,6 +5739,7 @@ struct option {
 	const char	*opts;
 	bool		no_dev;
 	bool		json;
+	bool		targets_phy;
 	int		(*func)(struct cmd_context *);
 	nl_chk_t	nlchk;
 	nl_func_t	nlfunc;
@@ -6158,12 +6159,14 @@ static const struct option args[] = {
 	},
 	{
 		.opts	= "--cable-test",
+		.targets_phy	= true,
 		.json	= true,
 		.nlfunc	= nl_cable_test,
 		.help	= "Perform a cable test",
 	},
 	{
 		.opts	= "--cable-test-tdr",
+		.targets_phy	= true,
 		.json	= true,
 		.nlfunc	= nl_cable_test_tdr,
 		.help	= "Print cable test time domain reflectrometery data",
@@ -6191,11 +6194,13 @@ static const struct option args[] = {
 	},
 	{
 		.opts	= "--get-plca-cfg",
+		.targets_phy	= true,
 		.nlfunc	= nl_plca_get_cfg,
 		.help	= "Get PLCA configuration",
 	},
 	{
 		.opts	= "--set-plca-cfg",
+		.targets_phy	= true,
 		.nlfunc	= nl_plca_set_cfg,
 		.help	= "Set PLCA configuration",
 		.xhelp  = "		[ enable on|off ]\n"
@@ -6207,6 +6212,7 @@ static const struct option args[] = {
 	},
 	{
 		.opts	= "--get-plca-status",
+		.targets_phy	= true,
 		.nlfunc	= nl_plca_get_status,
 		.help	= "Get PLCA status information",
 	},
@@ -6228,12 +6234,14 @@ static const struct option args[] = {
 	},
 	{
 		.opts	= "--show-pse",
+		.targets_phy	= true,
 		.json	= true,
 		.nlfunc	= nl_gpse,
 		.help	= "Show settings for Power Sourcing Equipment",
 	},
 	{
 		.opts	= "--set-pse",
+		.targets_phy	= true,
 		.nlfunc	= nl_spse,
 		.help	= "Set Power Sourcing Equipment settings",
 		.xhelp	= "		[ podl-pse-admin-control enable|disable ]\n"
@@ -6270,7 +6278,8 @@ static int show_usage(struct cmd_context *ctx __maybe_unused)
 	fprintf(stdout,	"Usage:\n");
 	for (i = 0; args[i].opts; i++) {
 		fputs("        ethtool [ FLAGS ] ", stdout);
-		fprintf(stdout, "%s %s\t%s\n",
+		fprintf(stdout, "%s%s %s\t%s\n",
+			args[i].targets_phy ? "[ --phy PHY ] " : "",
 			args[i].opts,
 			args[i].no_dev ? "\t" : "DEVNAME",
 			args[i].help);
@@ -6550,6 +6559,16 @@ int main(int argc, char **argp)
 			argc -= 1;
 			continue;
 		}
+		if (*argp && !strcmp(*argp, "--phy")) {
+			char *eptr;
+
+			ctx.phy_index = strtoul(argp[1], &eptr, 0);
+			if (!argp[1][0] || *eptr)
+				exit_bad_args();
+			argp += 2;
+			argc -= 2;
+			continue;
+		}
 		break;
 	}
 	if (*argp && !strcmp(*argp, "--monitor")) {
@@ -6585,6 +6604,10 @@ int main(int argc, char **argp)
 	}
 	if (ctx.json && !args[k].json)
 		exit_bad_args_info("JSON output not available for this subcommand");
+
+	if (!args[k].targets_phy && ctx.phy_index)
+		exit_bad_args();
+
 	ctx.argc = argc;
 	ctx.argp = argp;
 	netlink_run_handler(&ctx, args[k].nlchk, args[k].nlfunc, !args[k].func);
diff --git a/internal.h b/internal.h
index 4b994f5..afb8121 100644
--- a/internal.h
+++ b/internal.h
@@ -222,6 +222,7 @@ struct cmd_context {
 	unsigned long debug;	/* debugging mask */
 	bool json;		/* Output JSON, if supported */
 	bool show_stats;	/* include command-specific stats */
+	uint32_t phy_index;	/* the phy index this command targets */
 #ifdef ETHTOOL_ENABLE_NETLINK
 	struct nl_context *nlctx;	/* netlink context (opaque) */
 #endif
diff --git a/netlink/cable_test.c b/netlink/cable_test.c
index 9305a47..ba21c6c 100644
--- a/netlink/cable_test.c
+++ b/netlink/cable_test.c
@@ -572,8 +572,8 @@ int nl_cable_test_tdr(struct cmd_context *ctx)
 	if (ret < 0)
 		return 2;
 
-	if (ethnla_fill_header(msgbuff, ETHTOOL_A_CABLE_TEST_TDR_HEADER,
-			       ctx->devname, 0))
+	if (ethnla_fill_header_phy(msgbuff, ETHTOOL_A_CABLE_TEST_TDR_HEADER,
+				   ctx->devname, ctx->phy_index, 0))
 		return -EMSGSIZE;
 
 	ret = nl_parser(nlctx, tdr_params, NULL, PARSER_GROUP_NEST, NULL);
diff --git a/netlink/msgbuff.c b/netlink/msgbuff.c
index 216f5b9..2275840 100644
--- a/netlink/msgbuff.c
+++ b/netlink/msgbuff.c
@@ -138,17 +138,9 @@ struct nlattr *ethnla_nest_start(struct nl_msg_buff *msgbuff, uint16_t type)
 	return NULL;
 }
 
-/**
- * ethnla_fill_header() - write standard ethtool request header to message
- * @msgbuff: message buffer
- * @type:    attribute type for header nest
- * @devname: device name (NULL to omit)
- * @flags:   request flags (omitted if 0)
- *
- * Return: pointer to the nest attribute or null of error
- */
-bool ethnla_fill_header(struct nl_msg_buff *msgbuff, uint16_t type,
-			const char *devname, uint32_t flags)
+static bool __ethnla_fill_header_phy(struct nl_msg_buff *msgbuff, uint16_t type,
+				     const char *devname, uint32_t phy_index,
+				     uint32_t flags)
 {
 	struct nlattr *nest;
 
@@ -159,7 +151,9 @@ bool ethnla_fill_header(struct nl_msg_buff *msgbuff, uint16_t type,
 	if ((devname &&
 	     ethnla_put_strz(msgbuff, ETHTOOL_A_HEADER_DEV_NAME, devname)) ||
 	    (flags &&
-	     ethnla_put_u32(msgbuff, ETHTOOL_A_HEADER_FLAGS, flags)))
+	     ethnla_put_u32(msgbuff, ETHTOOL_A_HEADER_FLAGS, flags)) ||
+	    (phy_index &&
+	     ethnla_put_u32(msgbuff, ETHTOOL_A_HEADER_PHY_INDEX, phy_index)))
 		goto err;
 
 	ethnla_nest_end(msgbuff, nest);
@@ -170,6 +164,40 @@ err:
 	return true;
 }
 
+/**
+ * ethnla_fill_header() - write standard ethtool request header to message
+ * @msgbuff: message buffer
+ * @type:    attribute type for header nest
+ * @devname: device name (NULL to omit)
+ * @flags:   request flags (omitted if 0)
+ *
+ * Return: pointer to the nest attribute or null of error
+ */
+bool ethnla_fill_header(struct nl_msg_buff *msgbuff, uint16_t type,
+			const char *devname, uint32_t flags)
+{
+	return __ethnla_fill_header_phy(msgbuff, type, devname, 0, flags);
+}
+
+/**
+ * ethnla_fill_header_phy() - write standard ethtool request header to message,
+ *			      targetting a device's phy
+ * @msgbuff: message buffer
+ * @type:    attribute type for header nest
+ * @devname: device name (NULL to omit)
+ * @phy_index: phy index to target (0 to omit)
+ * @flags:   request flags (omitted if 0)
+ *
+ * Return: pointer to the nest attribute or null of error
+ */
+bool ethnla_fill_header_phy(struct nl_msg_buff *msgbuff, uint16_t type,
+			    const char *devname, uint32_t phy_index,
+			    uint32_t flags)
+{
+	return __ethnla_fill_header_phy(msgbuff, type, devname, phy_index,
+					flags);
+}
+
 /**
  * __msg_init() - init a genetlink message, fill netlink and genetlink header
  * @msgbuff: message buffer
diff --git a/netlink/msgbuff.h b/netlink/msgbuff.h
index 7d6731f..7df19fc 100644
--- a/netlink/msgbuff.h
+++ b/netlink/msgbuff.h
@@ -47,6 +47,9 @@ bool ethnla_put(struct nl_msg_buff *msgbuff, uint16_t type, size_t len,
 struct nlattr *ethnla_nest_start(struct nl_msg_buff *msgbuff, uint16_t type);
 bool ethnla_fill_header(struct nl_msg_buff *msgbuff, uint16_t type,
 			const char *devname, uint32_t flags);
+bool ethnla_fill_header_phy(struct nl_msg_buff *msgbuff, uint16_t type,
+			    const char *devname, uint32_t phy_index,
+			    uint32_t flags);
 
 /* length of current message */
 static inline unsigned int msgbuff_len(const struct nl_msg_buff *msgbuff)
diff --git a/netlink/nlsock.c b/netlink/nlsock.c
index 0ec2738..0b873a3 100644
--- a/netlink/nlsock.c
+++ b/netlink/nlsock.c
@@ -291,7 +291,8 @@ int nlsock_prep_get_request(struct nl_socket *nlsk, unsigned int nlcmd,
 	ret = msg_init(nlctx, &nlsk->msgbuff, nlcmd, nlm_flags);
 	if (ret < 0)
 		return ret;
-	if (ethnla_fill_header(&nlsk->msgbuff, hdr_attrtype, devname, flags))
+	if (ethnla_fill_header_phy(&nlsk->msgbuff, hdr_attrtype, devname,
+				   nlctx->ctx->phy_index, flags))
 		return -EMSGSIZE;
 
 	return 0;
diff --git a/netlink/plca.c b/netlink/plca.c
index 7d61e3b..7dc30a3 100644
--- a/netlink/plca.c
+++ b/netlink/plca.c
@@ -211,8 +211,8 @@ int nl_plca_set_cfg(struct cmd_context *ctx)
 		       NLM_F_REQUEST | NLM_F_ACK);
 	if (ret < 0)
 		return 2;
-	if (ethnla_fill_header(msgbuff, ETHTOOL_A_PLCA_HEADER,
-			       ctx->devname, 0))
+	if (ethnla_fill_header_phy(msgbuff, ETHTOOL_A_PLCA_HEADER,
+				   ctx->devname, ctx->phy_index, 0))
 		return -EMSGSIZE;
 
 	ret = nl_parser(nlctx, set_plca_params, NULL, PARSER_GROUP_NONE, NULL);
diff --git a/netlink/pse-pd.c b/netlink/pse-pd.c
index 2c8dd89..3f6b6aa 100644
--- a/netlink/pse-pd.c
+++ b/netlink/pse-pd.c
@@ -240,8 +240,8 @@ int nl_spse(struct cmd_context *ctx)
 		       NLM_F_REQUEST | NLM_F_ACK);
 	if (ret < 0)
 		return 2;
-	if (ethnla_fill_header(msgbuff, ETHTOOL_A_PSE_HEADER,
-			       ctx->devname, 0))
+	if (ethnla_fill_header_phy(msgbuff, ETHTOOL_A_PSE_HEADER,
+				   ctx->devname, ctx->phy_index, 0))
 		return -EMSGSIZE;
 
 	ret = nl_parser(nlctx, spse_params, NULL, PARSER_GROUP_NONE, NULL);
-- 
2.45.2

[PATCH ethtool-next v2 3/3] ethtool: Introduce a command to list PHYs

[Maxime Chevallier]

It is now possible to list all Ethernet PHYs that are present behind a
given interface, since the following linux commit :
63d5eaf35ac3 ("net: ethtool: Introduce a command to list PHYs on an interface")

This command relies on the netlink DUMP command to list them, by allowing to
pass an interface name/id as a parameter in the DUMP request to only
list PHYs on one interface.

Therefore, we introduce a new helper function to prepare a interface-filtered
dump request (the filter can be empty, to perform an unfiltered dump),
and then uses it to implement PHY enumeration through the --show-phys
command.

Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
 Makefile.am      |   1 +
 ethtool.8.in     |  36 +++++++++++++++
 ethtool.c        |   5 ++
 netlink/extapi.h |   1 +
 netlink/nlsock.c |  37 +++++++++++++++
 netlink/nlsock.h |   2 +
 netlink/phy.c    | 116 +++++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 198 insertions(+)
 create mode 100644 netlink/phy.c

diff --git a/Makefile.am b/Makefile.am
index 5a61a9a..862886b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -48,6 +48,7 @@ ethtool_SOURCES += \
 		  netlink/desc-rtnl.c netlink/cable_test.c netlink/tunnels.c \
 		  netlink/plca.c \
 		  netlink/pse-pd.c \
+		  netlink/phy.c \
 		  uapi/linux/ethtool_netlink.h \
 		  uapi/linux/netlink.h uapi/linux/genetlink.h \
 		  uapi/linux/rtnetlink.h uapi/linux/if_link.h \
diff --git a/ethtool.8.in b/ethtool.8.in
index a455b5d..816ac22 100644
--- a/ethtool.8.in
+++ b/ethtool.8.in
@@ -547,6 +547,9 @@ ethtool \- query or control network driver and hardware settings
 .IR FILE
 .RB [ pass
 .IR PASS ]
+.HP
+.B ethtool \-\-show\-phys
+.I devname
 .
 .\" Adjust lines (i.e. full justification) and hyphenate.
 .ad
@@ -1822,6 +1825,39 @@ is downloaded to the transceiver module, validated, run and committed.
 Optional transceiver module password that might be required as part of the
 transceiver module firmware update process.
 
+.RE
+.TP
+.B \-\-show\-phys
+Show the PHY devices attached to an interface, and the way they link together.
+.RS 4
+.TP
+.B phy_index
+The PHY's index, that identifies it within the network interface. If the
+interface has multiple PHYs, they will each have a unique index on that
+interface. This index can then be used for commands that targets PHYs.
+.TP
+.B drvname
+The name of the driver bound to this PHY device.
+.TP
+.B name
+The PHY's device name, matching the name found in sysfs.
+.TP
+.B downstream_sfp_name
+If the PHY drives an SFP cage, this field contains the name of the associated
+SFP bus.
+.TP
+.B upstream_type \ mac | phy
+Indicates the nature of the device the PHY is attached to.
+.TP
+.B upstream_index
+If the PHY's upstream_type is
+.B phy
+, this field indicates the phy_index of the upstream phy.
+.TP
+.B upstream_sfp_name
+If the PHY is withing an SFP/SFF module, this field contains the name of the
+upstream SFP bus.
+
 .SH BUGS
 Not supported (in part or whole) on all network drivers.
 .SH AUTHOR
diff --git a/ethtool.c b/ethtool.c
index 3bd777e..3591767 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -6254,6 +6254,11 @@ static const struct option args[] = {
 		.xhelp	= "		file FILE\n"
 			  "		[ pass PASS ]\n"
 	},
+	{
+		.opts	= "--show-phys",
+		.nlfunc	= nl_get_phy,
+		.help	= "List PHYs"
+	},
 	{
 		.opts	= "-h|--help",
 		.no_dev	= true,
diff --git a/netlink/extapi.h b/netlink/extapi.h
index c882295..fd99610 100644
--- a/netlink/extapi.h
+++ b/netlink/extapi.h
@@ -56,6 +56,7 @@ int nl_set_mm(struct cmd_context *ctx);
 int nl_gpse(struct cmd_context *ctx);
 int nl_spse(struct cmd_context *ctx);
 int nl_flash_module_fw(struct cmd_context *ctx);
+int nl_get_phy(struct cmd_context *ctx);
 
 void nl_monitor_usage(void);
 
diff --git a/netlink/nlsock.c b/netlink/nlsock.c
index 0b873a3..5450c9b 100644
--- a/netlink/nlsock.c
+++ b/netlink/nlsock.c
@@ -298,6 +298,43 @@ int nlsock_prep_get_request(struct nl_socket *nlsk, unsigned int nlcmd,
 	return 0;
 }
 
+/**
+ * nlsock_prep_filtered_dump_request() - Initialize a filtered DUMP request
+ * @nlsk: netlink socket
+ * @nlcmd: netlink command
+ * @hdr_attrtype: netlink command header attribute
+ * @flags: netlink command header flags
+ *
+ * Prepare a DUMP request that may include the device index as a filtering
+ * attribute in the header.
+ *
+ * Return: 0 on success, or a negative number on error
+ */
+int nlsock_prep_filtered_dump_request(struct nl_socket *nlsk,
+				      unsigned int nlcmd, uint16_t hdr_attrtype,
+				      u32 flags)
+{
+	struct nl_context *nlctx = nlsk->nlctx;
+	const char *devname = nlctx->ctx->devname;
+	unsigned int nlm_flags;
+	int ret;
+
+	nlctx->is_dump = true;
+	nlm_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_DUMP;
+
+	if (devname && !strcmp(devname, WILDCARD_DEVNAME))
+		devname = NULL;
+
+	ret = msg_init(nlctx, &nlsk->msgbuff, nlcmd, nlm_flags);
+	if (ret < 0)
+		return ret;
+
+	if (ethnla_fill_header(&nlsk->msgbuff, hdr_attrtype, devname, flags))
+		return -EMSGSIZE;
+
+	return 0;
+}
+
 #ifndef TEST_ETHTOOL
 /**
  * nlsock_sendmsg() - send a netlink message to kernel
diff --git a/netlink/nlsock.h b/netlink/nlsock.h
index b015f86..6a72966 100644
--- a/netlink/nlsock.h
+++ b/netlink/nlsock.h
@@ -38,6 +38,8 @@ int nlsock_init(struct nl_context *nlctx, struct nl_socket **__nlsk,
 void nlsock_done(struct nl_socket *nlsk);
 int nlsock_prep_get_request(struct nl_socket *nlsk, unsigned int nlcmd,
 			    uint16_t hdr_attrtype, u32 flags);
+int nlsock_prep_filtered_dump_request(struct nl_socket *nlsk, unsigned int nlcmd,
+				      uint16_t hdr_attrtype, u32 flags);
 ssize_t nlsock_sendmsg(struct nl_socket *nlsk, struct nl_msg_buff *__msgbuff);
 int nlsock_send_get_request(struct nl_socket *nlsk, mnl_cb_t cb);
 int nlsock_process_reply(struct nl_socket *nlsk, mnl_cb_t reply_cb, void *data);
diff --git a/netlink/phy.c b/netlink/phy.c
new file mode 100644
index 0000000..7578191
--- /dev/null
+++ b/netlink/phy.c
@@ -0,0 +1,116 @@
+/*
+ * phy.c - List PHYs on an interface and their parameters
+ *
+ * Implementation of "ethtool --show-phys <dev>"
+ */
+
+#include <errno.h>
+#include <inttypes.h>
+#include <string.h>
+#include <stdio.h>
+
+#include "../internal.h"
+#include "../common.h"
+#include "netlink.h"
+
+/* PHY_GET / PHY_DUMP */
+
+static const char * phy_upstream_type_to_str(uint8_t upstream_type)
+{
+	switch (upstream_type) {
+	case PHY_UPSTREAM_PHY: return "phy";
+	case PHY_UPSTREAM_MAC: return "mac";
+	default: return "Unknown";
+	}
+}
+
+int phy_reply_cb(const struct nlmsghdr *nlhdr, void *data)
+{
+	const struct nlattr *tb[ETHTOOL_A_PHY_MAX + 1] = {};
+	struct nl_context *nlctx = data;
+	DECLARE_ATTR_TB_INFO(tb);
+	uint8_t upstream_type;
+	bool silent;
+	int err_ret;
+	int ret;
+
+	silent = nlctx->is_dump || nlctx->is_monitor;
+	err_ret = silent ? MNL_CB_OK : MNL_CB_ERROR;
+	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
+	if (ret < 0)
+		return err_ret;
+	nlctx->devname = get_dev_name(tb[ETHTOOL_A_PHY_HEADER]);
+	if (!dev_ok(nlctx))
+		return err_ret;
+
+	if (silent)
+		print_nl();
+
+	open_json_object(NULL);
+
+	print_string(PRINT_ANY, "ifname", "PHY for %s:\n", nlctx->devname);
+
+	show_u32("phy_index", "PHY index: ", tb[ETHTOOL_A_PHY_INDEX]);
+
+	if (tb[ETHTOOL_A_PHY_DRVNAME])
+		print_string(PRINT_ANY, "drvname", "Driver name: %s\n",
+		     mnl_attr_get_str(tb[ETHTOOL_A_PHY_DRVNAME]));
+
+	if (tb[ETHTOOL_A_PHY_NAME])
+		print_string(PRINT_ANY, "name", "PHY device name: %s\n",
+		     mnl_attr_get_str(tb[ETHTOOL_A_PHY_NAME]));
+
+	if (tb[ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME])
+		print_string(PRINT_ANY, "downstream_sfp_name",
+			     "Downstream SFP bus name: %s\n",
+			     mnl_attr_get_str(tb[ETHTOOL_A_PHY_DOWNSTREAM_SFP_NAME]));
+
+	if (tb[ETHTOOL_A_PHY_UPSTREAM_TYPE]) {
+		upstream_type = mnl_attr_get_u8(tb[ETHTOOL_A_PHY_UPSTREAM_TYPE]);
+		print_string(PRINT_ANY, "upstream_type", "Upstream type: %s\n",
+			     phy_upstream_type_to_str(upstream_type));
+	}
+
+	if (tb[ETHTOOL_A_PHY_UPSTREAM_INDEX])
+		show_u32("upstream_index", "Upstream PHY index: ",
+			 tb[ETHTOOL_A_PHY_UPSTREAM_INDEX]);
+
+	if (tb[ETHTOOL_A_PHY_UPSTREAM_SFP_NAME])
+		print_string(PRINT_ANY, "upstream_sfp_name", "Upstream SFP name: %s\n",
+			     mnl_attr_get_str(tb[ETHTOOL_A_PHY_UPSTREAM_SFP_NAME]));
+
+	if (!silent)
+		print_nl();
+
+	close_json_object();
+
+	return MNL_CB_OK;
+
+	close_json_object();
+	return err_ret;
+}
+
+int nl_get_phy(struct cmd_context *ctx)
+{
+	struct nl_context *nlctx = ctx->nlctx;
+	struct nl_socket *nlsk = nlctx->ethnl_socket;
+	int ret;
+
+	if (netlink_cmd_check(ctx, ETHTOOL_MSG_PHY_GET, true))
+		return -EOPNOTSUPP;
+	if (ctx->argc > 0) {
+		fprintf(stderr, "ethtool: unexpected parameter '%s'\n",
+			*ctx->argp);
+		return 1;
+	}
+
+	ret = nlsock_prep_filtered_dump_request(nlsk, ETHTOOL_MSG_PHY_GET,
+						ETHTOOL_A_PHY_HEADER, 0);
+	if (ret)
+		return ret;
+
+	new_json_obj(ctx->json);
+	ret = nlsock_send_get_request(nlsk, phy_reply_cb);
+	delete_json_obj();
+	return ret;
+}
-- 
2.45.2

Re: [PATCH ethtool-next v2 2/3] ethtool: Allow passing a PHY index for phy-targetting commands

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 3654 bytes --]

On Wed, Aug 28, 2024 at 05:25:09PM +0200, Maxime Chevallier wrote:
> With the introduction of PHY topology and the ability to list PHYs, we
> can now target some netlink commands to specific PHYs. This is done by
> passing a PHY index as a request parameter in the netlink GET command.
> 
> This is useful for PSE-PD, PLCA and Cable-testing operations when
> multiple PHYs are on the link (e.g. when a PHY is used as an SFP
> upstream controller, and when there's another PHY within the SFP
> module).
> 
> Introduce a new, generic, option "--phy N" that can be used in
> conjunction with PHY-targetting commands to pass the PHY index for the
> targetted PHY.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
>  ethtool.8.in         | 20 +++++++++++++++++
>  ethtool.c            | 25 ++++++++++++++++++++-
>  internal.h           |  1 +
>  netlink/cable_test.c |  4 ++--
>  netlink/msgbuff.c    | 52 ++++++++++++++++++++++++++++++++++----------
>  netlink/msgbuff.h    |  3 +++
>  netlink/nlsock.c     |  3 ++-
>  netlink/plca.c       |  4 ++--
>  netlink/pse-pd.c     |  4 ++--
>  9 files changed, 96 insertions(+), 20 deletions(-)
> 
[...]
> @@ -6550,6 +6559,16 @@ int main(int argc, char **argp)
>  			argc -= 1;
>  			continue;
>  		}
> +		if (*argp && !strcmp(*argp, "--phy")) {
> +			char *eptr;
> +
> +			ctx.phy_index = strtoul(argp[1], &eptr, 0);
> +			if (!argp[1][0] || *eptr)
> +				exit_bad_args();
> +			argp += 2;
> +			argc -= 2;
> +			continue;
> +		}
>  		break;
>  	}
>  	if (*argp && !strcmp(*argp, "--monitor")) {

Could we have a meaningful error message that would tell user what was
wrong instead?

> @@ -6585,6 +6604,10 @@ int main(int argc, char **argp)
>  	}
>  	if (ctx.json && !args[k].json)
>  		exit_bad_args_info("JSON output not available for this subcommand");
> +
> +	if (!args[k].targets_phy && ctx.phy_index)
> +		exit_bad_args();
> +
>  	ctx.argc = argc;
>  	ctx.argp = argp;
>  	netlink_run_handler(&ctx, args[k].nlchk, args[k].nlfunc, !args[k].func);

Same here.

[...]
> diff --git a/netlink/msgbuff.c b/netlink/msgbuff.c
> index 216f5b9..2275840 100644
> --- a/netlink/msgbuff.c
> +++ b/netlink/msgbuff.c
> @@ -138,17 +138,9 @@ struct nlattr *ethnla_nest_start(struct nl_msg_buff *msgbuff, uint16_t type)
>  	return NULL;
>  }
>  
> -/**
> - * ethnla_fill_header() - write standard ethtool request header to message
> - * @msgbuff: message buffer
> - * @type:    attribute type for header nest
> - * @devname: device name (NULL to omit)
> - * @flags:   request flags (omitted if 0)
> - *
> - * Return: pointer to the nest attribute or null of error
> - */
> -bool ethnla_fill_header(struct nl_msg_buff *msgbuff, uint16_t type,
> -			const char *devname, uint32_t flags)
> +static bool __ethnla_fill_header_phy(struct nl_msg_buff *msgbuff, uint16_t type,
> +				     const char *devname, uint32_t phy_index,
> +				     uint32_t flags)
>  {
>  	struct nlattr *nest;
>  
> @@ -159,7 +151,9 @@ bool ethnla_fill_header(struct nl_msg_buff *msgbuff, uint16_t type,
>  	if ((devname &&
>  	     ethnla_put_strz(msgbuff, ETHTOOL_A_HEADER_DEV_NAME, devname)) ||
>  	    (flags &&
> -	     ethnla_put_u32(msgbuff, ETHTOOL_A_HEADER_FLAGS, flags)))
> +	     ethnla_put_u32(msgbuff, ETHTOOL_A_HEADER_FLAGS, flags)) ||
> +	    (phy_index &&
> +	     ethnla_put_u32(msgbuff, ETHTOOL_A_HEADER_PHY_INDEX, phy_index)))
>  		goto err;
>  
>  	ethnla_nest_end(msgbuff, nest);

Just to be sure: are we sure the PHY index cannot ever be zero (or that
we won't need to pass 0 index to kernel)?

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool-next v2 3/3] ethtool: Introduce a command to list PHYs

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 1386 bytes --]

On Wed, Aug 28, 2024 at 05:25:10PM +0200, Maxime Chevallier wrote:
> It is now possible to list all Ethernet PHYs that are present behind a
> given interface, since the following linux commit :
> 63d5eaf35ac3 ("net: ethtool: Introduce a command to list PHYs on an interface")
> 
> This command relies on the netlink DUMP command to list them, by allowing to
> pass an interface name/id as a parameter in the DUMP request to only
> list PHYs on one interface.
> 
> Therefore, we introduce a new helper function to prepare a interface-filtered
> dump request (the filter can be empty, to perform an unfiltered dump),
> and then uses it to implement PHY enumeration through the --show-phys
> command.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> ---
[...]
> diff --git a/netlink/extapi.h b/netlink/extapi.h
> index c882295..fd99610 100644
> --- a/netlink/extapi.h
> +++ b/netlink/extapi.h
> @@ -56,6 +56,7 @@ int nl_set_mm(struct cmd_context *ctx);
>  int nl_gpse(struct cmd_context *ctx);
>  int nl_spse(struct cmd_context *ctx);
>  int nl_flash_module_fw(struct cmd_context *ctx);
> +int nl_get_phy(struct cmd_context *ctx);
>  
>  void nl_monitor_usage(void);
>  

Please add also a fallback to !ETHTOOL_ENABLE_NETLINK branch, similar
to other netlink handlers, so that a build with --disable-netlink does
not fail.

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool-next v2 2/3] ethtool: Allow passing a PHY index for phy-targetting commands

[Maxime Chevallier]

Hello Michal,

On Mon, 2 Sep 2024 00:04:39 +0200
Michal Kubecek <mkubecek@suse.cz> wrote:

> On Wed, Aug 28, 2024 at 05:25:09PM +0200, Maxime Chevallier wrote:
> > With the introduction of PHY topology and the ability to list PHYs, we
> > can now target some netlink commands to specific PHYs. This is done by
> > passing a PHY index as a request parameter in the netlink GET command.
> > 
> > This is useful for PSE-PD, PLCA and Cable-testing operations when
> > multiple PHYs are on the link (e.g. when a PHY is used as an SFP
> > upstream controller, and when there's another PHY within the SFP
> > module).
> > 
> > Introduce a new, generic, option "--phy N" that can be used in
> > conjunction with PHY-targetting commands to pass the PHY index for the
> > targetted PHY.
> > 
> > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > ---
> >  ethtool.8.in         | 20 +++++++++++++++++
> >  ethtool.c            | 25 ++++++++++++++++++++-
> >  internal.h           |  1 +
> >  netlink/cable_test.c |  4 ++--
> >  netlink/msgbuff.c    | 52 ++++++++++++++++++++++++++++++++++----------
> >  netlink/msgbuff.h    |  3 +++
> >  netlink/nlsock.c     |  3 ++-
> >  netlink/plca.c       |  4 ++--
> >  netlink/pse-pd.c     |  4 ++--
> >  9 files changed, 96 insertions(+), 20 deletions(-)
> >   
> [...]
> > @@ -6550,6 +6559,16 @@ int main(int argc, char **argp)
> >  			argc -= 1;
> >  			continue;
> >  		}
> > +		if (*argp && !strcmp(*argp, "--phy")) {
> > +			char *eptr;
> > +
> > +			ctx.phy_index = strtoul(argp[1], &eptr, 0);
> > +			if (!argp[1][0] || *eptr)
> > +				exit_bad_args();
> > +			argp += 2;
> > +			argc -= 2;
> > +			continue;
> > +		}
> >  		break;
> >  	}
> >  	if (*argp && !strcmp(*argp, "--monitor")) {  
> 
> Could we have a meaningful error message that would tell user what was
> wrong instead?

Good point, I'll add one.

> 
> > @@ -6585,6 +6604,10 @@ int main(int argc, char **argp)
> >  	}
> >  	if (ctx.json && !args[k].json)
> >  		exit_bad_args_info("JSON output not available for this subcommand");
> > +
> > +	if (!args[k].targets_phy && ctx.phy_index)
> > +		exit_bad_args();
> > +
> >  	ctx.argc = argc;
> >  	ctx.argp = argp;
> >  	netlink_run_handler(&ctx, args[k].nlchk, args[k].nlfunc, !args[k].func);  
> 
> Same here.

Indeed, I'll update accordingly.

[...]

> > @@ -159,7 +151,9 @@ bool ethnla_fill_header(struct nl_msg_buff *msgbuff, uint16_t type,
> >  	if ((devname &&
> >  	     ethnla_put_strz(msgbuff, ETHTOOL_A_HEADER_DEV_NAME, devname)) ||
> >  	    (flags &&
> > -	     ethnla_put_u32(msgbuff, ETHTOOL_A_HEADER_FLAGS, flags)))
> > +	     ethnla_put_u32(msgbuff, ETHTOOL_A_HEADER_FLAGS, flags)) ||
> > +	    (phy_index &&
> > +	     ethnla_put_u32(msgbuff, ETHTOOL_A_HEADER_PHY_INDEX, phy_index)))
> >  		goto err;
> >  
> >  	ethnla_nest_end(msgbuff, nest);  
> 
> Just to be sure: are we sure the PHY index cannot ever be zero (or that
> we won't need to pass 0 index to kernel)?

I should better document that, sorry... The phy index assigned starts
at 1 at wraps-around back to 1 when we exhausted the indices, so it
can't ever be 0.

The netlink code in the kernel side interprets the fact that userspace
passes 0 as "use the default PHY", as if you didn't pass the parameter :

net/ethtool/netlink.c:

	if (!req_info->phy_index)
		return req_info->dev->phydev;

I'll send a patch to document that behaviour, thanks for pointing this
out.

Maxime

Re: [PATCH ethtool-next v2 3/3] ethtool: Introduce a command to list PHYs

[Maxime Chevallier]

Hello Michal,

On Mon, 2 Sep 2024 00:07:56 +0200
Michal Kubecek <mkubecek@suse.cz> wrote:

> On Wed, Aug 28, 2024 at 05:25:10PM +0200, Maxime Chevallier wrote:
> > It is now possible to list all Ethernet PHYs that are present behind a
> > given interface, since the following linux commit :
> > 63d5eaf35ac3 ("net: ethtool: Introduce a command to list PHYs on an interface")
> > 
> > This command relies on the netlink DUMP command to list them, by allowing to
> > pass an interface name/id as a parameter in the DUMP request to only
> > list PHYs on one interface.
> > 
> > Therefore, we introduce a new helper function to prepare a interface-filtered
> > dump request (the filter can be empty, to perform an unfiltered dump),
> > and then uses it to implement PHY enumeration through the --show-phys
> > command.
> > 
> > Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> > ---  
> [...]
> > diff --git a/netlink/extapi.h b/netlink/extapi.h
> > index c882295..fd99610 100644
> > --- a/netlink/extapi.h
> > +++ b/netlink/extapi.h
> > @@ -56,6 +56,7 @@ int nl_set_mm(struct cmd_context *ctx);
> >  int nl_gpse(struct cmd_context *ctx);
> >  int nl_spse(struct cmd_context *ctx);
> >  int nl_flash_module_fw(struct cmd_context *ctx);
> > +int nl_get_phy(struct cmd_context *ctx);
> >  
> >  void nl_monitor_usage(void);
> >    
> 
> Please add also a fallback to !ETHTOOL_ENABLE_NETLINK branch, similar
> to other netlink handlers, so that a build with --disable-netlink does
> not fail.

You're right, I'll add a fallback for that. I actually just faced that
exact issue trying to build this patchset using a fresh buildroot
setup, having forgotten to add netlink libraries.

Thanks for the reviews,

Maxime