netdev - Re: Fwd: [PATCH] Use ixgbe_ptp_reset on linkup/linkdown for X550

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 4 Oct 2022 11:14:50 -0700
From:   Alison Chaiken <achaiken@...ora.tech>
To:     anthony.l.nguyen@...el.com
Cc:     Steve Payne <spayne@...ora.tech>, jesse.brandeburg@...el.com,
        richardcochran@...il.com, netdev@...r.kernel.org,
        intel-wired-lan@...ts.osuosl.org,
        Jacob Keller <jacob.e.keller@...el.com>
Subject: Re: Fwd: [PATCH] Use ixgbe_ptp_reset on linkup/linkdown for X550

How about this Intel X550 PTP fix for 6.1?

https://lists.osuosl.org/pipermail/intel-wired-lan/Week-of-Mon-20220801/029590.html

Thanks,
Alison Chaiken
Aurora Innovation

On Mon, Aug 1, 2022 at 5:26 PM Jacob Keller <jacob.e.keller@...el.com> wrote:
>
> On 8/1/2022 4:29 PM, Jacob Keller wrote:
> >
> >
> > On 8/1/2022 4:00 PM, Ilya Evenbach wrote:
> >>>> -----Original Message-----
> >>>> From: achaiken@...ora.tech <achaiken@...ora.tech>
> >>>> Sent: Monday, August 01, 2022 6:38 AM
> >>>> To: Brandeburg, Jesse <jesse.brandeburg@...el.com>;
> >>>> richardcochran@...il.com
> >>>> Cc: spayne@...ora.tech; achaiken@...ora.tech; alison@...-devel.com;
> >>>> netdev@...r.kernel.org; intel-wired-lan@...ts.osuosl.org
> >>>> Subject: [PATCH] Use ixgbe_ptp_reset on linkup/linkdown for X550
> >>>>
> >>>> From: Steve Payne <spayne@...ora.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@...ora.tech>
> >>>> Signed-off-by: Alison Chaiken <achaiken@...ora.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.
>
> I just posted an alternative fix which I believe resolves this issue.
>
> Thanks,
> Jake