linux-kernel - Re: [PATCH net-next v4 0/8] gve: Add Rx HW timestamping support

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: <CAG-FcCNg5BWcqTBcMA+WYHqFdG-htnYXDuXZ=S+QuwhugZ6fyA@mail.gmail.com>
Date: Wed, 11 Jun 2025 15:05:50 -0700
From: Ziwei Xiao <ziweixiao@...gle.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: Harshitha Ramamurthy <hramamurthy@...gle.com>, netdev@...r.kernel.org, davem@...emloft.net, 
	edumazet@...gle.com, pabeni@...hat.com, jeroendb@...gle.com, 
	andrew+netdev@...n.ch, willemb@...gle.com, pkaligineedi@...gle.com, 
	yyd@...gle.com, joshwash@...gle.com, shailend@...gle.com, linux@...blig.org, 
	thostet@...gle.com, jfraker@...gle.com, richardcochran@...il.com, 
	jdamato@...tly.com, vadim.fedorenko@...ux.dev, horms@...nel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v4 0/8] gve: Add Rx HW timestamping support

On Tue, Jun 10, 2025 at 6:23 PM Jakub Kicinski <kuba@...nel.org> wrote:
>
> On Mon,  9 Jun 2025 18:40:21 +0000 Harshitha Ramamurthy wrote:
> > This patch series add the support of Rx HW timestamping, which sends
> > adminq commands periodically to the device for clock synchronization with
> > the nic.
>
> IIUC:
>  - the driver will only register the PHC if timestamping is enabled
>    (and unregister it when disabled),
I checked around the other drivers and it looked like they would
register the PHC when initializing the driver and keep it alive until
destroying the driver. I can change it to be that way on V5.
>  - there is no way to read the PHC from user space other than via
>    packet timestamps,
The ability to read the PHC from user space will be added in the
future patch series when adding the actual PTP support. For this
patch, it's adding the initial ptp to utilize the ptp_schedule_worker
to read the NIC clock as suggested in the previous review comments.
>  - the ethtool API does not report which PHC is associated with the
>    NIC, presumably because the PHC is useless to the user space.
>
Thanks for pointing it out. I can add the phc_index info into the
gve_get_ts_info on V5.
> Do I understand that correctly? It's pretty unusual. Why not let user
> read the clock?
>
> Why unregister the PHC? I understand that you may want to cancel
> the quite aggressive timestamp refresh work, but why kill the whole
> clock... Perhaps ptp_cancel_worker_sync() didn't exist when you wrote
> this code?
Will utilize ptp_cancel_worker_sync instead of unregistering the PHC
every time on V5.