[RFC,v3,0/3] add frequency adjustment support for PTP timesync

  This patchset cover below parts:
(1)Introduce a new timesync API called "rte_eth_timesync_adjust_freq" that
   enables frequency adjustment during PTP timesync. This new API aligns with
   the kernel PTP which already supports frequency adjustment. It brings DPDK
   closer in alignment with the kernel's best practice.

(2)Refine the ptpclient application by applying a PI algorithm that leverages
   the new API to further improve timesync accuracy. These changes doesn't break
   original solution and creates a more accurate solution for DPDK-based high
   accuracy PTP. We have provided significant improvements for timesync accuracy
   on e810 and we believe these improvements will also benefit other devices.

The original command for starting ptpclient is:
./build/examples/dpdk-ptpclient -a 0000:81:00.0 -c 1 -n 3 -- -T 0 -p 0x1

The command with PI algorithm is:
./build/examples/dpdk-ptpclient -a 0000:81:00.0 -c 1 -n 3 -- -T 0 -p 0x1 -- controller=pi

[RFC v3 1/3] ethdev: add frequency adjustment API.
[RFC v3 2/3] examples/ptpclient: refine application.
[RFC v3 3/3] examples/ptpclient: add frequency adjustment support.

v3:
* Refine commit log in patch.
* Add more description for the new API.

v2:
* Remove the ice PMD part from the RFC.
* Add description in cover letter.
* Refine commit log in patch.

Simei Su (3):
  ethdev: add frequency adjustment API
  examples/ptpclient: refine application
  examples/ptpclient: add frequency adjustment support

 examples/ptpclient/ptpclient.c   | 222 +++++++++++++++++++++++++++++++++------
 lib/ethdev/ethdev_driver.h       |   5 +
 lib/ethdev/ethdev_trace.h        |   9 ++
 lib/ethdev/ethdev_trace_points.c |   3 +
 lib/ethdev/rte_ethdev.c          |  19 ++++
 lib/ethdev/rte_ethdev.h          |  38 +++++++
 6 files changed, 265 insertions(+), 31 deletions(-)
  

On 5/22/2023 2:23 PM, Simei Su wrote:
> This patchset cover below parts:
> (1)Introduce a new timesync API called "rte_eth_timesync_adjust_freq" that
>    enables frequency adjustment during PTP timesync. This new API aligns with
>    the kernel PTP which already supports frequency adjustment. It brings DPDK
>    closer in alignment with the kernel's best practice.
> 

Hi Simei,

I am not expert on PTP, so please help me understand,

I can see there is a new API to update device frequency by updating
device clock, 'rte_eth_timesync_adjust_fine()', that part is OK.
(Although I have some reservation with the API name, we can visit it later.)

PTP doesn't enforce any specific sync method, right?
Like adjusting by adding/subtracting or adjusting by updating clock
frequency.
As far as I understand updating device clock frequency is HW capability
to adjust time, perhaps in a finer grain, is this correct?

If so, is this HW capability should be exposed to user with a new API?
If this is a generic HW capability, I think it is OK to have single
responsibility API, but if this is a vendor specific feature it should
be behind a generic API.


Or should we hide this capability behind existing
'rte_eth_timesync_adjust_time()' API, and HW/driver use best possible
method for synchronization, so this can be transparent the user.
What is the benefit to make user selected explicitly one option or other?



> (2)Refine the ptpclient application by applying a PI algorithm that leverages
>    the new API to further improve timesync accuracy. These changes doesn't break
>    original solution and creates a more accurate solution for DPDK-based high
>    accuracy PTP. We have provided significant improvements for timesync accuracy
>    on e810 and we believe these improvements will also benefit other devices.
> 

What is "PI algorithm"? and what PI stands for?

