[PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Vadim Fedorenko]

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.

Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.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) {
+		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);
-- 
2.47.3

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Kory Maincent]

On Sat,  4 Oct 2025 20:27:15 +0000
Vadim Fedorenko <vadim.fedorenko@linux.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@linux.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

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Vadim Fedorenko]

On 06/10/2025 13:45, Kory Maincent wrote:
> On Sat,  4 Oct 2025 20:27:15 +0000
> Vadim Fedorenko <vadim.fedorenko@linux.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.

Well, it's more about ability to configure HW time stamping by users.
Running software will potentially change the configuration anyways, but
it maybe helpful to test different HW configurations without changing
the software itself.

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Jakub Kicinski]

On Mon, 6 Oct 2025 14:45:12 +0200 Kory Maincent 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.

FWIW I sometimes enable Rx timestamping (from whatever chrony uses
to ALL) to measure burstiness of incoming traffic when debugging
application packet loss. Would be quite useful to have the ability
to configure this in ethtool.

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Michal Kubecek]

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

On Sat, Oct 04, 2025 at 08:27:15PM GMT, Vadim Fedorenko 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.
> 
> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

As far as I can see, you only allow one bit to be set in each of 
ETHTOOL_A_TSCONFIG_TX_TYPES and ETHTOOL_A_TSCONFIG_RX_FILTERS. If only
one bit is supposed to be set, why are they passed as bitmaps?
(The netlink interface only mirrors what (read-only) ioctl interface
did.)

Michal

> ---
>  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) {
> +		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);
> -- 
> 2.47.3
> 

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

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Kory Maincent]

On Sun, 26 Oct 2025 17:57:12 +0100
Michal Kubecek <mkubecek@suse.cz> wrote:

> On Sat, Oct 04, 2025 at 08:27:15PM GMT, Vadim Fedorenko 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.
> > 
> > Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>  
> 
> As far as I can see, you only allow one bit to be set in each of 
> ETHTOOL_A_TSCONFIG_TX_TYPES and ETHTOOL_A_TSCONFIG_RX_FILTERS. If only
> one bit is supposed to be set, why are they passed as bitmaps?
> (The netlink interface only mirrors what (read-only) ioctl interface
> did.)

Because I used the same design as tsinfo but indeed maybe that is not the best
choice and we shouldn't use bitmap. However, if we change this as it is uAPI we
will need to write a Linux fix to totally remove the bitmap support before we
see any tools using it.

Regards,

> Michal
> 
> > ---
> >  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) {
> > +		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); -- 
> > 2.47.3
> >   



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

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Vadim Fedorenko]

On 26/10/2025 16:57, Michal Kubecek wrote:
> On Sat, Oct 04, 2025 at 08:27:15PM GMT, Vadim Fedorenko 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.
>>
>> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> 
> As far as I can see, you only allow one bit to be set in each of
> ETHTOOL_A_TSCONFIG_TX_TYPES and ETHTOOL_A_TSCONFIG_RX_FILTERS. If only
> one bit is supposed to be set, why are they passed as bitmaps?
> (The netlink interface only mirrors what (read-only) ioctl interface
> did.)

Well, yes, it's only 1 bit is supposed to be set. Unfortunately, netlink
interface was added this way almost a year ago, we cannot change it
anymore without breaking user-space API.

> 
> Michal
> 
>> ---
>>   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) {
>> +		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);
>> -- 
>> 2.47.3
>>

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Michal Kubecek]

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

On Tue, Oct 28, 2025 at 09:48:00PM GMT, Vadim Fedorenko wrote:
> On 26/10/2025 16:57, Michal Kubecek wrote:
> > On Sat, Oct 04, 2025 at 08:27:15PM GMT, Vadim Fedorenko 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.
> > > 
> > > Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> > 
> > As far as I can see, you only allow one bit to be set in each of
> > ETHTOOL_A_TSCONFIG_TX_TYPES and ETHTOOL_A_TSCONFIG_RX_FILTERS. If only
> > one bit is supposed to be set, why are they passed as bitmaps?
> > (The netlink interface only mirrors what (read-only) ioctl interface
> > did.)
> 
> Well, yes, it's only 1 bit is supposed to be set. Unfortunately, netlink
> interface was added this way almost a year ago, we cannot change it
> anymore without breaking user-space API.

