netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Richard Cochran <richardcochran@gmail.com>
Cc: "Köry Maincent" <kory.maincent@bootlin.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	andrew@lunn.ch, davem@davemloft.net, f.fainelli@gmail.com,
	hkallweit1@gmail.com, netdev@vger.kernel.org,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Maxime Chevallier" <maxime.chevallier@bootlin.com>
Subject: Re: [PATCH RFC net-next] net: phy: add Marvell PHY PTP support [multicast/DSA issues]
Date: Thu, 2 Mar 2023 11:49:26 +0000	[thread overview]
Message-ID: <ZACNRjCojuK6tcnl@shell.armlinux.org.uk> (raw)
In-Reply-To: <ZAAn1deCtR0BoVAm@hoboy.vegasvil.org>

On Wed, Mar 01, 2023 at 08:36:37PM -0800, Richard Cochran wrote:
> On Wed, Mar 01, 2023 at 05:04:08PM +0100, Köry Maincent wrote:
> > I suppose the idea of Russell to rate each PTP clocks may be the best one, as
> > all others solutions have drawbacks. Does using the PTP clock period value (in
> > picoseconds) is enough to decide which PTP is the best one? It is hardware
> > specific therefore it is legitimate to be set by the MAC and PHY drivers.
> 
> It is not that simple.  In fact, I've never seen an objective
> comparision of different HW.  The vendors surely have no reason to
> conduct such a study.  Also, the data sheets never make any claim
> about synchronization quality.

mvpp2 (MAC) hardware has a counter in hardware that we can read the
timestamps from, and it supports being fine-tuned and stepped in
hardware. So the hardware gives us the timestamps with no shenanigans.
The hardware timestamp is incremented by a programmable amount
(including a 32-bit fractional part) every 3ns.

The mvpp2 hardware captures the timestamp when receiving packets, and
places the timestamp in the buffer descriptors - there is no filtering
and this timestamping essentially comes for free from the software
perspective. There is no requirement to read any registers before the
next packet that needs to be timestamped, and the timestamps can not
be confused which packet they are intended for. Since these timestamps
are in the buffer descriptors, they are available as soon as the packet
is ready to be passed to the networking layers.

