netdev - Re: [PATCH net-next] gve: Correctly report software timestamping capabilities

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: <87jzl5akh5.fsf@nvidia.com>
Date: Tue, 09 Apr 2024 21:40:46 -0700
From: Rahul Rameshbabu <rrameshbabu@...dia.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>, John Fraker
 <jfraker@...gle.com>, netdev@...r.kernel.org, Praveen  Kaligineedi
 <pkaligineedi@...gle.com>, Harshitha Ramamurthy <hramamurthy@...gle.com>,
 Shailend Chand <shailend@...gle.com>, Willem de  Bruijn
 <willemb@...gle.com>, "David S. Miller" <davem@...emloft.net>, Junfeng Guo
 <junfeng.guo@...el.com>, Ziwei Xiao <ziweixiao@...gle.com>, Jeroen de
 Borst <jeroendb@...gle.com>, linux-kernel@...r.kernel.org,
 kory.maincent@...tlin.com, andrew@...n.ch, richardcochran@...il.com
Subject: Re: [PATCH net-next] gve: Correctly report software timestamping
 capabilities

On Tue, 09 Apr, 2024 17:28:38 -0700 Jakub Kicinski <kuba@...nel.org> wrote:
> On Tue, 09 Apr 2024 10:29:55 -0400 Willem de Bruijn wrote:
>> This device calls skb_tx_timestamp in its ndo_start_xmit: the
>> prerequisite for SOF_TIMESTAMPING_TX_SOFTWARE.
>> 
>> All devices support SOF_TIMESTAMPING_RX_SOFTWARE by virtue of
>> net_timestamp_check being called in the device independent code.
>> 
>> To ethtool timestamping maintainers: It's quite unnecessary to have
>> each device advertise SOF_TIMESTAMPING_RX_SOFTWARE |
>> SOF_TIMESTAMPING_SOFTWARE. In __ethtool_get_ts_info we could just
>> always add those flags to the result from the callees.
>> 
>>         if (phy_has_tsinfo(phydev))
>>                 return phy_ts_info(phydev, info);
>>         if (ops->get_ts_info)
>>                 return ops->get_ts_info(dev, info);
>> 
>>         info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
>>                                 SOF_TIMESTAMPING_SOFTWARE;
>
> My gut tells me we force drivers to set the ethtool op because
> while at it they will probably also implement tx stamping.

I think the logic should be the other way (in terms of the
relationship). A call to skb_tx_timestamp should throw a warning if the
driver does not advertise its timestamping capabilities. This way, a
naive netdev driver for some lightweight device does not need to worry
about this. I agree that anyone implementing tx timestamping should have
this operation defined. An skb does not contain any mechanism to
reference the driver's ethtool callback. Maybe the right choice is to
have a ts capability function registered for each netdev that can be
used by the core stack and that powers the ethtool operation as well
instead of the existing callback for ethtool?

>
> Even more unhelpful point I'll risk making is that we could 
> add a test and make people who submit new drivers run it :)

--
Thanks,

Rahul Rameshbabu