The netlink interface only mirrors what we already had in struct
ethtool_ts_info (i.e. the ioctl interface). Therefore my question was
not really about this part of kernel API (which is fixed already) but
rather about the ethtool command line syntax.

In other words, what I really want to ask is: Can we be absolutely sure
that it can never possibly happen in the future that we might need to
set more than one bit in a set message?

If the answer is positive, I'm OK with the patch but perhaps we should
document it explicitly in the TSCONFIG_SET description in kernel file
Documentation/networking/ethtool-netlink.rst

Michal

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

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Vadim Fedorenko]

On 29/10/2025 16:26, Michal Kubecek wrote:
> On Tue, Oct 28, 2025 at 09:48:00PM GMT, Vadim Fedorenko wrote:
>> On 26/10/2025 16:57, Michal Kubecek wrote:
>>> On Sat, Oct 04, 2025 at 08:27:15PM GMT, Vadim Fedorenko 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.
>>>>
>>>> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
>>>
>>> As far as I can see, you only allow one bit to be set in each of
>>> ETHTOOL_A_TSCONFIG_TX_TYPES and ETHTOOL_A_TSCONFIG_RX_FILTERS. If only
>>> one bit is supposed to be set, why are they passed as bitmaps?
>>> (The netlink interface only mirrors what (read-only) ioctl interface
>>> did.)
>>
>> Well, yes, it's only 1 bit is supposed to be set. Unfortunately, netlink
>> interface was added this way almost a year ago, we cannot change it
>> anymore without breaking user-space API.
> 
> The netlink interface only mirrors what we already had in struct
> ethtool_ts_info (i.e. the ioctl interface). Therefore my question was
> not really about this part of kernel API (which is fixed already) but
> rather about the ethtool command line syntax.
> 
> In other words, what I really want to ask is: Can we be absolutely sure
> that it can never possibly happen in the future that we might need to
> set more than one bit in a set message?
> 
> If the answer is positive, I'm OK with the patch but perhaps we should
> document it explicitly in the TSCONFIG_SET description in kernel file
> Documentation/networking/ethtool-netlink.rst

Well, I cannot say about long-long future, but for the last decade we
haven't had a need for multiple bits to be set up. I would assume that
the reality will be around the same.

Jakub/Kory do you have thoughts?

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Jakub Kicinski]

On Wed, 29 Oct 2025 18:53:20 +0000 Vadim Fedorenko wrote:
> >> Well, yes, it's only 1 bit is supposed to be set. Unfortunately, netlink
> >> interface was added this way almost a year ago, we cannot change it
> >> anymore without breaking user-space API.  
> > 
> > The netlink interface only mirrors what we already had in struct
> > ethtool_ts_info (i.e. the ioctl interface). Therefore my question was
> > not really about this part of kernel API (which is fixed already) but
> > rather about the ethtool command line syntax.
> > 
> > In other words, what I really want to ask is: Can we be absolutely sure
> > that it can never possibly happen in the future that we might need to
> > set more than one bit in a set message?
> > 
> > If the answer is positive, I'm OK with the patch but perhaps we should
> > document it explicitly in the TSCONFIG_SET description in kernel file
> > Documentation/networking/ethtool-netlink.rst  
> 
> Well, I cannot say about long-long future, but for the last decade we
> haven't had a need for multiple bits to be set up. I would assume that
> the reality will be around the same.
> 
> Jakub/Kory do you have thoughts?

hard to prove a negative, is the question leading to a different
argument format which will let us set multiple bits? Looks like
we could potentially allow specifying tx / rx-filter multiple
times? Or invent new keywords for the extra bits which presumably 
would be somehow orthogonal to filtering?

tl;dr I'm unclear on the exact concern..

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Kory Maincent]

