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. It
doesn't strictly require having a PHC index, but leaving the data
structures like that, now that we are aware of this corner case, is
prone to breakage introduced by future changes.
To fix the problem, we maintain that the PTP clock char device must
contain a number, if its path begins with /dev/ptp. But this time, we
follow any symbolic links that might exist, before attempting to deduce
the number from the path string.
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>
---
util.c | 21 +++++++++++++++------
1 file changed, 15 insertions(+), 6 deletions(-)
diff --git a/util.c b/util.c
index cc4f11008be2..686ed5e2f110 100644
--- a/util.c
+++ b/util.c
@@ -18,6 +18,7 @@
*/
#include <arpa/inet.h>
#include <errno.h>
+#include <linux/limits.h>
#include <signal.h>
#include <stdarg.h>
#include <stdio.h>
@@ -200,6 +201,7 @@ void posix_clock_close(clockid_t clock)
clockid_t posix_clock_open(const char *device, int *phc_index)
{
+ char phc_device_path[PATH_MAX];
struct sk_ts_info ts_info;
char phc_device[19];
int clkid;
@@ -208,21 +210,28 @@ clockid_t posix_clock_open(const char *device, int
*phc_index)
if (!strcasecmp(device, "CLOCK_REALTIME")) {
return CLOCK_REALTIME;
}
- /* check if device is valid phc device */
- clkid = phc_open(device);
- if (clkid != CLOCK_INVALID) {
- if (!strncmp(device, "/dev/ptp", strlen("/dev/ptp"))) {
- int r = get_ranged_int(device + strlen("/dev/ptp"),
+
+ /* if the device name resolves so a plausible filesystem path, we
+ * assume it is the path to a PHC char device, and treat it as such
+ */
+ if (realpath(device, phc_device_path)) {
+ clkid = phc_open(device);
+ if (clkid == CLOCK_INVALID)
+ return clkid;
+
+ if (!strncmp(phc_device_path, "/dev/ptp", strlen("/dev/ptp"))) {
+ int r = get_ranged_int(phc_device_path +
strlen("/dev/ptp"),
phc_index, 0, 65535);
if (r) {
fprintf(stderr,
"failed to parse PHC index from %s\n",
- device);
+ phc_device_path);
return -1;
}
}
return clkid;
}
+
/* check if device is a valid ethernet device */
if (sk_get_ts_info(device, &ts_info) || !ts_info.valid) {
pr_err("unknown clock %s: %m", device);
--
2.25.1
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
