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

[Ferruh Yigit <ferruh.yigit@amd.com>]

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(-)
>