On Wed, 29 Oct 2025 15:38:12 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Wed, 29 Oct 2025 18:53:20 +0000 Vadim Fedorenko wrote:
> > >> Well, yes, it's only 1 bit is supposed to be set. Unfortunately, netlink
> > >> interface was added this way almost a year ago, we cannot change it
> > >> anymore without breaking user-space API.    
> > > 
> > > The netlink interface only mirrors what we already had in struct
> > > ethtool_ts_info (i.e. the ioctl interface). Therefore my question was
> > > not really about this part of kernel API (which is fixed already) but
> > > rather about the ethtool command line syntax.
> > > 
> > > In other words, what I really want to ask is: Can we be absolutely sure
> > > that it can never possibly happen in the future that we might need to
> > > set more than one bit in a set message?
> > > 
> > > If the answer is positive, I'm OK with the patch but perhaps we should
> > > document it explicitly in the TSCONFIG_SET description in kernel file
> > > Documentation/networking/ethtool-netlink.rst    
> > 
> > Well, I cannot say about long-long future, but for the last decade we
> > haven't had a need for multiple bits to be set up. I would assume that
> > the reality will be around the same.
> > 
> > Jakub/Kory do you have thoughts?  
> 
> hard to prove a negative, is the question leading to a different
> argument format which will let us set multiple bits? Looks like
> we could potentially allow specifying tx / rx-filter multiple
> times? Or invent new keywords for the extra bits which presumably 
> would be somehow orthogonal to filtering?
> 
> tl;dr I'm unclear on the exact concern..

Yes I don't know either. There is already such "orthogonal" flags:
https://elixir.bootlin.com/linux/v6.17.1/source/include/uapi/linux/net_tstamp.h#L180

We could change the bitmap to a value here, even if we don't know what the
future is.
Jakub, as it is already in uAPI but not used at all, would it be possible to
change it or is it already too late?

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

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Jakub Kicinski]

On Thu, 30 Oct 2025 15:37:23 +0100 Kory Maincent wrote:
> Jakub, as it is already in uAPI but not used at all, would it be possible to
> change it or is it already too late?

We'd need a very strong reason to consider changing it now.

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Kory Maincent]

On Thu, 30 Oct 2025 08:14:23 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Thu, 30 Oct 2025 15:37:23 +0100 Kory Maincent wrote:
> > Jakub, as it is already in uAPI but not used at all, would it be possible to
> > change it or is it already too late?  
> 
> We'd need a very strong reason to consider changing it now.

And I don't think we have a strong enough reason here.
So lets keep it as a bitmap then.

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

Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping configuration

[Michal Kubecek]

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

On Wed, Oct 29, 2025 at 03:38:12PM GMT, Jakub Kicinski wrote:
> On Wed, 29 Oct 2025 18:53:20 +0000 Vadim Fedorenko wrote:
> > >> Well, yes, it's only 1 bit is supposed to be set. Unfortunately, netlink
> > >> interface was added this way almost a year ago, we cannot change it
> > >> anymore without breaking user-space API.  
> > > 
> > > The netlink interface only mirrors what we already had in struct
> > > ethtool_ts_info (i.e. the ioctl interface). Therefore my question was
> > > not really about this part of kernel API (which is fixed already) but
> > > rather about the ethtool command line syntax.
> > > 
> > > In other words, what I really want to ask is: Can we be absolutely sure
> > > that it can never possibly happen in the future that we might need to
> > > set more than one bit in a set message?
> > > 
> > > If the answer is positive, I'm OK with the patch but perhaps we should
> > > document it explicitly in the TSCONFIG_SET description in kernel file
> > > Documentation/networking/ethtool-netlink.rst  
> > 
> > Well, I cannot say about long-long future, but for the last decade we
> > haven't had a need for multiple bits to be set up. I would assume that
> > the reality will be around the same.
> > 
> > Jakub/Kory do you have thoughts?
> 
> hard to prove a negative, is the question leading to a different
> argument format which will let us set multiple bits? Looks like
> we could potentially allow specifying tx / rx-filter multiple
> times? Or invent new keywords for the extra bits which presumably 
> would be somehow orthogonal to filtering?
> 
> tl;dr I'm unclear on the exact concern..

My only concern was to make (reasonably) sure we won't have to make an
incompatible change of the command line syntax. But you are right that
there is alwasy an option to have e.g. "rx-filter" and "rx-filters"
(mutually exclusive).

Michal

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