Re: [PATCH net v3 5/5] ptp: vclock: preserve missing hardware timestamps

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] The patch adds a new early return to ptp_convert_timestamp() in
  drivers/ptp/ptp_vclock.c: when the input hardware timestamp is zero…

--

ptp: vclock: preserve missing hardware timestamps

Socket timestamp delivery converts hardware timestamps to a vclock time
before it checks whether a hardware timestamp is present. With a zero
input, timecounter_cyc2time() can return a nonzero value. This patch
makes ptp_convert_timestamp() return 0 without converting when the input
timestamp is zero.

> diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
> index 6675466bc3074..4a62047f0ce0f 100644
> --- a/drivers/ptp/ptp_vclock.c
> +++ b/drivers/ptp/ptp_vclock.c
> @@ -307,6 +307,8 @@ ktime_t ptp_convert_timestamp(const ktime_t *hwtstamp, int vclock_index)
>  	u64 ns;
>  
>  	ns = ktime_to_ns(*hwtstamp);
> +	if (!ns)
> +		return 0;

[Severity: Low]

This isn't a bug, but should the kernel-doc for ptp_convert_timestamp()
in include/linux/ptp_clock_kernel.h be updated as well? It still says:

 * Returns: converted timestamp, or 0 on error.

The new early return gives 0 on a path that is not an error. It passes
through the "no hardware timestamp" case. After this patch, a 0 return
means either a failed lookup or conversion, or a zero @hwtstamp.

Both callers in net/socket.c already treat 0 as "no timestamp", so
nothing changes at runtime. A wording such as "converted timestamp, or 0
if @hwtstamp is zero or on error" would match the new behaviour.

>  
>  	srcu_idx = srcu_read_lock(&vclock_srcu);
>

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261013-ptp-vclock-sampling-v3-0-d3ee4ea62fc6%40gmail.com