Timestamps can be inserted into transmitted packets and the checksums
updated by the hardware. The hardware has capability to do a number
of operations on the packets when inserting the timestamp, although
I never wrote software support for this (I wanted some way to
positively test these but I don't think I had a way to do that.)
Multiple transmit packets can be queued for timestamping and the
hardware will cope.

Hardware signals are supported. The hardware has event capture,
capabilities, which snapshots the counter, and this should give high
accuracy. However, for generation of events, e.g. PPS, I have observed
that there is no way to ensure that the PPS signal is aligned to the
start of a second. That said, the signal is subject to the fine
adjustments of the hardware counter - so increasing the hardware
counter's increment correctly shortens the PPS signal period.

All accesses to the hardware are fast, being MMIO, which gives a
very stable reading when the hardware clock is adjusted by ptp4l.

The hardware timestamp counter is shared across all three ethernet
interfaces of the CP110 die, and there can be more than one CP110
die in a SoC.


Marvell PHY hardware is qutie different. The counter is updated every
8ns, and merely increments by one each time. There is no fractional
adjustment, meaning that there is no fine adjustment of the hardware.
Fine adjustment is performed by using the kernel's timecounter
infrastructure.

The counter is accessed over MDIO which appears to introduce a lot of
variability in latency, which transfers over to read accesses to the
counter, and thus the read timestamp. This makes it harder for ptp4l to
synchronise the counter, and from my testing, introduces a lot of
noise.

The PHY can't modify the packet in any way, but merely captures the
counter when the packet is transmitted. The PHY needs to be programmed
with the byte offset from the start of the packet to the ethernet
protocol, and the ethernet protocol to the IP header - suggesting
that the PTP hardware needs to know whether packets will or will not
be incorporating a VLAN header (this is explicitly mentioned in e.g.
the 88e151x manual.)

The PHY's filtering applies to both the transmit and receive paths,
and as there is only one set of capture registers for a transmit
packet, this may become a problem, especially if we are not using
interrupts for the PHY. Without interrupts, at best we poll the PHY
every 2ms (but could be longer due to MDIO access times.)

For receive, the PHY implement two capture register sets. The PHY
needs to parse the packet (using the offsets into the packet above)
in order to retrieve the MessageID field which is then used to work
out which receive capture register to write the packet's counter
value to. This means if one receives several PTP packets in quick
succession, there is a chance to lose timestamps.

The situation with hardware signals will be similar to PP2, in that
event capture takes a snapshot of the hardware counter, which can
then be translated. However, as the hardware counter does not get
adjusted, signal generation is tied to the unadjusted rate at which
the hardware ticks at, which means that asking for a PPS, the
generated signal will drift as the PHY 125MHz clock drifts.


In conclusion, not only does PP2 have a 3ns tick vs 8ns for PHY, PP2
can more reliably capture the timestamps, reading the hardware counter
value has less noise, and thus can be synchronised by ptp4l with far
less random variance than the PHY implementation.

Therefore, I believe that the Marvell PHY PTP implementation is all
round inferior to that found in the Marvell PP2 MAC, and hence why I
believe that the PP2 MAC implementation should be used by default over
the PHY.


(In essence, because of all the noise when trying the Marvell PHY with
ptp4l, I came to the conlusion that NTP was a far better solution to
time synchronisation between machines than PTP would ever be due to
the nose induced by MDIO access. However, I should also state that I
basically gave up with PTP in the end because hardware support is
overall poor, and NTP just works - and I'd still have to run NTP for
the machines that have no PTP capabilities. PTP probably only makes
sense if one has a nice expensive grand master PTP clock on ones
network, and all the machines one wants to synchronise have decent
PTP implementations.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2023-03-02 11:49 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-14 16:26 [PATCH RFC net-next] net: phy: add Marvell PHY PTP support Russell King
2020-07-15 18:38 ` Andrew Lunn
2020-07-15 18:56   ` Russell King - ARM Linux admin
2020-07-16 11:33     ` Russell King - ARM Linux admin
2020-07-16 20:53       ` Richard Cochran
2020-07-16 20:48 ` Richard Cochran
2020-07-17  7:54   ` Kurt Kanzenbach
2020-07-18  2:24     ` Richard Cochran
2020-07-20 14:21       ` Richard Cochran
2020-07-20 14:37         ` Kurt Kanzenbach
2020-07-26 23:48 ` Russell King - ARM Linux admin
2020-07-29 10:58 ` Russell King - ARM Linux admin
2020-07-29 13:19   ` Richard Cochran
2020-07-29 13:28     ` Russell King - ARM Linux admin
2020-07-29 22:07       ` Russell King - ARM Linux admin
2020-07-29 22:53         ` Vladimir Oltean
2020-07-30 15:53         ` Richard Cochran
2020-07-30 18:38           ` Russell King - ARM Linux admin
2020-07-30 19:32             ` Richard Cochran
2020-07-30 19:44               ` Russell King - ARM Linux admin
2020-07-30 11:06     ` [PATCH RFC net-next] net: phy: add Marvell PHY PTP support [multicast/DSA issues] Russell King - ARM Linux admin
2020-07-30 11:54       ` Russell King - ARM Linux admin
2020-07-30 12:47         ` Russell King - ARM Linux admin
2023-02-27 14:40           ` Köry Maincent
2023-02-27 15:20             ` Russell King (Oracle)
2023-02-27 17:30               ` Köry Maincent
2023-02-27 17:42                 ` Russell King (Oracle)
2023-02-27 19:45               ` Richard Cochran
2023-02-27 20:09                 ` Russell King (Oracle)
2023-02-27 20:19                   ` Richard Cochran
2023-02-28 12:07                     ` Russell King (Oracle)
2023-02-28 13:16                       ` Köry Maincent
2023-02-28 13:36                         ` Russell King (Oracle)
2023-02-28 14:50                           ` Köry Maincent
2023-02-28 15:16                         ` Richard Cochran
2023-02-28 15:33                           ` Andrew Lunn
2023-02-28 21:13                             ` Richard Cochran
2023-02-28 16:27                           ` Russell King (Oracle)
2023-02-28 16:44                             ` Michael Walle
2023-02-28 16:58                               ` Russell King (Oracle)
2023-02-28 20:13                                 ` Michael Walle
2023-02-28 21:11                                   ` Richard Cochran
2023-02-28 21:24                             ` Richard Cochran
2023-02-28 22:26                             ` Jakub Kicinski
2023-02-28 22:40                               ` Russell King (Oracle)
2023-02-28 22:59                                 ` Jakub Kicinski
2023-03-01 16:04                                   ` Köry Maincent
2023-03-02  4:36                                     ` Richard Cochran
2023-03-02 11:49                                       ` Russell King (Oracle) [this message]
2023-03-02 16:49                                         ` Jakub Kicinski
2023-03-02 17:06                                           ` Köry Maincent
2023-03-02 17:23                                             ` Jakub Kicinski
2023-03-03 13:12                                               ` Köry Maincent
2023-03-03 23:28                                                 ` Jakub Kicinski
2023-03-02 17:26                                           ` Russell King (Oracle)
2023-03-03 10:20                                             ` Michael Walle
2023-03-03 13:20                                               ` Andrew Lunn
2023-03-03 13:34                                                 ` Köry Maincent
2023-03-03 13:59                                                   ` Andrew Lunn
2023-03-03 14:03                                                 ` Russell King (Oracle)
2023-03-03 16:34                                                   ` Andrew Lunn
2023-03-03 17:32                                                     ` Richard Cochran
2023-03-03 17:35                                                       ` Richard Cochran
2023-03-03 23:40                                                   ` Jakub Kicinski
2023-03-02 21:28                                           ` Richard Cochran
2023-03-02 21:19                                         ` Richard Cochran
2023-04-27 15:13               ` Köry Maincent
2023-04-27 16:50                 ` Andrew Lunn
2023-04-28  8:51                   ` Köry Maincent
2020-07-30 15:50         ` Richard Cochran
2020-07-31 14:41         ` Andrew Lunn
2023-03-02 10:37   ` [PATCH RFC net-next] net: phy: add Marvell PHY PTP support Köry Maincent
2023-03-02 17:38     ` Russell King (Oracle)
2023-03-02 21:35     ` Richard Cochran

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=ZACNRjCojuK6tcnl@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=kory.maincent@bootlin.com \
    --cc=kuba@kernel.org \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=richardcochran@gmail.com \
    --cc=thomas.petazzoni@bootlin.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).