diff mbox series

[net-next] net: fix SOF_TIMESTAMPING_BIND_PHC to work with multiple sockets

Message ID 20220105103326.3130875-1-mlichvar@redhat.com (mailing list archive)
State Accepted
Commit 007747a984ea5e895b7d8b056b24ebf431e1e71d
Delegated to: Netdev Maintainers
Headers show
Series [net-next] net: fix SOF_TIMESTAMPING_BIND_PHC to work with multiple sockets | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Single patches do not need cover letters
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 275 this patch: 275
netdev/cc_maintainers fail 1 blamed authors not CCed: davem@davemloft.net; 2 maintainers not CCed: davem@davemloft.net kuba@kernel.org
netdev/build_clang success Errors and warnings before: 150 this patch: 150
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 309 this patch: 309
netdev/checkpatch warning WARNING: line length of 89 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 5 this patch: 5
netdev/source_inline success Was 0 now: 0

Commit Message

Miroslav Lichvar Jan. 5, 2022, 10:33 a.m. UTC 
When multiple sockets using the SOF_TIMESTAMPING_BIND_PHC flag received
a packet with a hardware timestamp (e.g. multiple PTP instances in
different PTP domains using the UDPv4/v6 multicast or L2 transport),
the timestamps received on some sockets were corrupted due to repeated
conversion of the same timestamp (by the same or different vclocks).

Fix ptp_convert_timestamp() to not modify the shared skb timestamp
and return the converted timestamp as a ktime_t instead. If the
conversion fails, return 0 to not confuse the application with
timestamps corresponding to an unexpected PHC.

Fixes: d7c088265588 ("net: socket: support hardware timestamp conversion to PHC bound")
Signed-off-by: Miroslav Lichvar <mlichvar@redhat.com>
Cc: Yangbo Lu <yangbo.lu@nxp.com>
Cc: Richard Cochran <richardcochran@gmail.com>
---
 drivers/ptp/ptp_vclock.c         | 10 +++++-----
 include/linux/ptp_clock_kernel.h | 12 +++++++-----
 net/socket.c                     |  9 ++++++---
 3 files changed, 18 insertions(+), 13 deletions(-)

Comments

Richard Cochran Jan. 5, 2022, 8:10 p.m. UTC | #1 
On Wed, Jan 05, 2022 at 11:33:26AM +0100, Miroslav Lichvar wrote:
> When multiple sockets using the SOF_TIMESTAMPING_BIND_PHC flag received
> a packet with a hardware timestamp (e.g. multiple PTP instances in
> different PTP domains using the UDPv4/v6 multicast or L2 transport),
> the timestamps received on some sockets were corrupted due to repeated
> conversion of the same timestamp (by the same or different vclocks).
> 
> Fix ptp_convert_timestamp() to not modify the shared skb timestamp
> and return the converted timestamp as a ktime_t instead. If the
> conversion fails, return 0 to not confuse the application with
> timestamps corresponding to an unexpected PHC.

Acked-by: Richard Cochran <richardcochran@gmail.com>
patchwork-bot+netdevbpf@kernel.org Jan. 6, 2022, 12:40 p.m. UTC | #2 
Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Wed,  5 Jan 2022 11:33:26 +0100 you wrote:
> When multiple sockets using the SOF_TIMESTAMPING_BIND_PHC flag received
> a packet with a hardware timestamp (e.g. multiple PTP instances in
> different PTP domains using the UDPv4/v6 multicast or L2 transport),
> the timestamps received on some sockets were corrupted due to repeated
> conversion of the same timestamp (by the same or different vclocks).
> 
> Fix ptp_convert_timestamp() to not modify the shared skb timestamp
> and return the converted timestamp as a ktime_t instead. If the
> conversion fails, return 0 to not confuse the application with
> timestamps corresponding to an unexpected PHC.
> 
> [...]

Here is the summary with links:
  - [net-next] net: fix SOF_TIMESTAMPING_BIND_PHC to work with multiple sockets
    https://git.kernel.org/netdev/net-next/c/007747a984ea

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/ptp/ptp_vclock.c b/drivers/ptp/ptp_vclock.c
index baee0379482b..ab1d233173e1 100644
--- a/drivers/ptp/ptp_vclock.c
+++ b/drivers/ptp/ptp_vclock.c
@@ -185,8 +185,8 @@  int ptp_get_vclocks_index(int pclock_index, int **vclock_index)
 }
 EXPORT_SYMBOL(ptp_get_vclocks_index);
 