> The original command for starting ptpclient is:
> ./build/examples/dpdk-ptpclient -a 0000:81:00.0 -c 1 -n 3 -- -T 0 -p 0x1
> 
> The command with PI algorithm is:
> ./build/examples/dpdk-ptpclient -a 0000:81:00.0 -c 1 -n 3 -- -T 0 -p 0x1 -- controller=pi
> 
> [RFC v3 1/3] ethdev: add frequency adjustment API.
> [RFC v3 2/3] examples/ptpclient: refine application.
> [RFC v3 3/3] examples/ptpclient: add frequency adjustment support.
> 
> v3:
> * Refine commit log in patch.
> * Add more description for the new API.
> 
> v2:
> * Remove the ice PMD part from the RFC.
> * Add description in cover letter.
> * Refine commit log in patch.
> 
> Simei Su (3):
>   ethdev: add frequency adjustment API
>   examples/ptpclient: refine application
>   examples/ptpclient: add frequency adjustment support
> 
>  examples/ptpclient/ptpclient.c   | 222 +++++++++++++++++++++++++++++++++------
>  lib/ethdev/ethdev_driver.h       |   5 +
>  lib/ethdev/ethdev_trace.h        |   9 ++
>  lib/ethdev/ethdev_trace_points.c |   3 +
>  lib/ethdev/rte_ethdev.c          |  19 ++++
>  lib/ethdev/rte_ethdev.h          |  38 +++++++
>  6 files changed, 265 insertions(+), 31 deletions(-)
>
  

Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Saturday, June 3, 2023 3:44 AM
> To: Su, Simei <simei.su@intel.com>; thomas@monjalon.net;
> andrew.rybchenko@oktetlabs.ru; Rybalchenko, Kirill
> <kirill.rybalchenko@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wu, Wenjun1 <wenjun1.wu@intel.com>
> Subject: Re: [RFC v3 0/3] add frequency adjustment support for PTP timesync
> 
> On 5/22/2023 2:23 PM, Simei Su wrote:
> > This patchset cover below parts:
> > (1)Introduce a new timesync API called "rte_eth_timesync_adjust_freq"
> that
> >    enables frequency adjustment during PTP timesync. This new API aligns
> with
> >    the kernel PTP which already supports frequency adjustment. It brings
> DPDK
> >    closer in alignment with the kernel's best practice.
> >
> 
> Hi Simei,
> 
> I am not expert on PTP, so please help me understand,
> 
> I can see there is a new API to update device frequency by updating device
> clock, 'rte_eth_timesync_adjust_fine()', that part is OK.
> (Although I have some reservation with the API name, we can visit it later.)

Yes, this API is to adjust the frequency in a finer granularity, so naming it as 'rte_eth_timesync_adjust_fine()'.

> 
> PTP doesn't enforce any specific sync method, right?
> Like adjusting by adding/subtracting or adjusting by updating clock frequency.
> As far as I understand updating device clock frequency is HW capability to
> adjust time, perhaps in a finer grain, is this correct?

Yes, it's correct.

> 
> If so, is this HW capability should be exposed to user with a new API?
> If this is a generic HW capability, I think it is OK to have single responsibility
> API, but if this is a vendor specific feature it should be behind a generic API.

It's a generic HW capability instead of vendor specific feature.

> 
> 
> Or should we hide this capability behind existing
> 'rte_eth_timesync_adjust_time()' API, and HW/driver use best possible
> method for synchronization, so this can be transparent the user.
> What is the benefit to make user selected explicitly one option or other?
> 

In general, 'rte_eth_timesync_adjust_fine()' is used in conjunction with
'rte_eth_timesync_adjust_time()' at different moment to do more precise time control.

> 
> 
> > (2)Refine the ptpclient application by applying a PI algorithm that leverages
> >    the new API to further improve timesync accuracy. These changes
> doesn't break
> >    original solution and creates a more accurate solution for DPDK-based
> high
> >    accuracy PTP. We have provided significant improvements for timesync
> accuracy
> >    on e810 and we believe these improvements will also benefit other
> devices.
> >
> 
> What is "PI algorithm"? and what PI stands for?

Proportional-integral-derivative control is called PID controller. It is widely used in industrial control.
The control deviation is constituted based on the given value and the actual output value.
The deviation produces control quantity based on proportional, integrated and differentiated linear combination, thereby controlling the controlled object.

