Per https://www.kernel.org/doc/Documentation/networking/timestamping.txt section 3: "User space is responsible to ensure that multiple processes don't interfere with each other and that the settings are reset."
Add locking for the interface's HW timestamping mode to ensure that in a setup with multiple ptp4l sessions sharing interfaces the sessions don't overwrite each other's timestamping mode and that there is one session responsible for resetting the mode on exit. Since we do a SIOCGHWTSTAMP before SIOCSHWTSTAMP, the timestamping setup can be treated as a read-modify-write operation so the locking also protects the setup operation. The first instance that uses an interface writes its TX and RX timestamping modes after acquiring a filesystem lock on /var/run/ptp4l.<ifindex>.lock. Instances that start afterwards will fail to acquire an exclusive lock and will only check that the timestamping mode is set to the desired values after acquiring a shared lock. The last instance exiting will be able to acquire an exclusive lock again (all shared locks will have been released) and will reset the timestamping mode to no timestamps. Signed-off-by: Andrew Zaborowski <andrew.zaborow...@intel.com> --- Ideally on exit we'd restore the values we found when we started but since it may be a different instance doing the cleanup than the one that initialized the timestamping mode, we'd have to store the original values externally and it may not be worth the code complication. raw.c | 7 +++- sk.c | 118 ++++++++++++++++++++++++++++++++++++++++++++++++++++----- sk.h | 21 +++++++++- udp.c | 6 ++- udp6.c | 7 +++- 5 files changed, 145 insertions(+), 14 deletions(-) diff --git a/raw.c b/raw.c index a76fab6..09fff66 100644 --- a/raw.c +++ b/raw.c @@ -53,6 +53,7 @@ struct raw { struct address ptp_addr; struct address p2p_addr; int vlan; + struct sk_ts_lock_info ts_lock_info; }; #define PTP_GEN_BIT 0x08 /* indicates general message, if set in message type */ @@ -193,6 +194,9 @@ static int raw_configure(int fd, int event, int index, static int raw_close(struct transport *t, struct fdarray *fda) { + struct raw *raw = container_of(t, struct raw, t); + + sk_timestamping_exit(&raw->ts_lock_info); close(fda->fd[0]); close(fda->fd[1]); return 0; @@ -351,7 +355,8 @@ static int raw_open(struct transport *t, struct interface *iface, goto no_general; if (sk_timestamping_init(efd, name, ts_type, TRANS_IEEE_802_3, - interface_get_vclock(iface))) + interface_get_vclock(iface), + &raw->ts_lock_info)) goto no_timestamping; if (sk_general_init(gfd)) diff --git a/sk.c b/sk.c index 6a9e5b8..d64c3e7 100644 --- a/sk.c +++ b/sk.c @@ -26,10 +26,14 @@ #include <netinet/in.h> #include <string.h> #include <sys/ioctl.h> +#include <sys/types.h> +#include <sys/stat.h> +#include <sys/file.h> #include <unistd.h> #include <ifaddrs.h> #include <stdlib.h> #include <poll.h> +#include <fcntl.h> #include "address.h" #include "ether.h" @@ -56,8 +60,8 @@ static void init_ifreq(struct ifreq *ifreq, struct hwtstamp_config *cfg, ifreq->ifr_data = (void *) cfg; } -static int hwts_init(int fd, const char *device, int rx_filter, - int rx_filter2, int tx_type) +static int hwts_set(int fd, const char *device, enum hwts_filter_mode mode, + int rx_filter, int rx_filter2, int tx_type) { struct ifreq ifreq; struct hwtstamp_config cfg; @@ -71,7 +75,7 @@ static int hwts_init(int fd, const char *device, int rx_filter, if (ioctl(fd, SIOCGHWTSTAMP, &ifreq) == -EINVAL) init_ifreq(&ifreq, &cfg, device); - switch (sk_hwts_filter_mode) { + switch (mode) { case HWTS_FILTER_CHECK: err = ioctl(fd, SIOCGHWTSTAMP, &ifreq); if (err < 0) { @@ -542,10 +546,15 @@ int sk_set_priority(int fd, int family, uint8_t dscp) } int sk_timestamping_init(int fd, const char *device, enum timestamp_type type, - enum transport_type transport, int vclock) + enum transport_type transport, int vclock, + struct sk_ts_lock_info *lock_info) { - int err, filter1, filter2 = 0, flags, tx_type = HWTSTAMP_TX_ON; + int err = -1, filter1, filter2 = 0, flags, tx_type = HWTSTAMP_TX_ON; struct so_timestamping timestamping; + uint32_t index; + char *lock_name = NULL; + int lock_fd = -1; + enum hwts_filter_mode filter_mode = sk_hwts_filter_mode; switch (type) { case TS_SOFTWARE: @@ -600,9 +609,62 @@ int sk_timestamping_init(int fd, const char *device, enum timestamp_type type, case TRANS_UDS: return -1; } - err = hwts_init(fd, device, filter1, filter2, tx_type); - if (err) - return err; + + index = sk_interface_index(fd, device); + if (index < 0) { + return -1; + } + if (asprintf(&lock_name, "/var/run/ptp4l.%i.lock", index) == -1) { + return -1; + } + lock_fd = open(lock_name, O_RDWR | O_CREAT | O_CLOEXEC, 0600); + if (lock_fd == -1) { + pr_err("sk: failed to open(%s): %m", lock_name); + goto error; + } + if (flock(lock_fd, LOCK_EX | LOCK_NB) == 0) { + /* + * No one is using timestamping on this interface, + * set it up, then downgrade the lock to shared. + */ + err = hwts_set(fd, device, filter_mode, + filter1, filter2, tx_type); + if (err) + goto error; + else + err = -1; + + if (flock(lock_fd, LOCK_SH | LOCK_NB) != 0) { + pr_err("sk: failed to flock(%s): %m", + lock_name); + goto error; + } + } else { + if (errno != EWOULDBLOCK) { + pr_err("sk: failed to flock(%s): %m", + lock_name); + goto error; + } + + /* + * Another instance owns the timestamping mode on this + * interface, only check that it has configured the mode + * that we need after acquiring a shared lock (block if + * necessary.) + */ + if (flock(lock_fd, LOCK_SH) != 0) { + pr_err("sk: failed to flock(%s): %m", + lock_name); + goto error; + } + + err = hwts_set(fd, device, HWTS_FILTER_CHECK, + filter1, filter2, tx_type); + if (err) + goto error; + else + err = -1; + } } if (vclock >= 0) @@ -614,7 +676,7 @@ int sk_timestamping_init(int fd, const char *device, enum timestamp_type type, if (setsockopt(fd, SOL_SOCKET, SO_TIMESTAMPING, ×tamping, sizeof(timestamping)) < 0) { pr_err("ioctl SO_TIMESTAMPING failed: %m"); - return -1; + goto error; } flags = 1; @@ -627,8 +689,44 @@ int sk_timestamping_init(int fd, const char *device, enum timestamp_type type, /* Enable the sk_check_fupsync option, perhaps. */ if (sk_general_init(fd)) { - return -1; + goto error; + } + + if (lock_fd != -1) { + lock_info->fd = fd; + lock_info->device = strdup(device); + lock_info->lock_name = lock_name; + lock_info->lock_fd = lock_fd; } return 0; + +error: + free(lock_name); + if (lock_fd != -1) { + close(lock_fd); + } + + return err; +} + +void sk_timestamping_exit(struct sk_ts_lock_info *lock_info) +{ + if (!lock_info->device) + return; + + /* If no one else is using timestamping on the interface, clean it up */ + if (sk_hwts_filter_mode != HWTS_FILTER_CHECK && + flock(lock_info->lock_fd, LOCK_EX | LOCK_NB) == 0) { + hwts_set(lock_info->fd, lock_info->device, HWTS_FILTER_NORMAL, + HWTSTAMP_FILTER_NONE, HWTSTAMP_FILTER_NONE, + HWTSTAMP_TX_OFF); + } + + unlink(lock_info->lock_name); + free(lock_info->lock_name); + lock_info->lock_name = NULL; + close(lock_info->lock_fd); + free(lock_info->device); + lock_info->device = NULL; } diff --git a/sk.h b/sk.h index 76062d0..4b3725b 100644 --- a/sk.h +++ b/sk.h @@ -62,6 +62,17 @@ struct sk_if_info { uint64_t iface_bit_period; }; +/** + * Contains state needed to release the timestamping mode lock set by + * sk_timestamping_init(), if any. + */ +struct sk_ts_lock_info { + int fd; + char *device; + char *lock_name; + int lock_fd; +}; + /** * Obtains a socket suitable for use with sk_interface_index(). * @return An open socket on success, -1 otherwise. @@ -153,10 +164,18 @@ int sk_set_priority(int fd, int family, uint8_t dscp); * @param type The requested flavor of time stamping. * @param transport The type of transport used. * @param vclock Index of the virtual PHC, or -1 for the physical clock. + * @param info (output) Pointer to cleanup data for sk_timestamping_exit() * @return Zero on success, non-zero otherwise. */ int sk_timestamping_init(int fd, const char *device, enum timestamp_type type, - enum transport_type transport, int vclock); + enum transport_type transport, int vclock, + struct sk_ts_lock_info *lock_info); + +/** + * Reset an interface's timestamping mode after a sk_timestamping_init(). + * @param info Pointer to data saved by sk_timestamping_init() + */ +void sk_timestamping_exit(struct sk_ts_lock_info *lock_info); /** * Limits the time that RECVMSG(2) will poll while waiting for the tx timestamp diff --git a/udp.c b/udp.c index 7c9402e..58e7d44 100644 --- a/udp.c +++ b/udp.c @@ -46,6 +46,7 @@ struct udp { struct transport t; struct address ip; struct address mac; + struct sk_ts_lock_info ts_lock_info; }; static int mcast_bind(int fd, int index) @@ -85,6 +86,9 @@ static int mcast_join(int fd, int index, const struct sockaddr_in *sa) static int udp_close(struct transport *t, struct fdarray *fda) { + struct udp *udp = container_of(t, struct udp, t); + + sk_timestamping_exit(&udp->ts_lock_info); close(fda->fd[0]); close(fda->fd[1]); return 0; @@ -180,7 +184,7 @@ static int udp_open(struct transport *t, struct interface *iface, goto no_general; if (sk_timestamping_init(efd, interface_label(iface), ts_type, TRANS_UDP_IPV4, - interface_get_vclock(iface))) + interface_get_vclock(iface), &udp->ts_lock_info)) goto no_timestamping; if (sk_general_init(gfd)) diff --git a/udp6.c b/udp6.c index bde1710..1b6a61e 100644 --- a/udp6.c +++ b/udp6.c @@ -52,6 +52,7 @@ struct udp6 { struct address ip; struct address mac; struct in6_addr mc6_addr[2]; + struct sk_ts_lock_info ts_lock_info; }; static int is_link_local(struct in6_addr *addr) @@ -96,6 +97,9 @@ static int mc_join(int fd, int index, const struct sockaddr_in6 *sa) static int udp6_close(struct transport *t, struct fdarray *fda) { + struct udp6 *udp6 = container_of(t, struct udp6, t); + + sk_timestamping_exit(&udp6->ts_lock_info); close(fda->fd[0]); close(fda->fd[1]); return 0; @@ -197,7 +201,8 @@ static int udp6_open(struct transport *t, struct interface *iface, goto no_general; if (sk_timestamping_init(efd, interface_label(iface), ts_type, - TRANS_UDP_IPV6, interface_get_vclock(iface))) + TRANS_UDP_IPV6, interface_get_vclock(iface), + &udp6->ts_lock_info)) goto no_timestamping; if (sk_general_init(gfd)) -- 2.34.1 _______________________________________________ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel