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

From: Vadim Fedorenko <[email-masked]>
Date: Tue, 28 Oct 2025 21:48:00 +0000

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 <[email-masked]>
> 
> 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
>>