'P' means proportional controller. It changes the magnitude only.
'I' means integral controller. It lags the output phase.
'PI' means proportional Integral controller. It changes the magnitude as well as laging the output.

Proportional-Integral(PI) controller is used to produce a fractional tick-rate adjustment that coordinates the local clock with 
master clock time. The PI controller corrects both the time and rate of the local clock. The proportional term tracks and 
corrects the direct input, which is the time difference between two clocks. The integral term tracks and corrects steady-state 
error, which is the rate difference between two clocks.

Thanks,
Simei

> 
> > The original command for starting ptpclient is:
> > ./build/examples/dpdk-ptpclient -a 0000:81:00.0 -c 1 -n 3 -- -T 0 -p
> > 0x1
> >
> > The command with PI algorithm is:
> > ./build/examples/dpdk-ptpclient -a 0000:81:00.0 -c 1 -n 3 -- -T 0 -p
> > 0x1 -- controller=pi
> >
> > [RFC v3 1/3] ethdev: add frequency adjustment API.
> > [RFC v3 2/3] examples/ptpclient: refine application.
> > [RFC v3 3/3] examples/ptpclient: add frequency adjustment support.
> >
> > v3:
> > * Refine commit log in patch.
> > * Add more description for the new API.
> >
> > v2:
> > * Remove the ice PMD part from the RFC.
> > * Add description in cover letter.
> > * Refine commit log in patch.
> >
> > Simei Su (3):
> >   ethdev: add frequency adjustment API
> >   examples/ptpclient: refine application
> >   examples/ptpclient: add frequency adjustment support
> >
> >  examples/ptpclient/ptpclient.c   | 222
> +++++++++++++++++++++++++++++++++------
> >  lib/ethdev/ethdev_driver.h       |   5 +
> >  lib/ethdev/ethdev_trace.h        |   9 ++
> >  lib/ethdev/ethdev_trace_points.c |   3 +
> >  lib/ethdev/rte_ethdev.c          |  19 ++++
> >  lib/ethdev/rte_ethdev.h          |  38 +++++++
> >  6 files changed, 265 insertions(+), 31 deletions(-)
> >
  

On 6/7/2023 9:19 AM, Su, Simei wrote:
> Hi Ferruh,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Saturday, June 3, 2023 3:44 AM
>> To: Su, Simei <simei.su@intel.com>; thomas@monjalon.net;
>> andrew.rybchenko@oktetlabs.ru; Rybalchenko, Kirill
>> <kirill.rybalchenko@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
>> Cc: dev@dpdk.org; Wu, Wenjun1 <wenjun1.wu@intel.com>
>> Subject: Re: [RFC v3 0/3] add frequency adjustment support for PTP timesync
>>
>> On 5/22/2023 2:23 PM, Simei Su wrote:
>>> This patchset cover below parts:
>>> (1)Introduce a new timesync API called "rte_eth_timesync_adjust_freq"
>> that
>>>    enables frequency adjustment during PTP timesync. This new API aligns
>> with
>>>    the kernel PTP which already supports frequency adjustment. It brings
>> DPDK
>>>    closer in alignment with the kernel's best practice.
>>>
>>
>> Hi Simei,
>>
>> I am not expert on PTP, so please help me understand,
>>
>> I can see there is a new API to update device frequency by updating device
>> clock, 'rte_eth_timesync_adjust_fine()', that part is OK.
>> (Although I have some reservation with the API name, we can visit it later.)
> 
> Yes, this API is to adjust the frequency in a finer granularity, so naming it as 'rte_eth_timesync_adjust_fine()'.
> 

Can existing 'rte_eth_timesync_adjust_time()' API also used to adjust
the device clock frequency?

If so, since 'fine' is subjective, how user can decide to use one or
other, and why we can't use existing API?

