netdev - Re: [PATCH net-next v2 6/7] tsnep: convert to ndo

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <aO9xf0gW9F0qsaCz@horms.kernel.org>
Date: Wed, 15 Oct 2025 11:03:43 +0100
From: Simon Horman <horms@...nel.org>
To: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
Cc: Shyam Sundar S K <Shyam-sundar.S-k@....com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Egor Pomozov <epomozov@...vell.com>,
	Potnuri Bharat Teja <bharat@...lsio.com>,
	Dimitris Michailidis <dmichail@...gible.com>,
	MD Danish Anwar <danishanwar@...com>,
	Roger Quadros <rogerq@...nel.org>,
	Richard Cochran <richardcochran@...il.com>,
	Russell King <linux@...linux.org.uk>,
	Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 6/7] tsnep: convert to ndo_hwtstatmp API

On Tue, Oct 14, 2025 at 10:42:15PM +0000, Vadim Fedorenko wrote:
> Convert to .ndo_hwtstamp_get()/.ndo_hwtstamp_set() callbacks.
> After conversions the rest of tsnep_netdev_ioctl() becomes pure
> phy_do_ioctl_running(), so remove tsnep_netdev_ioctl() and replace
> it with phy_do_ioctl_running() in .ndo_eth_ioctl.
> 
> Signed-off-by: Vadim Fedorenko <vadim.fedorenko@...ux.dev>

...

> diff --git a/drivers/net/ethernet/engleder/tsnep_ptp.c b/drivers/net/ethernet/engleder/tsnep_ptp.c
> index 54fbf0126815..ae1308eb813d 100644
> --- a/drivers/net/ethernet/engleder/tsnep_ptp.c
> +++ b/drivers/net/ethernet/engleder/tsnep_ptp.c
> @@ -19,57 +19,53 @@ void tsnep_get_system_time(struct tsnep_adapter *adapter, u64 *time)
>  	*time = (((u64)high) << 32) | ((u64)low);
>  }
>  
> -int tsnep_ptp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
> +int tsnep_ptp_hwtstamp_get(struct net_device *netdev,
> +			   struct kernel_hwtstamp_config *config)
>  {
>  	struct tsnep_adapter *adapter = netdev_priv(netdev);
> -	struct hwtstamp_config config;
> -
> -	if (!ifr)
> -		return -EINVAL;
> -
> -	if (cmd == SIOCSHWTSTAMP) {
> -		if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
> -			return -EFAULT;
> -
> -		switch (config.tx_type) {
> -		case HWTSTAMP_TX_OFF:
> -		case HWTSTAMP_TX_ON:
> -			break;
> -		default:
> -			return -ERANGE;
> -		}
> -
> -		switch (config.rx_filter) {
> -		case HWTSTAMP_FILTER_NONE:
> -			break;
> -		case HWTSTAMP_FILTER_ALL:
> -		case HWTSTAMP_FILTER_PTP_V1_L4_EVENT:
> -		case HWTSTAMP_FILTER_PTP_V1_L4_SYNC:
> -		case HWTSTAMP_FILTER_PTP_V1_L4_DELAY_REQ:
> -		case HWTSTAMP_FILTER_PTP_V2_L4_EVENT:
> -		case HWTSTAMP_FILTER_PTP_V2_L4_SYNC:
> -		case HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ:
> -		case HWTSTAMP_FILTER_PTP_V2_L2_EVENT:
> -		case HWTSTAMP_FILTER_PTP_V2_L2_SYNC:
> -		case HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ:
> -		case HWTSTAMP_FILTER_PTP_V2_EVENT:
> -		case HWTSTAMP_FILTER_PTP_V2_SYNC:
> -		case HWTSTAMP_FILTER_PTP_V2_DELAY_REQ:
> -		case HWTSTAMP_FILTER_NTP_ALL:
> -			config.rx_filter = HWTSTAMP_FILTER_ALL;
> -			break;
> -		default:
> -			return -ERANGE;
> -		}

Hi Vadim,

I'm probably missing something obvious, but it's not clear to me why
removing the inner switch statements above is ok. Or, perhaps more to the
point, it seems inconsistent with other patches in this series.

OTOH, I do see why dropping the outer if conditions makes sense.

> -
> -		memcpy(&adapter->hwtstamp_config, &config,
> -		       sizeof(adapter->hwtstamp_config));
> +
> +	*config = adapter->hwtstamp_config;
> +	return 0;
> +}

...