DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@amd.com>
To: "Su, Simei" <simei.su@intel.com>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	"andrew.rybchenko@oktetlabs.ru" <andrew.rybchenko@oktetlabs.ru>,
	"Rybalchenko, Kirill" <kirill.rybalchenko@intel.com>,
	"Zhang, Qi Z" <qi.z.zhang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "Wu, Wenjun1" <wenjun1.wu@intel.com>
Subject: Re: [RFC v3 0/3] add frequency adjustment support for PTP timesync
Date: Wed, 7 Jun 2023 19:29:07 +0100	[thread overview]
Message-ID: <7dcff833-e921-8ee9-b821-25bd6c7cc980@amd.com> (raw)
In-Reply-To: <SA1PR11MB6613F8345FD92502E4B110229C53A@SA1PR11MB6613.namprd11.prod.outlook.com>

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


  reply	other threads:[~2023-06-07 18:29 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-31  2:22 [RFC 0/4] add frequency adjustment support for PTP Simei Su
2023-03-31  2:22 ` [RFC 1/4] ethdev: add frequency adjustment API Simei Su
2023-03-31  2:22 ` [RFC 2/4] net/ice: add frequency adjustment support for PTP Simei Su
2023-03-31  2:22 ` [RFC 3/4] examples/ptpclient: refine application Simei Su
2023-03-31  2:22 ` [RFC 4/4] examples/ptpclient: add frequency adjustment support Simei Su
2023-04-03  9:22 ` [RFC v2 0/3] add frequency adjustment support for PTP timesync Simei Su
2023-04-03  9:22   ` [RFC v2 1/3] ethdev: add frequency adjustment API Simei Su
2023-05-15 14:18     ` Thomas Monjalon
2023-05-24  9:25       ` Su, Simei
2023-04-03  9:22   ` [RFC v2 2/3] examples/ptpclient: refine application Simei Su
2023-04-03  9:22   ` [RFC v2 3/3] examples/ptpclient: add frequency adjustment support Simei Su
2023-05-22 13:23   ` [RFC v3 0/3] add frequency adjustment support for PTP timesync Simei Su
2023-05-22 13:23     ` [RFC v3 1/3] ethdev: add frequency adjustment API Simei Su
2023-05-22 13:23     ` [RFC v3 2/3] examples/ptpclient: refine application Simei Su
2023-06-02 19:45       ` Ferruh Yigit
2023-05-22 13:23     ` [RFC v3 3/3] examples/ptpclient: add frequency adjustment support Simei Su
2023-06-02 19:52       ` Ferruh Yigit
2023-06-07 10:04         ` Su, Simei
2023-06-02 19:44     ` [RFC v3 0/3] add frequency adjustment support for PTP timesync Ferruh Yigit
2023-06-07  8:19       ` Su, Simei
2023-06-07 18:29         ` Ferruh Yigit [this message]
2023-06-08  4:05           ` Su, Simei

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7dcff833-e921-8ee9-b821-25bd6c7cc980@amd.com \
    --to=ferruh.yigit@amd.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=kirill.rybalchenko@intel.com \
    --cc=qi.z.zhang@intel.com \
    --cc=simei.su@intel.com \
    --cc=thomas@monjalon.net \
    --cc=wenjun1.wu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).