>>
>> PTP doesn't enforce any specific sync method, right?
>> Like adjusting by adding/subtracting or adjusting by updating clock frequency.
>> As far as I understand updating device clock frequency is HW capability to
>> adjust time, perhaps in a finer grain, is this correct?
> 
> Yes, it's correct.
> 
>>
>> If so, is this HW capability should be exposed to user with a new API?
>> If this is a generic HW capability, I think it is OK to have single responsibility
>> API, but if this is a vendor specific feature it should be behind a generic API.
> 
> It's a generic HW capability instead of vendor specific feature.
> 
>>
>>
>> Or should we hide this capability behind existing
>> 'rte_eth_timesync_adjust_time()' API, and HW/driver use best possible
>> method for synchronization, so this can be transparent the user.
>> What is the benefit to make user selected explicitly one option or other?
>>
> 
> In general, 'rte_eth_timesync_adjust_fine()' is used in conjunction with
> 'rte_eth_timesync_adjust_time()' at different moment to do more precise time control.
> 

If I understand this is not defined by PTP control, so how user can do
distinction between two?

Or same argument again, can user call same existing API and this detail
can be hidden behind implementation? Meaning driver/HW selects that
moment to call one or other.

>>
>>
>>> (2)Refine the ptpclient application by applying a PI algorithm that leverages
>>>    the new API to further improve timesync accuracy. These changes
>> doesn't break
>>>    original solution and creates a more accurate solution for DPDK-based
>> high
>>>    accuracy PTP. We have provided significant improvements for timesync
>> accuracy
>>>    on e810 and we believe these improvements will also benefit other
>> devices.
>>>
>>
>> What is "PI algorithm"? and what PI stands for?
> 
> Proportional-integral-derivative control is called PID controller. It is widely used in industrial control.
> The control deviation is constituted based on the given value and the actual output value.
> The deviation produces control quantity based on proportional, integrated and differentiated linear combination, thereby controlling the controlled object.
> 
> 'P' means proportional controller. It changes the magnitude only.
> 'I' means integral controller. It lags the output phase.
> 'PI' means proportional Integral controller. It changes the magnitude as well as laging the output.
> 
> Proportional-Integral(PI) controller is used to produce a fractional tick-rate adjustment that coordinates the local clock with 
> master clock time. The PI controller corrects both the time and rate of the local clock. The proportional term tracks and 
> corrects the direct input, which is the time difference between two clocks. The integral term tracks and corrects steady-state 
> error, which is the rate difference between two clocks.
> 

Thank you.

