netdev - Re: igc: missing HW timestamps at TX

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 17 Aug 2022 12:24:05 -0700
From:   Vinicius Costa Gomes <vinicius.gomes@...el.com>
To:     Kurt Kanzenbach <kurt@...utronix.de>,
        Vladimir Oltean <vladimir.oltean@....com>
Cc:     Ferenc Fejes <ferenc.fejes@...csson.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "marton12050@...il.com" <marton12050@...il.com>,
        "peti.antal99@...il.com" <peti.antal99@...il.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>
Subject: Re: igc: missing HW timestamps at TX

Kurt Kanzenbach <kurt@...utronix.de> writes:

> On Tue Aug 16 2022, Vinicius Costa Gomes wrote:
>> Kurt Kanzenbach <kurt@...utronix.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