netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH ethtool-next v3] netlink: tsconfig: add HW time stamping configuration
@ 2025-11-07 18:20 Vadim Fedorenko
  2025-11-12  8:48 ` Kory Maincent
  0 siblings, 1 reply; 3+ messages in thread
From: Vadim Fedorenko @ 2025-11-07 18:20 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: Jakub Kicinski, netdev, Kory Maincent, 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.
We keep TX type and RX filter configuration as a bit value, but if we
will need multibit value to be set in the future, there is an option to
use "rx-filters" keyword which will be mutually exclusive with current
"rx-filter" keyword. The same applies to "tx-type".

Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>
---
v2 -> v3:
* improve code style
v1 -> v2:
* improve commit message
---
 ethtool.8.in       | 12 +++++++-
 ethtool.c          |  1 +
 netlink/tsconfig.c | 77 +++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/ethtool.8.in b/ethtool.8.in
index 8874ade..1788588 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
@@ -1287,7 +1291,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
@@ -1296,6 +1300,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 bd45b9e..521e6fe 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -6068,6 +6068,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..f4ed10e 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,66 @@ 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)
+{
+	struct nlattr *bits_attr, *bit_attr;
+	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)
+		return -EINVAL;
+
+	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;
+}
+
 static const struct param_parser stsconfig_params[] = {
 	{
 		.arg		= "index",
@@ -109,6 +170,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 +209,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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH ethtool-next v3] netlink: tsconfig: add HW time stamping configuration
  2025-11-07 18:20 [PATCH ethtool-next v3] netlink: tsconfig: add HW time stamping configuration Vadim Fedorenko
@ 2025-11-12  8:48 ` Kory Maincent
  2025-11-12 17:24   ` Vadim Fedorenko
  0 siblings, 1 reply; 3+ messages in thread
From: Kory Maincent @ 2025-11-12  8:48 UTC (permalink / raw)
  To: Vadim Fedorenko; +Cc: Michal Kubecek, Jakub Kicinski, netdev

On Fri,  7 Nov 2025 18:20:44 +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.
> We keep TX type and RX filter configuration as a bit value, but if we
> will need multibit value to be set in the future, there is an option to
> use "rx-filters" keyword which will be mutually exclusive with current
> "rx-filter" keyword. The same applies to "tx-type".
> 
> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
> Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
> v2 -> v3:
> * improve code style
> v1 -> v2:
> * improve commit message
> ---
>  ethtool.8.in       | 12 +++++++-
>  ethtool.c          |  1 +
>  netlink/tsconfig.c | 77 +++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 88 insertions(+), 2 deletions(-)
> 
> diff --git a/ethtool.8.in b/ethtool.8.in
> index 8874ade..1788588 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
> @@ -1287,7 +1291,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
> @@ -1296,6 +1300,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 bd45b9e..521e6fe 100644
> --- a/ethtool.c
> +++ b/ethtool.c
> @@ -6068,6 +6068,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..f4ed10e 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,66 @@ 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)
> +{
> +	struct nlattr *bits_attr, *bit_attr;
> +	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)
> +		return -EINVAL;

It seems you update your patch too quickly and don't test it.
It should be "if (i == count)" here.

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH ethtool-next v3] netlink: tsconfig: add HW time stamping configuration
  2025-11-12  8:48 ` Kory Maincent
@ 2025-11-12 17:24   ` Vadim Fedorenko
  0 siblings, 0 replies; 3+ messages in thread
From: Vadim Fedorenko @ 2025-11-12 17:24 UTC (permalink / raw)
  To: Kory Maincent; +Cc: Michal Kubecek, Jakub Kicinski, netdev

On 12/11/2025 08:48, Kory Maincent wrote:
> On Fri,  7 Nov 2025 18:20:44 +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.
>> We keep TX type and RX filter configuration as a bit value, but if we
>> will need multibit value to be set in the future, there is an option to
>> use "rx-filters" keyword which will be mutually exclusive with current
>> "rx-filter" keyword. The same applies to "tx-type".
>>
>> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
>> Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>
>> ---
>> v2 -> v3:
>> * improve code style
>> v1 -> v2:
>> * improve commit message
>> ---
>>   ethtool.8.in       | 12 +++++++-
>>   ethtool.c          |  1 +
>>   netlink/tsconfig.c | 77 +++++++++++++++++++++++++++++++++++++++++++++-
>>   3 files changed, 88 insertions(+), 2 deletions(-)
>>
>> diff --git a/ethtool.8.in b/ethtool.8.in
>> index 8874ade..1788588 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
>> @@ -1287,7 +1291,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
>> @@ -1296,6 +1300,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 bd45b9e..521e6fe 100644
>> --- a/ethtool.c
>> +++ b/ethtool.c
>> @@ -6068,6 +6068,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..f4ed10e 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,66 @@ 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)
>> +{
>> +	struct nlattr *bits_attr, *bit_attr;
>> +	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)
>> +		return -EINVAL;
> 
> It seems you update your patch too quickly and don't test it.
> It should be "if (i == count)" here.

You are right, thanks for catching it.
v4 is on the way.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-11-12 17:24 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-07 18:20 [PATCH ethtool-next v3] netlink: tsconfig: add HW time stamping configuration Vadim Fedorenko
2025-11-12  8:48 ` Kory Maincent
2025-11-12 17:24   ` Vadim Fedorenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).