> Thanks,
> Simei
> 
>>
>>> The original command for starting ptpclient is:
>>> ./build/examples/dpdk-ptpclient -a 0000:81:00.0 -c 1 -n 3 -- -T 0 -p
>>> 0x1
>>>
>>> The command with PI algorithm is:
>>> ./build/examples/dpdk-ptpclient -a 0000:81:00.0 -c 1 -n 3 -- -T 0 -p
>>> 0x1 -- controller=pi
>>>
>>> [RFC v3 1/3] ethdev: add frequency adjustment API.
>>> [RFC v3 2/3] examples/ptpclient: refine application.
>>> [RFC v3 3/3] examples/ptpclient: add frequency adjustment support.
>>>
>>> v3:
>>> * Refine commit log in patch.
>>> * Add more description for the new API.
>>>
>>> v2:
>>> * Remove the ice PMD part from the RFC.
>>> * Add description in cover letter.
>>> * Refine commit log in patch.
>>>
>>> Simei Su (3):
>>>   ethdev: add frequency adjustment API
>>>   examples/ptpclient: refine application
>>>   examples/ptpclient: add frequency adjustment support
>>>
>>>  examples/ptpclient/ptpclient.c   | 222
>> +++++++++++++++++++++++++++++++++------
>>>  lib/ethdev/ethdev_driver.h       |   5 +
>>>  lib/ethdev/ethdev_trace.h        |   9 ++
>>>  lib/ethdev/ethdev_trace_points.c |   3 +
>>>  lib/ethdev/rte_ethdev.c          |  19 ++++
>>>  lib/ethdev/rte_ethdev.h          |  38 +++++++
>>>  6 files changed, 265 insertions(+), 31 deletions(-)
>>>
>
  

Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Thursday, June 8, 2023 2:29 AM
> To: Su, Simei <simei.su@intel.com>; thomas@monjalon.net;
> andrew.rybchenko@oktetlabs.ru; Rybalchenko, Kirill
> <kirill.rybalchenko@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wu, Wenjun1 <wenjun1.wu@intel.com>
> Subject: Re: [RFC v3 0/3] add frequency adjustment support for PTP timesync
> 
> On 6/7/2023 9:19 AM, Su, Simei wrote:
> > Hi Ferruh,
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@amd.com>
> >> Sent: Saturday, June 3, 2023 3:44 AM
> >> To: Su, Simei <simei.su@intel.com>; thomas@monjalon.net;
> >> andrew.rybchenko@oktetlabs.ru; Rybalchenko, Kirill
> >> <kirill.rybalchenko@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> >> Cc: dev@dpdk.org; Wu, Wenjun1 <wenjun1.wu@intel.com>
> >> Subject: Re: [RFC v3 0/3] add frequency adjustment support for PTP
> >> timesync
> >>
> >> On 5/22/2023 2:23 PM, Simei Su wrote:
> >>> This patchset cover below parts:
> >>> (1)Introduce a new timesync API called "rte_eth_timesync_adjust_freq"
> >> that
> >>>    enables frequency adjustment during PTP timesync. This new API
> >>> aligns
> >> with
> >>>    the kernel PTP which already supports frequency adjustment. It
> >>> brings
> >> DPDK
> >>>    closer in alignment with the kernel's best practice.
> >>>
> >>
> >> Hi Simei,
> >>
> >> I am not expert on PTP, so please help me understand,
> >>
> >> I can see there is a new API to update device frequency by updating
> >> device clock, 'rte_eth_timesync_adjust_fine()', that part is OK.
> >> (Although I have some reservation with the API name, we can visit it
> >> later.)
> >
> > Yes, this API is to adjust the frequency in a finer granularity, so naming it as
> 'rte_eth_timesync_adjust_fine()'.
> >
> 
> Can existing 'rte_eth_timesync_adjust_time()' API also used to adjust the
> device clock frequency?

' rte_eth_timesync_adjust_time()' API focuses on phase adjustment while
' rte_eth_timesync_adjust_fine()' API focused on frequency adjustment.
They focus on different aspects.

> 
> If so, since 'fine' is subjective, how user can decide to use one or other, and
> why we can't use existing API?

Previously, we only use ' rte_eth_timesync_adjust_time()' API to do time synchronization,
but the accuracy is low. We found the gap maybe missing frequency adjustment
which is already supported in kernel driver and linuxptp application.
So we implement it to bring DPDK closer in alignment with the kernel's best practice and
find the accuracy has a significant improvement.

E.g, below is a simple usage:
if master offset > master offset threshold
   do rte_eth_timesync_adjust_time;
else
   do rte_eth_timesync_adjust_fine;.

> 
> >>
> >> PTP doesn't enforce any specific sync method, right?
> >> Like adjusting by adding/subtracting or adjusting by updating clock
> frequency.
> >> As far as I understand updating device clock frequency is HW
> >> capability to adjust time, perhaps in a finer grain, is this correct?
> >
> > Yes, it's correct.
> >
> >>
> >> If so, is this HW capability should be exposed to user with a new API?
> >> If this is a generic HW capability, I think it is OK to have single
> >> responsibility API, but if this is a vendor specific feature it should be behind
> a generic API.
> >
> > It's a generic HW capability instead of vendor specific feature.
> >
> >>
> >>
> >> Or should we hide this capability behind existing
> >> 'rte_eth_timesync_adjust_time()' API, and HW/driver use best possible
> >> method for synchronization, so this can be transparent the user.
> >> What is the benefit to make user selected explicitly one option or other?
> >>
> >
> > In general, 'rte_eth_timesync_adjust_fine()' is used in conjunction
> > with 'rte_eth_timesync_adjust_time()' at different moment to do more
> precise time control.
> >
> 
> If I understand this is not defined by PTP control, so how user can do
> distinction between two?
> 
> Or same argument again, can user call same existing API and this detail can be
> hidden behind implementation? Meaning driver/HW selects that moment to
> call one or other.

