Hi Richard,

-----Original Message-----
From: Richard Cochran <richardcochran@gmail.com>
Sent: 2021年5月9日 3:17
To: Y.b. Lu <yangbo.lu@nxp.com>
Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>; Claudiu
Manoil [off-list ref]; Jakub Kicinski [off-list ref]
Subject: Re: [net-next 0/6] ptp: support virtual clocks for multiple domains

On Fri, May 07, 2021 at 04:57:50PM +0800, Yangbo Lu wrote:

quoted

  ptp4l -i eno0 -p/dev/ptp1 -m --domainNumber=1 --priority1=128 >

domain1-slave.log 2>&1 &

quoted

  ptp4l -i eno0 -p/dev/ptp2 -m --domainNumber=2 --priority1=128 >

domain2-slave.log 2>&1 &

quoted

  ptp4l -i eno0 -p/dev/ptp3 -m --domainNumber=3 --priority1=127 >
domain3-master.log 2>&1 &

quoted

- Make changing on physical clock transparent to virtual clocks.
  The virtual clock is based on free running physical clock. If physical
  clock has to be changed, how to make the change transparent to all
  virtual clocks?

Yes, this is a serious defect of this patch series, and there is no way to fix it.  In
the above example, suppose that domainNumber 1 needs +11 ppm and
domainNumber 2 needs -12 ppm.  You can't adjust one clock in two different
ways.

There may be some misunderstanding. In the example, domain 1, 2, 3 are based on PTP virtual clocks ptp1, ptp2 and ptp3 which are utilizing their own timecounter.
The clock adjustment on them won't affect each other. The example worked fine in my verification.

quoted

  Actually the frequency adjustmend on physical clock
  may be hidden by updating virtual clocks in opposite direction at same
  time. Considering the ppb values adjusted, the code execution delay
  will be little enough to ignore.

Assuming that the frequency offset is exactly the same on all domains, which
will very often be false.

I mean if the physical clock keeps free running, all virtual clocks utilizing their own timercounter can work fine independently without affecting on each other.
If the physical clock has change on frequency, there is also way to make the change not affect virtual clocks.
For example, when there is +12 ppm change on physical clock, just give -12 ppm change on virtual clocks.
And the code execution delay just has very little affecting on the time error considering only 12 ppm frequency adjusted.
This is a TODO work.

quoted

  But it's hard to hide clock stepping,
  by now I think the workaround may be inhibiting physical clock stepping
  when run user space ptp application.

That won't work either, because a phase offset on one domain will result in a
large slew at the maximum rate, but that rate would spoil the other domains.

Phase offset changing on physical clock affects virtual clocks. This is not able to address.
But there is workaround, like we inhibit physical clock stepping during PTP application running.

The best way to support multiple PTP domains simultaneously is in the
application.  It is really the only way, because the kernel does not handle any
details of the PTP, like domainNumber.  The kernel only provides clock control
and packet time stamping.

I understand. I explain my idea above to make it clear avoid misunderstanding.
This patch-set focuses on clock control and timestamping. It provides support for virtual clocks and domain timestamp related.
Put these in kernel, PTP applications don't have to do such thing by each of them.

ptp4l does not handle multiple domains today, but it definitely could be added
with some effort.  It would have to synchronize the clock to one chosen
domain, and track the phase and frequency offsets of each of the other
domains with respect to the chosen domain.  Having done this, the software
can convert time stamps between the domains perfectly.  Using the tracked
phase and frequency offsets, it can also switch domains seamlessly without
hacks or guesswork.

Sure. This can be done in application. I'm not sure whether my explaining address the "defect" you mentioned :)
I hope yes. Actually this patch-set can provide ptp1, ptp2, ... , which are standard ptp devices with domains bound.
ptp4l can directly use them for domain clock controls. The timestamp is already converted to domain in kernel.
This benefits all PTP applications.

So I have to say NAK to this series because it can't do any of that, and it cannot
be made to work either.

There may be misunderstanding.
Hope for comments after seeing my explaining:)

Thank you.

Thanks,
Richard