lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5w25bm7gnbrq4cwtefmunmcylqav524roamuvoz2zv5piadpek@4vpzw533uuyd>
Date: Sun, 26 Oct 2025 17:57:12 +0100
From: Michal Kubecek <mkubecek@...e.cz>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org, 
	Kory Maincent <kory.maincent@...tlin.com>
Subject: Re: [PATCH ethtool-next] netlink: tsconfig: add HW time stamping
 configuration

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@...ux.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
> 

Download attachment "signature.asc" of type "application/pgp-signature" (485 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