In summary, the new API is introduced because of below consideration:
(1) align with kernel driver and linuxptp application
(2) it's not vendor specific API but generic API
(3) refine current algorithm in ptpclient application to improve time accuracy
(4) the focus is different from ' rte_eth_timesync_adjust_time'

In order not to destroy original framework in ptpclient application and let it can be used by other PMDs which is without frequency adjustment for normal use,
we add --controller=pi in starting command to select using PI algorithm to improve accuracy. Because besides PI algorithm, other linear algorithm can
be used for it.

Thanks,
Simei

> 
> >>
> >>
> >>> (2)Refine the ptpclient application by applying a PI algorithm that
> leverages
> >>>    the new API to further improve timesync accuracy. These changes
> >> doesn't break
> >>>    original solution and creates a more accurate solution for
> >>> DPDK-based
> >> high
> >>>    accuracy PTP. We have provided significant improvements for
> >>> timesync
> >> accuracy
> >>>    on e810 and we believe these improvements will also benefit other
> >> devices.
> >>>
> >>
> >> What is "PI algorithm"? and what PI stands for?
> >
> > Proportional-integral-derivative control is called PID controller. It is widely
> used in industrial control.
> > The control deviation is constituted based on the given value and the actual
> output value.
> > The deviation produces control quantity based on proportional, integrated
> and differentiated linear combination, thereby controlling the controlled
> object.
> >
> > 'P' means proportional controller. It changes the magnitude only.
> > 'I' means integral controller. It lags the output phase.
> > 'PI' means proportional Integral controller. It changes the magnitude as well
> as laging the output.
> >
> > Proportional-Integral(PI) controller is used to produce a fractional
> > tick-rate adjustment that coordinates the local clock with master
> > clock time. The PI controller corrects both the time and rate of the
> > local clock. The proportional term tracks and corrects the direct input,
> which is the time difference between two clocks. The integral term tracks and
> corrects steady-state error, which is the rate difference between two clocks.
> >
> 
> Thank you.
> 
> > Thanks,
> > Simei
> >
> >>
> >>> The original command for starting ptpclient is:
> >>> ./build/examples/dpdk-ptpclient -a 0000:81:00.0 -c 1 -n 3 -- -T 0 -p
> >>> 0x1
> >>>
> >>> The command with PI algorithm is:
> >>> ./build/examples/dpdk-ptpclient -a 0000:81:00.0 -c 1 -n 3 -- -T 0 -p
> >>> 0x1 -- controller=pi
> >>>
> >>> [RFC v3 1/3] ethdev: add frequency adjustment API.
> >>> [RFC v3 2/3] examples/ptpclient: refine application.
> >>> [RFC v3 3/3] examples/ptpclient: add frequency adjustment support.
> >>>
> >>> v3:
> >>> * Refine commit log in patch.
> >>> * Add more description for the new API.
> >>>
> >>> v2:
> >>> * Remove the ice PMD part from the RFC.
> >>> * Add description in cover letter.
> >>> * Refine commit log in patch.
> >>>
> >>> Simei Su (3):
> >>>   ethdev: add frequency adjustment API
> >>>   examples/ptpclient: refine application
> >>>   examples/ptpclient: add frequency adjustment support
> >>>
> >>>  examples/ptpclient/ptpclient.c   | 222
> >> +++++++++++++++++++++++++++++++++------
> >>>  lib/ethdev/ethdev_driver.h       |   5 +
> >>>  lib/ethdev/ethdev_trace.h        |   9 ++
> >>>  lib/ethdev/ethdev_trace_points.c |   3 +
> >>>  lib/ethdev/rte_ethdev.c          |  19 ++++
> >>>  lib/ethdev/rte_ethdev.h          |  38 +++++++
> >>>  6 files changed, 265 insertions(+), 31 deletions(-)
> >>>
> >