Re: [PATCH RFC v2 1/6] ethtool: add interface to read Tx hardware timestamping statistics

[Rahul Rameshbabu]

On Wed, 13 Mar, 2024 17:41:07 -0700 Jakub Kicinski <kuba@xxxxxxxxxx> wrote:
> On Wed, 13 Mar 2024 17:26:11 -0700 Rahul Rameshbabu wrote:
>> Makes sense given that these are stale and should have been changed
>> between my v1 and v2. Here is my new attempt at this.
>> 
>>  /**
>>   * struct ethtool_ts_stats - HW timestamping statistics
>>   * @tx_stats: struct group for TX HW timestamping
>>   *	@pkts: Number of packets successfully timestamped by the hardware.
>>   *	@lost: Number of hardware timestamping requests where the timestamping
>>   *            information from the hardware never arrived for submission with
>>   *            the skb.
>
> Should we give some guidance to drivers which "ignore" time stamping
> requests if they used up all the "slots"? Even if just temporary until
> they are fixed? Maybe we can add after all the fields something like:
>
>   For drivers which ignore further timestamping requests when there are
>   too many in flight, the ignored requests are currently not counted by
>   any of the statistics.

I was actually thinking it would be better to merge them into the error
counter temporarily. Reason being is that in the case Intel notices that
their slots are full, they just drop traffic from my understanding
today. If the error counters increment in that situation, it helps with
the debug to a degree. EBUSY is an error in general.

>
> Adjust as needed, I basing this on the vague memory that this was 
> the conclusion in the last discussion :)
>
>>   *	@err: Number of arbitrary timestamp generation error events that the
>>   *           hardware encountered.
>>   */
>
> Just to be crystal clear let's also call out that @lost is not included
> in @err.

Ack.

--
Thanks,

Rahul Rameshbabu