Intel-Wired-Lan Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Jacob Keller <jacob.e.keller@intel.com>
To: Ilya Evenbach <ievenbach@aurora.tech>,
	Alison Chaiken <achaiken@aurora.tech>,
	Steve Payne <spayne@aurora.tech>, <jesse.brandeburg@intel.com>,
	<richardcochran@gmail.com>, <netdev@vger.kernel.org>,
	<intel-wired-lan@lists.osuosl.org>
Subject: Re: [Intel-wired-lan] Fwd: [PATCH] Use ixgbe_ptp_reset on linkup/linkdown for X550
Date: Mon, 1 Aug 2022 16:29:40 -0700	[thread overview]
Message-ID: <bd24eeb0-318c-71a4-527f-02832b74250c@intel.com> (raw)
In-Reply-To: <CAJmffrr=J_s9cFw5Q58rvZRWLpsrDnx3RkRXS3oLZDYY3BrNcw@mail.gmail.com>



On 8/1/2022 4:00 PM, Ilya Evenbach wrote:
>>> -----Original Message-----
>>> From: achaiken@aurora.tech <achaiken@aurora.tech>
>>> Sent: Monday, August 01, 2022 6:38 AM
>>> To: Brandeburg, Jesse <jesse.brandeburg@intel.com>;
>>> richardcochran@gmail.com
>>> Cc: spayne@aurora.tech; achaiken@aurora.tech; alison@she-devel.com;
>>> netdev@vger.kernel.org; intel-wired-lan@lists.osuosl.org
>>> Subject: [PATCH] Use ixgbe_ptp_reset on linkup/linkdown for X550
>>>
>>> From: Steve Payne <spayne@aurora.tech>
>>>
>>> For an unknown reason, when `ixgbe_ptp_start_cyclecounter` is called
>>> from `ixgbe_watchdog_link_is_down` the PHC on the NIC jumps backward
>>> by a seemingly inconsistent amount, which causes discontinuities in
>>> time synchronization. Explicitly reset the NIC's PHC to
>>> `CLOCK_REALTIME` whenever the NIC goes up or down by calling
>>> `ixgbe_ptp_reset` instead of the bare `ixgbe_ptp_start_cyclecounter`.
>>>
>>> Signed-off-by: Steve Payne <spayne@aurora.tech>
>>> Signed-off-by: Alison Chaiken <achaiken@aurora.tech>
>>>
>>
>> Resetting PTP could be a problem if the clock was not being synchronized with the kernel CLOCK_REALTIME,
> 
> That is true, but most likely not really important, as the unmitigated
> problem also introduces significant discontinuities in time.
> Basically, this patch does not make things worse.
> 

Sure, but I am trying to see if I can understand *why* things get wonky.
I suspect the issue is caused because of how we're resetting the
cyclecounter.

>>
>> and does result in some loss of timer precision either way due to the delays involved with setting the time.
> 
>  That precision loss is negligible compared to jumps resulting from
> link down/up, and should be corrected by normal PTP operation very
> quickly.
> 

Only if CLOCK_REALTIME is actually being synchronized. Yes, that is
generally true, but its not necessarily guaranteed.

>>
>> Do you have an example of the clock jump? How much is it?
> 
> 2021-02-12T09:24:37.741191+00:00 bench-12 phc2sys: [195230.451]
> CLOCK_REALTIME phc offset        61 s2 freq  -36503 delay   2298
> 2021-02-12T09:24:38.741315+00:00 bench-12 phc2sys: [195231.451]
> CLOCK_REALTIME phc offset       169 s2 freq  -36377 delay   2294
> 2021-02-12T09:24:39.741407+00:00 bench-12 phc2sys: [195232.451]
> CLOCK_REALTIME phc offset 195213702387037 s2 freq +100000000 delay
> 2301
> 2021-02-12T09:24:40.741489+00:00 bench-12 phc2sys: [195233.452]
> CLOCK_REALTIME phc offset 195213591220495 s2 freq +100000000 delay
> 2081
> 

Thanks.

I think what's actually going on is a bug in the
ixgbe_ptp_start_cyclecounter function where the system time registers
are being reset.

What hardware are you operating on? Do you know if its an X550 board? It
looks like this has been the case since a9763f3cb54c ("ixgbe: Update PTP
to support X550EM_x devices").

The start_cyclecounter was never supposed to modify the current time
registers, but resetting it to 0 as it does for X550 devices would give
the exact behavior you're seeing.
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

  reply	other threads:[~2022-08-01 23:29 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-01 13:37 [Intel-wired-lan] [PATCH] Use ixgbe_ptp_reset on linkup/linkdown for X550 achaiken
2022-08-01 21:34 ` Keller, Jacob E
     [not found]   ` <CAFzL-7tX845o2kJmE4o8EhbeD-=vkR6rmaiz_ZEWfSD4W+iWEA@mail.gmail.com>
2022-08-01 22:53     ` Ilya Evenbach
2022-08-01 23:00       ` [Intel-wired-lan] Fwd: " Ilya Evenbach
2022-08-01 23:29         ` Jacob Keller [this message]
2022-08-02  0:24           ` Alison Chaiken
2022-08-02  0:38             ` Jacob Keller
2022-08-02  0:26           ` Jacob Keller
2022-10-04 18:14             ` Alison Chaiken
2022-10-04 21:08               ` Jacob Keller

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=bd24eeb0-318c-71a4-527f-02832b74250c@intel.com \
    --to=jacob.e.keller@intel.com \
    --cc=achaiken@aurora.tech \
    --cc=ievenbach@aurora.tech \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jesse.brandeburg@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=richardcochran@gmail.com \
    --cc=spayne@aurora.tech \
    /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