On 8/26/2025 5:59 AM, Sebastian Andrzej Siewior wrote: > On 2025-08-25 16:28:38 [-0700], Jacob Keller wrote: >> Ya, I don't think we fully understand either. Miroslav said he tested on >> I350 which is a different MAC from the I210, so it could be something >> there. Theoretically we could handle just I210 directly in the interrupt >> and leave the other variants to the kworker.. but I don't know how much >> benefit we get from that. The data sheet for the I350 appears to have >> more or less the same logic for Tx timestamps. It is significantly >> different for Rx timestamps though. > > From logical point of view it makes sense to retrieve the HW timestamp > immediately when it becomes available and feed it to the stack. I can't > imagine how delaying it to yet another thread improves the situation. > The benchmark is about > 1k packets/ second while in reality you have > less than 20 packets a second. With multiple applications you usually > need a "second timestamp register" or you may lose packets. > > Delaying it to the AUX worker makes sense for hardware which can't fire > an interrupt and polling is the only option left. This is sane in this > case but I don't like this solution as some kind compromise for > everyone. Simply because it adds overhead and requires additional > configuration. >
I agree. Its just frustrating that doing so appears to cause a
regression in at least one test setup on hardware which uses this method.
>>> Also I couldn't really see a performance degradation with ntpperf. In my
>>> tests the IRQ variant reached an equal or higher rate. But sometimes I
>>> get 'Could not send requests at rate X'. No idea what that means.
>>>
>>> Anyway, this patch is basically a compromise. It works for Miroslav and
>>> my use case.
>>>
>>>> This is also what the igc does and the performance improved
>>>> afa141583d827 ("igc: Retrieve TX timestamp during interrupt handling")
>>>>
>>
>> igc supports several hardware variations which are all a lot similar to
>> i210 than i350 is to i210 in igb. I could see this working fine for i210
>> if it works fine in igb.. I honestly am at a loss currently why i350 is
>> much worse.
>>
>>>> and here it causes the opposite?
>>>
>>> As said above, I'm out of ideas here.
>>>
>>
>> Same. It may be one of those things where the effort to dig up precisely
>> what has gone wrong is so large that it becomes not feasible relative to
>> the gain :(
>
> Could we please use the direct retrieval/ submission for HW which
> supports it and fallback to the AUX worker (instead of the kworker) for
> HW which does not have an interrupt for it?
>
I have no objection. Perhaps we could assume the high end of the ntpperf
benchmark is not reflective of normal use case? We *are* limited to only
one timestamp register, which the igb driver does protect by bitlock.
>>> Thanks,
>>> Kurt
>
> Sebastian
OpenPGP_signature.asc
Description: OpenPGP digital signature
