Ed uses the ptp_kvm driver with phc2sys, and Debian 10 has an udev rule
which creates a symlink for this device called /dev/ptp_kvm.
/lib/udev/rules.d/50-udev-default.rules:
SUBSYSTEM=="ptp", ATTR{clock_name}=="KVM virtual PTP", SYMLINK += "ptp_kvm"
Since the blamed patch, posix_clock_open(), called from a variety of
places including the phc2sys clock_add(), returns an error which states
that it cannot deduce the PHC index from this char device symlink.
It appears that phc2sys supports, to some extent, synchronizing PTP
clocks specified by path to the char device, for which the PHC index is
not known. In fact, there are some places in phc2sys where clocks are
compared by PHC number, but almost miraculously, it manages to not trip
over the fact that the PHC number is missing.
For example:
- find_dst_clock() finds clocks by PHC index, but it is only called from
the "automatic" (phc2sys -a) code path. In that mode, phc2sys finds
the PHC through this algorithm:
- send a PORT_PROPERTIES_NP management TLV to ptp4l which contains the
port interface name as string ("eth0" etc)
- retrieve the PHC device through ethtool ioctls and compute the device
path as /dev/ptpN as discussed
So it manages to find the char device path in the reverse direction
(the PHC number is already known).
- do_loop() has this logic:
/* don't try to synchronize the clock to itself */
if (clock->clkid == priv->master->clkid ||
(clock->phc_index >= 0 &&
clock->phc_index == priv->master->phc_index) ||
!strcmp(clock->device, priv->master->device))
continue;
which again, magically manages to not trip over a comparison between
the priv->master_phc_index (CLOCK_REALTIME, has phc_index == -1) and
clock->phc_index (/dev/ptp_kvm, also has phc_index == -1), simply
because the slave clock first has to have a valid phc_index before the
comparison is even been done.
Otherwise said, for phc2sys it is an actual regression to require that
the path to the PTP char device be in the "/dev/ptpN" format.
On the other hand, ts2phc has very different requirements, and it cannot
do anything useful with /dev/ptp_kvm or a PHC whose index it cannot
determine.
So what we do to fix the issue is remove the error code from
posix_clock_open() itself, and just error out in ts2phc, leaving phc2sys
to continue to work with phc_index == -1 for that clock.
The third caller of posix_clock_open(), phc_ctl, puts the returned
phc_index pointer into a "junk" variable, so technically this fixes a
regression which was introduces for that program, too.
Fixes: 380d023abb1f ("posix_clock_open: derive PHC index from device name if
possible")
Reported-by: Ed Branch <bra...@arlut.utexas.edu>
Signed-off-by: Vladimir Oltean <olte...@gmail.com>
---
Sorry for sending the email twice, last time the patch didn't make it to
the list, I got this reply back from
linuxptp-devel-ow...@lists.sourceforge.net: "In order to reduce spam on
the list, you must be subscribed in order to post new messages."
ts2phc.c | 6 ++++++
util.c | 6 ++----
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/ts2phc.c b/ts2phc.c
index f0b63baa6f8b..520ab36eb87c 100644
--- a/ts2phc.c
+++ b/ts2phc.c
@@ -194,6 +194,12 @@ struct clock *clock_add(struct ts2phc_private *priv, const
char *device)
if (clkid == CLOCK_INVALID)
return NULL;
+ if (phc_index < 0) {
+ pr_err("failed to retrieve PHC index for clock %s", device);
+ posix_clock_close(clkid);
+ return NULL;
+ }
+
LIST_FOREACH(c, &priv->clocks, list) {
if (c->phc_index == phc_index) {
/* Already have the clock, don't add it again */
diff --git a/util.c b/util.c
index cc4f11008be2..a11266689166 100644
--- a/util.c
+++ b/util.c
@@ -215,10 +215,8 @@ clockid_t posix_clock_open(const char *device, int
*phc_index)
int r = get_ranged_int(device + strlen("/dev/ptp"),
phc_index, 0, 65535);
if (r) {
- fprintf(stderr,
- "failed to parse PHC index from %s\n",
- device);
- return -1;
+ /* Don't completely error out on failure */
+ *phc_index = -1;
}
}
return clkid;
--
2.25.1
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
