[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251006144512.003d4d13@kmaincent-XPS-13-7390>
Date: Mon, 6 Oct 2025 14:45:12 +0200
From: Kory Maincent <kory.maincent@...tlin.com>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: mkubecek@...e.cz, Jakub Kicinski <kuba@...nel.org>,
netdev@...r.kernel.org
Subject: Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping
configuration
On Sat, 4 Oct 2025 20:27:15 +0000
Vadim Fedorenko <vadim.fedorenko@...ux.dev> wrote:
> The kernel supports configuring HW time stamping modes via netlink
> messages, but previous implementation added support for HW time stamping
> source configuration. Add support to configure TX/RX time stamping.
For the information, I didn't add this support because it kind of conflict with
ptp4l which is already configuring this. So if you set it with ethtool, running
ptp4l will change it. I am not really a PTP user so maybe I missed cases where
we need these hwtstamp config change without using ptp4l.
> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
> ---
> ethtool.8.in | 12 ++++++-
> ethtool.c | 1 +
> netlink/tsconfig.c | 78 +++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 89 insertions(+), 2 deletions(-)
>
> diff --git a/ethtool.8.in b/ethtool.8.in
> index 553592b..e9eb2d7 100644
> --- a/ethtool.8.in
> +++ b/ethtool.8.in
> @@ -357,6 +357,10 @@ ethtool \- query or control network driver and hardware
> settings .IR N
> .BI qualifier
> .IR precise|approx ]
> +.RB [ tx
> +.IR TX-TYPE ]
> +.RB [ rx-filter
> +.IR RX-FILTER ]
> .HP
> .B ethtool \-x|\-\-show\-rxfh\-indir|\-\-show\-rxfh
> .I devname
> @@ -1286,7 +1290,7 @@ for IEEE 1588 quality and "approx" is for NICs DMA
> point. Show the selected time stamping PTP hardware clock configuration.
> .TP
> .B \-\-set\-hwtimestamp\-cfg
> -Select the device's time stamping PTP hardware clock.
> +Sets the device's time stamping PTP hardware clock configuration.
> .RS 4
> .TP
> .BI index \ N
> @@ -1295,6 +1299,12 @@ Index of the ptp hardware clock
> .BI qualifier \ precise | approx
> Qualifier of the ptp hardware clock. Mainly "precise" the default one is
> for IEEE 1588 quality and "approx" is for NICs DMA point.
> +.TP
> +.BI tx \ TX-TYPE
> +Type of TX time stamping to configure
> +.TP
> +.BI rx-filter \ RX-FILTER
> +Type of RX time stamping filter to configure
> .RE
> .TP
> .B \-x \-\-show\-rxfh\-indir \-\-show\-rxfh
> diff --git a/ethtool.c b/ethtool.c
> index 948d551..2e03b74 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -6063,6 +6063,7 @@ static const struct option args[] = {
> .nlfunc = nl_stsconfig,
> .help = "Select hardware time stamping",
> .xhelp = " [ index N qualifier
> precise|approx ]\n"
> + " [ tx TX-TYPE ] [ rx-filter
> RX-FILTER ]\n" },
> {
> .opts = "-x|--show-rxfh-indir|--show-rxfh",
> diff --git a/netlink/tsconfig.c b/netlink/tsconfig.c
> index d427c7b..7dee4d1 100644
> --- a/netlink/tsconfig.c
> +++ b/netlink/tsconfig.c
> @@ -17,6 +17,7 @@
> #include "netlink.h"
> #include "bitset.h"
> #include "parser.h"
> +#include "strset.h"
> #include "ts.h"
>
> /* TSCONFIG_GET */
> @@ -94,6 +95,67 @@ int nl_gtsconfig(struct cmd_context *ctx)
>
> /* TSCONFIG_SET */
>
> +int tsconfig_txrx_parser(struct nl_context *nlctx, uint16_t type,
> + const void *data __maybe_unused,
> + struct nl_msg_buff *msgbuff,
> + void *dest __maybe_unused)
> +{
> + const struct stringset *values;
> + const char *arg = *nlctx->argp;
> + unsigned int count, i;
> +
> + nlctx->argp++;
> + nlctx->argc--;
> + if (netlink_init_ethnl2_socket(nlctx) < 0)
> + return -EIO;
> +
> + switch (type) {
> + case ETHTOOL_A_TSCONFIG_TX_TYPES:
> + values = global_stringset(ETH_SS_TS_TX_TYPES,
> nlctx->ethnl2_socket);
> + break;
> + case ETHTOOL_A_TSCONFIG_RX_FILTERS:
> + values = global_stringset(ETH_SS_TS_RX_FILTERS,
> nlctx->ethnl2_socket);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + count = get_count(values);
> + for (i = 0; i < count; i++) {
> + const char *name = get_string(values, i);
> +
> + if (!strcmp(name, arg))
> + break;
> + }
> +
> + if (i != count) {
Maybe you could use the equal condition instead of having all inside the if:
if (i == count)
return -EINVAL;
> + struct nlattr *bits_attr, *bit_attr;
> +
> + if (ethnla_put_flag(msgbuff, ETHTOOL_A_BITSET_NOMASK, true))
> + return -EMSGSIZE;
> +
> + bits_attr = ethnla_nest_start(msgbuff,
> ETHTOOL_A_BITSET_BITS);
> + if (!bits_attr)
> + return -EMSGSIZE;
> +
> + bit_attr = ethnla_nest_start(msgbuff,
> ETHTOOL_A_BITSET_BITS_BIT);
> + if (!bit_attr) {
> + ethnla_nest_cancel(msgbuff, bits_attr);
> + return -EMSGSIZE;
> + }
> + if (ethnla_put_u32(msgbuff, ETHTOOL_A_BITSET_BIT_INDEX, i) ||
> + ethnla_put_flag(msgbuff, ETHTOOL_A_BITSET_BIT_VALUE,
> true)) {
> + ethnla_nest_cancel(msgbuff, bits_attr);
> + ethnla_nest_cancel(msgbuff, bit_attr);
> + return -EMSGSIZE;
> + }
> + mnl_attr_nest_end(msgbuff->nlhdr, bit_attr);
> + mnl_attr_nest_end(msgbuff->nlhdr, bits_attr);
> + return 0;
> + }
> + return -EINVAL;
> +}
> +
> static const struct param_parser stsconfig_params[] = {
> {
> .arg = "index",
> @@ -109,6 +171,20 @@ static const struct param_parser stsconfig_params[] = {
> .handler = tsinfo_qualifier_parser,
> .min_argc = 1,
> },
> + {
> + .arg = "tx",
> + .type = ETHTOOL_A_TSCONFIG_TX_TYPES,
> + .handler = tsconfig_txrx_parser,
> + .group = ETHTOOL_A_TSCONFIG_TX_TYPES,
> + .min_argc = 1,
> + },
> + {
> + .arg = "rx-filter",
> + .type = ETHTOOL_A_TSCONFIG_RX_FILTERS,
> + .handler = tsconfig_txrx_parser,
> + .group = ETHTOOL_A_TSCONFIG_RX_FILTERS,
> + .min_argc = 1,
> + },
> {}
> };
>
> @@ -134,7 +210,7 @@ int nl_stsconfig(struct cmd_context *ctx)
> if (ret < 0)
> return ret;
> if (ethnla_fill_header(msgbuff, ETHTOOL_A_TSCONFIG_HEADER,
> - ctx->devname, 0))
> + ctx->devname, ETHTOOL_FLAG_COMPACT_BITSETS))
> return -EMSGSIZE;
>
> ret = nl_parser(nlctx, stsconfig_params, NULL, PARSER_GROUP_NEST,
> NULL);
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Powered by blists - more mailing lists