netdev - Re: [PATCH net-next 2/2] net: Remove setting of RX software timestamp from drivers

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0ce52835-e60a-4ca5-9001-ac48e5a6abf5@nvidia.com>
Date: Fri, 30 Aug 2024 20:07:06 +0300
From: Gal Pressman <gal@...dia.com>
To: Niklas Söderlund <niklas.soderlund@...natech.se>
Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski
 <kuba@...nel.org>, netdev@...r.kernel.org, Jay Vosburgh <jv@...sburgh.net>,
 Andy Gospodarek <andy@...yhouse.net>, Marc Kleine-Budde
 <mkl@...gutronix.de>, Vincent Mailhol <mailhol.vincent@...adoo.fr>,
 Shyam Sundar S K <Shyam-sundar.S-k@....com>,
 Sudarsana Kalluru <skalluru@...vell.com>, Manish Chopra
 <manishc@...vell.com>, Michael Chan <michael.chan@...adcom.com>,
 Pavan Chebbi <pavan.chebbi@...adcom.com>,
 Nicolas Ferre <nicolas.ferre@...rochip.com>,
 Claudiu Beznea <claudiu.beznea@...on.dev>,
 Sunil Goutham <sgoutham@...vell.com>,
 Potnuri Bharat Teja <bharat@...lsio.com>,
 Christian Benvenuti <benve@...co.com>, Satish Kharat <satishkh@...co.com>,
 Claudiu Manoil <claudiu.manoil@....com>,
 Vladimir Oltean <vladimir.oltean@....com>, Wei Fang <wei.fang@....com>,
 Shenwei Wang <shenwei.wang@....com>, Clark Wang <xiaoning.wang@....com>,
 Dimitris Michailidis <dmichail@...gible.com>,
 Yisen Zhuang <yisen.zhuang@...wei.com>, Salil Mehta
 <salil.mehta@...wei.com>, Jijie Shao <shaojijie@...wei.com>,
 Tony Nguyen <anthony.l.nguyen@...el.com>,
 Przemek Kitszel <przemyslaw.kitszel@...el.com>,
 Marcin Wojtas <marcin.s.wojtas@...il.com>,
 Russell King <linux@...linux.org.uk>, Geetha sowjanya <gakula@...vell.com>,
 Subbaraya Sundeep <sbhatta@...vell.com>, hariprasad <hkelam@...vell.com>,
 Ido Schimmel <idosch@...dia.com>, Petr Machata <petrm@...dia.com>,
 Bryan Whitehead <bryan.whitehead@...rochip.com>,
 UNGLinuxDriver@...rochip.com, Horatiu Vultur <horatiu.vultur@...rochip.com>,
 Lars Povlsen <lars.povlsen@...rochip.com>,
 Steen Hegelund <Steen.Hegelund@...rochip.com>,
 Daniel Machon <daniel.machon@...rochip.com>,
 Alexandre Belloni <alexandre.belloni@...tlin.com>,
 Shannon Nelson <shannon.nelson@....com>,
 Brett Creeley <brett.creeley@....com>, Sergey Shtylyov <s.shtylyov@....ru>,
 Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
 Edward Cree <ecree.xilinx@...il.com>,
 Martin Habets <habetsm.xilinx@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Jose Abreu <joabreu@...opsys.com>,
 Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Siddharth Vadapalli <s-vadapalli@...com>, Roger Quadros <rogerq@...nel.org>,
 MD Danish Anwar <danishanwar@...com>, Linus Walleij <linusw@...nel.org>,
 Imre Kaloz <kaloz@...nwrt.org>, Richard Cochran <richardcochran@...il.com>,
 Willem de Bruijn <willemdebruijn.kernel@...il.com>,
 Carolina Jubran <cjubran@...dia.com>,
 Rahul Rameshbabu <rrameshbabu@...dia.com>
Subject: Re: [PATCH net-next 2/2] net: Remove setting of RX software timestamp
 from drivers

On 29/08/2024 23:44, Niklas Söderlund wrote:
> Hi Gal,
> 
> Thanks for your work.
> 
> On 2024-08-29 17:42:53 +0300, Gal Pressman wrote:
> 
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c 
>> b/drivers/net/ethernet/renesas/ravb_main.c
>> index c02fb296bf7d..c7ec23688d56 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -1744,8 +1744,6 @@ static int ravb_get_ts_info(struct net_device *ndev,
>>  
>>  	info->so_timestamping =
>>  		SOF_TIMESTAMPING_TX_SOFTWARE |
>> -		SOF_TIMESTAMPING_RX_SOFTWARE |
>> -		SOF_TIMESTAMPING_SOFTWARE |
>>  		SOF_TIMESTAMPING_TX_HARDWARE |
>>  		SOF_TIMESTAMPING_RX_HARDWARE |
>>  		SOF_TIMESTAMPING_RAW_HARDWARE;
>> @@ -1756,6 +1754,8 @@ static int ravb_get_ts_info(struct net_device *ndev,
>>  		(1 << HWTSTAMP_FILTER_ALL);
>>  	if (hw_info->gptp || hw_info->ccc_gac)
>>  		info->phc_index = ptp_clock_index(priv->ptp.clock);
>> +	else
>> +		info->phc_index = 0;
> 
> I understand this work keeps things the same as they where before and 
> that this change do not alter the existing behavior. But should not 
> phc_index be left untouched here (kept at -1) as there are no ptp clock?

Yes, setting the phc_index to zero felt bad and is likely a bug, but
this patch is probably not the place to fix it.

> 
> I think this might have been introduced in commit 7e09a052dc4e ("ravb: 
> Exclude gPTP feature support for RZ/G2L") when the driver excluded ptp 
> support for some devices. I suspect the so_timestamping mask should also 
> depend on this check and only advertise hardware clocks if it indeed 
> exists?
> 
> If my assumption is correct I can fix this on top. For this change the 
> existing behavior is kept, so for drivers/net/ethernet/renesas,

Sounds good to me.

> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@...natech.se>

Thank you Niklas!