Re: [PATCH iwl-net 08/10] idpf: add Tx timestamp flows

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 18/11/2024 07:07, Olech, Milena wrote:
> On 11/14/2024 1:52 PM, Vadim Fedorenko wrote:
> 
>> On 13/11/2024 15:46, Milena Olech wrote:
>>> Add functions to request Tx timestamp for the PTP packets, read the Tx
>>> timestamp when the completion tag for that packet is being received,
>>> extend the Tx timestamp value and set the supported timestamping modes.
>>>
>>> Tx timestamp is requested for the PTP packets by setting a TSYN bit and
>>> index value in the Tx context descriptor. The driver assumption is that
>>> the Tx timestamp value is ready to be read when the completion tag is
>>> received. Then the driver schedules delayed work and the Tx timestamp
>>> value read is requested through virtchnl message. At the end, the Tx
>>> timestamp value is extended to 64-bit and provided back to the skb.
>>>
>>> Co-developed-by: Josh Hay <joshua.a.hay@intel.com>
>>> Signed-off-by: Josh Hay <joshua.a.hay@intel.com>
>>> Reviewed-by: Alexander Lobakin <aleksander.lobakin@intel.com>
>>> Signed-off-by: Milena Olech <milena.olech@intel.com>

[skipped]

>>> +/**
>>> + * idpf_ptp_extend_ts - Convert a 40b timestamp to 64b nanoseconds
>>> + * @adapter: Driver specific private structure
>>> + * @in_tstamp: Ingress/egress timestamp value
>>> + *
>>> + * It is assumed that the caller verifies the timestamp is valid prior to
>>> + * calling this function.
>>> + *
>>> + * Extract the 32bit nominal nanoseconds and extend them. Use the cached PHC
>>> + * time stored in the device private PTP structure as the basis for timestamp
>>> + * extension.
>>> + *
>>> + * Return: Tx timestamp value extended to 64 bits.
>>> + */
>>> +u64 idpf_ptp_extend_ts(const struct idpf_adapter *adapter, u64 in_tstamp)
>>> +{
>>> +	unsigned long discard_time;
>>> +
>>> +	discard_time = adapter->ptp->cached_phc_jiffies + 2 * HZ;
>>> +
>>> +	if (time_is_before_jiffies(discard_time))
>>> +		return 0;
>>
>> It might be a good idea to count such events, just to provide at least
>> some information to the client regarding zero timestamp?
> 
> You mean to calculate skipped Tx timestamps?

Yes, we have special metric in ethtool to provide info about issues with
TX timestamp, it would be great to have idpf implemented this interface.

> 
>>
>>> +
>>> +	return idpf_ptp_tstamp_extend_32b_to_64b(adapter->ptp->cached_phc_time,
>>> +						 lower_32_bits(in_tstamp));
>>> +}
>>
>>
>> [... skip ...]
> 
> Thanks,
> Milena