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.2On 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
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
© 2016 - 2025 Red Hat, Inc.