netdev - Re: [PATCH net-next 1/3] net: cpsw: return proper RX timestamping filter in cpsw_hwtstamp

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: <de814321-7ede-4325-be9e-3dd40be68391@linux.dev>
Date: Thu, 8 May 2025 23:33:06 +0100
From: Vadim Fedorenko <vadim.fedorenko@...ux.dev>
To: Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org
Cc: Köry Maincent <kory.maincent@...tlin.com>,
 Andrew Lunn <andrew@...n.ch>, Siddharth Vadapalli <s-vadapalli@...com>,
 Roger Quadros <rogerq@...nel.org>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
 Richard Cochran <richardcochran@...il.com>,
 Russell King <linux@...linux.org.uk>, linux-kernel@...r.kernel.org,
 linux-omap@...r.kernel.org
Subject: Re: [PATCH net-next 1/3] net: cpsw: return proper RX timestamping
 filter in cpsw_hwtstamp_get()

On 08/05/2025 20:48, Vladimir Oltean wrote:
> priv->rx_ts_enabled is a boolean variable (0 or 1). Overlapped over enum
> hwtstamp_rx_filters, it makes cfg.rx_filter take the value of either
> HWTSTAMP_FILTER_NONE (when 0) or HWTSTAMP_FILTER_ALL (when 1).

Hmm.. I have to disagree here. rx_ts_enabled is int, not bool:

struct cpsw_priv {
         struct net_device               *ndev;
         struct device                   *dev;
         u32                             msg_enable;
         u8                              mac_addr[ETH_ALEN];
         bool                            rx_pause;
         bool                            tx_pause;
         bool                            mqprio_hw;
         int                             fifo_bw[CPSW_TC_NUM];
         int                             shp_cfg_speed;
         int                             tx_ts_enabled;
         int                             rx_ts_enabled;
         struct bpf_prog                 *xdp_prog;
	....

And it's assigned a value of HWTSTAMP_FILTER_PTP_V2_EVENT in
cpsw_hwtstamp_set(). Not sure this change is actually needed.

> 
> But this is inconsistent with what is returned in cpsw_hwtstamp_set().
> There, HWTSTAMP_FILTER_ALL is refused (-ERANGE), and a subset of the RX
> filters requestable by user space are all replaced with
> HWTSTAMP_FILTER_PTP_V2_EVENT. So the driver should be reporting this
> value during SIOCGHWTSTAMP as well.
> 
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>   drivers/net/ethernet/ti/cpsw_priv.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/ti/cpsw_priv.c b/drivers/net/ethernet/ti/cpsw_priv.c
> index 6fe4edabba44..68d8f7ea0e44 100644
> --- a/drivers/net/ethernet/ti/cpsw_priv.c
> +++ b/drivers/net/ethernet/ti/cpsw_priv.c
> @@ -687,7 +687,8 @@ static int cpsw_hwtstamp_get(struct net_device *dev, struct ifreq *ifr)
>   
>   	cfg.flags = 0;
>   	cfg.tx_type = priv->tx_ts_enabled ? HWTSTAMP_TX_ON : HWTSTAMP_TX_OFF;
> -	cfg.rx_filter = priv->rx_ts_enabled;
> +	cfg.rx_filter = priv->rx_ts_enabled ? HWTSTAMP_FILTER_PTP_V2_EVENT :
> +			HWTSTAMP_FILTER_NONE;
>   
>   	return copy_to_user(ifr->ifr_data, &cfg, sizeof(cfg)) ? -EFAULT : 0;
>   }