-void ptp_convert_timestamp(struct skb_shared_hwtstamps *hwtstamps,
-			   int vclock_index)
+ktime_t ptp_convert_timestamp(const struct skb_shared_hwtstamps *hwtstamps,
+			      int vclock_index)
 {
 	char name[PTP_CLOCK_NAME_LEN] = "";
 	struct ptp_vclock *vclock;
@@ -198,12 +198,12 @@  void ptp_convert_timestamp(struct skb_shared_hwtstamps *hwtstamps,
 	snprintf(name, PTP_CLOCK_NAME_LEN, "ptp%d", vclock_index);
 	dev = class_find_device_by_name(ptp_class, name);
 	if (!dev)
-		return;
+		return 0;
 
 	ptp = dev_get_drvdata(dev);
 	if (!ptp->is_virtual_clock) {
 		put_device(dev);
-		return;
+		return 0;
 	}
 
 	vclock = info_to_vclock(ptp->info);
@@ -215,7 +215,7 @@  void ptp_convert_timestamp(struct skb_shared_hwtstamps *hwtstamps,
 	spin_unlock_irqrestore(&vclock->lock, flags);
 
 	put_device(dev);
-	hwtstamps->hwtstamp = ns_to_ktime(ns);
+	return ns_to_ktime(ns);
 }
 EXPORT_SYMBOL(ptp_convert_timestamp);
 #endif
diff --git a/include/linux/ptp_clock_kernel.h b/include/linux/ptp_clock_kernel.h
index 2e5565067355..554454cb8693 100644
--- a/include/linux/ptp_clock_kernel.h
+++ b/include/linux/ptp_clock_kernel.h
@@ -351,15 +351,17 @@  int ptp_get_vclocks_index(int pclock_index, int **vclock_index);
  *
  * @hwtstamps:    skb_shared_hwtstamps structure pointer
  * @vclock_index: phc index of ptp vclock.
+ *
+ * Returns converted timestamp, or 0 on error.
  */
-void ptp_convert_timestamp(struct skb_shared_hwtstamps *hwtstamps,
-			   int vclock_index);
+ktime_t ptp_convert_timestamp(const struct skb_shared_hwtstamps *hwtstamps,
+			      int vclock_index);
 #else
 static inline int ptp_get_vclocks_index(int pclock_index, int **vclock_index)
 { return 0; }
-static inline void ptp_convert_timestamp(struct skb_shared_hwtstamps *hwtstamps,
-					 int vclock_index)
-{ }
+static inline ktime_t ptp_convert_timestamp(const struct skb_shared_hwtstamps *hwtstamps,
+					    int vclock_index)
+{ return 0; }
 
 #endif
 
diff --git a/net/socket.c b/net/socket.c
index 7f64a6eccf63..5053eb0100e4 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -829,6 +829,7 @@  void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
 	int empty = 1, false_tstamp = 0;
 	struct skb_shared_hwtstamps *shhwtstamps =
 		skb_hwtstamps(skb);
+	ktime_t hwtstamp;
 
 	/* Race occurred between timestamp enabling and packet
 	   receiving.  Fill in the current time for now. */
@@ -877,10 +878,12 @@  void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
 	    (sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE) &&
 	    !skb_is_swtx_tstamp(skb, false_tstamp)) {
 		if (sk->sk_tsflags & SOF_TIMESTAMPING_BIND_PHC)
-			ptp_convert_timestamp(shhwtstamps, sk->sk_bind_phc);
+			hwtstamp = ptp_convert_timestamp(shhwtstamps,
+							 sk->sk_bind_phc);
+		else
+			hwtstamp = shhwtstamps->hwtstamp;
 
-		if (ktime_to_timespec64_cond(shhwtstamps->hwtstamp,
-					     tss.ts + 2)) {
+		if (ktime_to_timespec64_cond(hwtstamp, tss.ts + 2)) {
 			empty = 0;
 
 			if ((sk->sk_tsflags & SOF_TIMESTAMPING_OPT_PKTINFO) &&