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

[Kory Maincent <kory.maincent@bootlin.com>]

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