netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rahul Rameshbabu <rrameshbabu@nvidia.com>
To: Richard Cochran <richardcochran@gmail.com>
Cc: netdev@vger.kernel.org,  Saeed Mahameed <saeed@kernel.org>,
	 Gal Pressman <gal@nvidia.com>,  Tariq Toukan <tariqt@nvidia.com>,
	 "David S. Miller" <davem@davemloft.net>,
	 Jakub Kicinski <kuba@kernel.org>,
	 Jacob Keller <jacob.e.keller@intel.com>
Subject: Re: [PATCH net-next 1/9] ptp: Clarify ptp_clock_info .adjphase expects an internal servo to be used
Date: Mon, 22 May 2023 10:03:23 -0700	[thread overview]
Message-ID: <87mt1w5mec.fsf@nvidia.com> (raw)
In-Reply-To: <ZF2No6gW3HlzZscV@hoboy.vegasvil.org> (Richard Cochran's message of "Thu, 11 May 2023 17:51:47 -0700")

Hi Richard,

I have a v2 prepared. I have just one question left before sending that
it out.

On Thu, 11 May, 2023 17:51:47 -0700 Richard Cochran <richardcochran@gmail.com> wrote:
> On Thu, May 11, 2023 at 01:20:45PM -0700, Rahul Rameshbabu wrote:
>
>> If the PHC does not restore the frequency, won't the value cached in the
>> ptp stack in the kernel become inaccurate compared to the frequency
>> change induced by the '.adjphase' call?
>
> If the HW implements a PI controller, and if it has converged, then
> the current frequency will be close to the remote time server's.

This point makes sense to me. However, I have a concern about the case
where the linuxptp servo has not had a chance to make a single frequency
adjustment (0 ppb) and .adjphase/LOCKED_STABLE state is initially
called/reached. After converging, the frequency will be close to the
remote time's server's frequency, but that frequency will likely not be
0 ppb. If .adjfine had been called previously, the difference between
the remote time server's frequency and the cached frequency in the ptp
stack would likely be significantly closer. That said, do you think it
makes sense to have some kind of API that gives information about the
in-HW controller such as the frequency offset it operated? Or maybe in
general an API in the future for introspecting the state of this in-HW
servo?

>
>> This concern is why I added this
>> clause in the documentation. Let me know if my understanding is off with
>> regards to this. I think we had a similar conversation on this
>> previously in the mailing list.
>> 
>> https://lore.kernel.org/netdev/Y88L6EPtgvW4tSA+@hoboy.vegasvil.org/
>
> I guess it depends on the HW algorithm and the situation.  But I don't
> think there is a "rule" that always gets the best result.

Agreed. I do not think enforcing the PHC to restore the frequency to the
value before .adjphase is called would be helpful. This preserves the
integrity of the cached value in the kernel stack, but that is not
helpful since we can potentially see an initial growing error in the
offset between the remote server's time and the PHC's time after making
this frequency reversion.

>
> Thanks,
> Richard

-- Rahul Rameshbabu

  reply	other threads:[~2023-05-22 17:03 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-10 20:52 [PATCH net-next 0/9] ptp .adjphase cleanups Rahul Rameshbabu
2023-05-10 20:52 ` [PATCH net-next 1/9] ptp: Clarify ptp_clock_info .adjphase expects an internal servo to be used Rahul Rameshbabu
2023-05-11  2:09   ` Richard Cochran
2023-05-11 20:20     ` Rahul Rameshbabu
2023-05-12  0:51       ` Richard Cochran
2023-05-22 17:03         ` Rahul Rameshbabu [this message]
2023-05-22 20:07           ` Richard Cochran
2023-05-10 20:52 ` [PATCH net-next 2/9] docs: ptp.rst: Add information about NVIDIA Mellanox devices Rahul Rameshbabu
2023-05-11  2:10   ` Richard Cochran
2023-05-10 20:53 ` [PATCH net-next 3/9] testptp: Remove magic numbers related to nanosecond to second conversion Rahul Rameshbabu
2023-05-11  2:14   ` Richard Cochran
2023-05-10 20:53 ` [PATCH net-next 4/9] testptp: Add support for testing ptp_clock_info .adjphase callback Rahul Rameshbabu
2023-05-11  2:15   ` Richard Cochran
2023-05-10 20:53 ` [PATCH net-next 5/9] ptp: Add .getmaxphase callback to ptp_clock_info Rahul Rameshbabu
2023-05-11  2:20   ` Richard Cochran
2023-05-10 20:53 ` [PATCH net-next 6/9] net/mlx5: Add .getmaxphase ptp_clock_info callback Rahul Rameshbabu
2023-05-11  2:20   ` Richard Cochran
2023-05-10 20:53 ` [PATCH net-next 7/9] ptp: ptp_clockmatrix: " Rahul Rameshbabu
2023-05-10 20:53 ` [PATCH net-next 8/9] ptp: idt82p33: " Rahul Rameshbabu
2023-05-10 20:53 ` [PATCH net-next 9/9] ptp: ocp: " Rahul Rameshbabu
2023-05-11 11:12   ` Vadim Fedorenko
2023-05-11 20:35     ` Rahul Rameshbabu
2023-05-11  2:23 ` [PATCH net-next 0/9] ptp .adjphase cleanups Richard Cochran
2023-05-11 20:26   ` Rahul Rameshbabu

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=87mt1w5mec.fsf@nvidia.com \
    --to=rrameshbabu@nvidia.com \
    --cc=davem@davemloft.net \
    --cc=gal@nvidia.com \
    --cc=jacob.e.keller@intel.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=richardcochran@gmail.com \
    --cc=saeed@kernel.org \
    --cc=tariqt@nvidia.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).