Re: [Intel-wired-lan] [PATCH next-queue v2 3/3] igc: Add support for PTP getcrosststamp()

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

Miroslav Lichvar <mlichvar@redhat.com> writes:

> On Tue, Nov 10, 2020 at 11:06:07AM -0800, Vinicius Costa Gomes wrote:
>> Miroslav Lichvar <mlichvar@redhat.com> writes:
>> > I suspect the estimate would be valid only when the NIC is connected
>> > directly to the PTM root (PCI root complex). Is it possible to get the
>> > timestamps or delay from PTM-capable switches on the path between CPU
>> > and NIC? Also, how frequent can be the PTM dialogs? Could they be
>> > performed synchronously in the ioctl?
>> 
>> Reading the PTM specs, it could work over PCIe switches (if they also
>> support PTM).
>
> I saw some "implementation specific means" mentioned in the spec, so
> I'm not sure what and how exactly it works with the existing CPUs,
> NICs and PCIe switches. But even if the reported delay was valid only
> for directly connected NICs, I think that could still be useful as
> long as the user/application can figure out whether that is the case.

Right now, the logic I am using is that the PTM cycles (and measurement
reporting) are started only if PTM could be enabled in the endpoint PCI
device. If I understand the code right, PTM will only be enabled if it
could be enabled all along the path to the root port.

In other words, what this series does is: if the CONFIG_PCIE_PTM option
is enabled, and if PTM is supported along the path to the NIC, the PTM
cycles will be started during the device initialization, and the
measurements will be reported when the ioctl() is called.

>
>> The NIC I have supports PTM cycles from every ~1ms to ~512ms, and from
>> my tests it wants to be kept running "in background" always, i.e. set
>> the cycles to run, and only report the data when necessary. Trying to
>> only enable the cycles "on demand" was unreliable.
>
> I see. It does makes sense if the clocks need to be are synchronized.
> For the case of this ioctl, I think it would be better if it we could
> just collect the measurements and leave the clocks free running.

Not sure if I understand. This is what this series does, it only adds
support for starting the PTM cycles and reporting the measurements.

>
>> (so for the _EXTENDED case, I would need to accumulate multiple values
>> in the driver, and report them later, a bit annoying, but not
>> impossible)
>
> I think you could simply repeat the sample in the output up to the
> requested number.
>
> I suspect a bigger issue, for both the PRECISE and EXTENDED variants,
> is that it would return old data. I'm not sure if the existing
> applications are ready to deal with that. With high clock update
> rates, a delay of 50 milliseconds could cause an instability. You can
> try phc2sys -R 50 and see if it stays stable.

After a couple of hours of testing, with the current 50ms delay,
'phc2sys -R 50' is stable, but I got the impression that it takes longer
(~10s) to get to ~10ns offset.

Just for fun, I tried 'phc2sys -R 100' and it doesn't stabilize.

So it seems that applications (phc2sys) are able to tolerate some old
data.


Cheers,
-- 
Vinicius