Re: igc: missing HW timestamps at TX

[Vinicius Costa Gomes <vinicius.gomes@intel.com>]

Kurt Kanzenbach <kurt@linutronix.de> writes:

> On Tue Aug 16 2022, Vinicius Costa Gomes wrote:
>> Kurt Kanzenbach <kurt@linutronix.de> writes:
>>
>>> Hi Vinicius,
>>>
>>> On Mon Aug 15 2022, Vinicius Costa Gomes wrote:
>>>> I think your question is more "why there's that workqueue on igc?"/"why
>>>> don't you retrieve the TX timestamp 'inline' with the interrupt?", if I
>>>> got that right, then, I don't have a good reason, apart from the feeling
>>>> that reading all those (5-6?) registers may take too long for a
>>>> interrupt handler. And it's something that's being done the same for
>>>> most (all?) Intel drivers.
>>>
>>> We do have one optimization for igb which attempts to read the Tx
>>> timestamp directly from the ISR. If that's not ready *only* then we
>>> schedule the worker. I do assume igb and igc have the same logic for
>>> retrieving the timestamps here.
>>>
>>
>> That seems a sensible approach. And yes, the timestamping logic is the
>> same.
>>
>>> The problem with workqueues is that under heavy system load, it might be
>>> deferred and timestamps will be lost. I guess that workqueue was added
>>> because of something like this: 1f6e8178d685 ("igb: Prevent dropped Tx
>>> timestamps via work items and interrupts.").
>>>
>>>>
>>>> I have a TODO to experiment with removing the workqueue, and retrieving
>>>> the TX timestamp in the same context as the interrupt handler, but other
>>>> things always come up.
>>>
>>> Let me know if you have interest in that igb patch.
>>
>> That would be great! Thanks.
>
> Sure. See igb patch below.

Thanks!

>
> I'm also wondering whether that delayed work should be replaced
> completely by the PTP AUX worker, because that one can be prioritized in
> accordance to the use case. And I see Vladimir already suggested this.

That was the idea.


Cheers,
-- 
Vinicius