netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v3 0/5] net-timestamp: additional sw tstamps and bytestream support
@ 2014-07-21 19:35 Willem de Bruijn
  2014-07-21 19:35 ` [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct Willem de Bruijn
                   ` (4 more replies)
  0 siblings, 5 replies; 15+ messages in thread
From: Willem de Bruijn @ 2014-07-21 19:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, richardcochran, Willem de Bruijn

Extend socket tx timestamping:
- allow multiple types of software timestamps aside from send (1)
- add software timestamp on tc enqueue (3)
- add software timestamp for TCP (4)
- add software timestamp for TCP on ACK (5)

The sk_flags option space is nearly exhausted. Also move the
many timestamp options to a new sk->sk_tstamps (2).

The patchset extends Linux tx timestamping to monitoring of latency
incurred within the kernel stack and to protocols embedded in TCP.
Complex kernel setups may have multiple layers of queueing, including
multiple instances of traffic shaping, and many classes per layer.
Many applications embed discrete payloads into TCP bytestreams for
reliability, flow control, etcetera. Detecting application tail
latency in such scenarios relies on identifying the exact queue
responsible if on the host, or the network latency if otherwise.

Changelog:
v2->v3
  - extend the SO_TIMESTAMPING API, instead of defining a new one.
  - add protocol independent support to correlate tstamps with data,
    based on returning skb->mark.
  - removed no-payload optimization and documentation (for now):

    I have a follow-on patch that reintroduces MSG_TSTAMP along with a
    new socket option SOF_TIMESTAMPING_OPT_ONFLAG. This is equivalent
    to sequence setsockopt(<enable>); send(..); setsockopt(<disable>),
    but avoids the need to define a MSG_TSTAMP_<TYPE> for each type.

    I will leave these three patches as follow-on, as this patchset is
    large enough as is.

v1->2
  - expand timestamping (existing and new) to SOCK_RAW and ping sockets
  - rename sock_errqueue_timestamping to scm_timestamping
  - change timestamp data format: do not add fields to scm_timestamping.
      Doing so could break legacy applications. Instead, communicate
      through an existing, but unused, field in the error message.
  - rename SOF_.._OPT_TX_NO_PAYLOAD to shorter SOF_.._OPT_TSONLY
  - move msg_tstamp test app out of patchset and to github
      git://github.com/wdebruij/kerneltools.git

-- 
2.0.0.526.g5318336

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct
  2014-07-21 19:35 [PATCH net-next v3 0/5] net-timestamp: additional sw tstamps and bytestream support Willem de Bruijn
@ 2014-07-21 19:35 ` Willem de Bruijn
  2014-07-21 23:24   ` David Miller
  2014-07-21 19:35 ` [PATCH net-next v3 2/5] net-timestamp: move timestamp flags out of sk_flags Willem de Bruijn
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 15+ messages in thread
From: Willem de Bruijn @ 2014-07-21 19:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, richardcochran, Willem de Bruijn

Applications that request kernel tx timestamps with SO_TIMESTAMPING
read timestamps as recvmsg() ancillary data. The response is defined
implicitly as timespec[3]. Define the struct, and

1) Add support for new tstamp types. On tx, scm_timestamping always
accompanies a sock_extended_err. Define previously unused field
ee_info to signal the type of ts[0]. Introduce SCM_TSTAMP_SND.

2) Add support to discern timestamps. When multiple timestamped
packets are in flight concurrently, correlating a timestamp with
the right send() call is non-trivial. Define previously unused field
ee_data to communicate a send() specific key, skb->mark. It is
up to the application to optionally set this field to a unique
value for each send call.

The reception path is not modified. On rx, no struct similar to
sock_extended_err is passed along with SCM_TIMESTAMPING.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/skbuff.h        |  3 +++
 include/net/sock.h            |  3 ++-
 include/uapi/linux/errqueue.h | 15 +++++++++++++++
 net/core/skbuff.c             |  2 ++
 net/socket.c                  | 22 ++++++++++++----------
 5 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 3694303..e71224a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -262,6 +262,9 @@ enum {
 	SKBTX_SHARED_FRAG = 1 << 5,
 };
 
+#define SKBTX_ANY_SW_TSTAMP	SKBTX_SW_TSTAMP
+#define SKBTX_ANY_TSTAMP	(SKBTX_HW_TSTAMP | SKBTX_ANY_SW_TSTAMP)
+
 /*
  * The callback notifies userspace to release buffers when skb DMA is done in
  * lower device, the skb last reference should be 0 when calling this.
diff --git a/include/net/sock.h b/include/net/sock.h
index 28f7346..00abc4c 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2172,7 +2172,8 @@ sock_recv_timestamp(struct msghdr *msg, struct sock *sk, struct sk_buff *skb)
 	 */
 	if (sock_flag(sk, SOCK_RCVTSTAMP) ||
 	    sock_flag(sk, SOCK_TIMESTAMPING_RX_SOFTWARE) ||
-	    (kt.tv64 && sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE)) ||
+	    (kt.tv64 && (sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE) ||
+	     skb_shinfo(skb)->tx_flags & SKBTX_ANY_SW_TSTAMP)) ||
 	    (hwtstamps->hwtstamp.tv64 &&
 	     sock_flag(sk, SOCK_TIMESTAMPING_RAW_HARDWARE)) ||
 	    (hwtstamps->syststamp.tv64 &&
diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
index aacd4fb..97e1872 100644
--- a/include/uapi/linux/errqueue.h
+++ b/include/uapi/linux/errqueue.h
@@ -22,5 +22,20 @@ struct sock_extended_err {
 
 #define SO_EE_OFFENDER(ee)	((struct sockaddr*)((ee)+1))
 
+/**
+ *	struct scm_timestamping - timestamps exposed through cmsg
+ *
+ *	The timestamping interfaces SO_TIMESTAMPING, MSG_TSTAMP_*
+ *	communicate network timestamps by passing this struct in a cmsg with
+ *	recvmsg(). See Documentation/networking/timestamping.txt for details.
+ */
+struct scm_timestamping {
+	struct timespec ts[3];
+};
+
+/* type of ts[0], passed in ee_info */
+enum {
+	SCM_TSTAMP_SND = 1,	/* driver passed skb to NIC */
+};
 
 #endif /* _UAPI_LINUX_ERRQUEUE_H */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index c1a3303..d95b09d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3521,6 +3521,8 @@ void skb_tstamp_tx(struct sk_buff *orig_skb,
 	memset(serr, 0, sizeof(*serr));
 	serr->ee.ee_errno = ENOMSG;
 	serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING;
+	serr->ee.ee_info = hwtstamps ? 0 : SCM_TSTAMP_SND;
+	serr->ee.ee_data = skb->mark;
 
 	err = sock_queue_err_skb(sk, skb);
 
diff --git a/net/socket.c b/net/socket.c
index abf56b2..7fc2eef 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -106,6 +106,7 @@
 #include <linux/sockios.h>
 #include <linux/atalk.h>
 #include <net/busy_poll.h>
+#include <linux/errqueue.h>
 
 #ifdef CONFIG_NET_RX_BUSY_POLL
 unsigned int sysctl_net_busy_read __read_mostly;
@@ -697,7 +698,7 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
 	struct sk_buff *skb)
 {
 	int need_software_tstamp = sock_flag(sk, SOCK_RCVTSTAMP);
-	struct timespec ts[3];
+	struct scm_timestamping tss;
 	int empty = 1;
 	struct skb_shared_hwtstamps *shhwtstamps =
 		skb_hwtstamps(skb);
@@ -714,28 +715,29 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
 			put_cmsg(msg, SOL_SOCKET, SCM_TIMESTAMP,
 				 sizeof(tv), &tv);
 		} else {
-			skb_get_timestampns(skb, &ts[0]);
+			struct timespec ts;
+			skb_get_timestampns(skb, &ts);
 			put_cmsg(msg, SOL_SOCKET, SCM_TIMESTAMPNS,
-				 sizeof(ts[0]), &ts[0]);
+				 sizeof(ts), &ts);
 		}
 	}
 
-
-	memset(ts, 0, sizeof(ts));
-	if (sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE) &&
-	    ktime_to_timespec_cond(skb->tstamp, ts + 0))
+	memset(&tss, 0, sizeof(tss));
+	if ((sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE) ||
+	     skb_shinfo(skb)->tx_flags & SKBTX_ANY_SW_TSTAMP) &&
+	    ktime_to_timespec_cond(skb->tstamp, tss.ts + 0))
 		empty = 0;
 	if (shhwtstamps) {
 		if (sock_flag(sk, SOCK_TIMESTAMPING_SYS_HARDWARE) &&
-		    ktime_to_timespec_cond(shhwtstamps->syststamp, ts + 1))
+		    ktime_to_timespec_cond(shhwtstamps->syststamp, tss.ts + 1))
 			empty = 0;
 		if (sock_flag(sk, SOCK_TIMESTAMPING_RAW_HARDWARE) &&
-		    ktime_to_timespec_cond(shhwtstamps->hwtstamp, ts + 2))
+		    ktime_to_timespec_cond(shhwtstamps->hwtstamp, tss.ts + 2))
 			empty = 0;
 	}
 	if (!empty)
 		put_cmsg(msg, SOL_SOCKET,
-			 SCM_TIMESTAMPING, sizeof(ts), &ts);
+			 SCM_TIMESTAMPING, sizeof(tss), &tss);
 }
 EXPORT_SYMBOL_GPL(__sock_recv_timestamp);
 
-- 
2.0.0.526.g5318336

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH net-next v3 2/5] net-timestamp: move timestamp flags out of sk_flags
  2014-07-21 19:35 [PATCH net-next v3 0/5] net-timestamp: additional sw tstamps and bytestream support Willem de Bruijn
  2014-07-21 19:35 ` [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct Willem de Bruijn
@ 2014-07-21 19:35 ` Willem de Bruijn
  2014-07-21 19:35 ` [PATCH net-next v3 3/5] net-timestamp: ENQ timestamp on enqueue to traffic shaping layer Willem de Bruijn
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 15+ messages in thread
From: Willem de Bruijn @ 2014-07-21 19:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, richardcochran, Willem de Bruijn

sk_flags is reaching its limit. New timestamping options will not fit.
Move all of them into a new field sk->sk_tsflags.

Added benefit is that this removes boilerplate code to convert between
SOF_TIMESTAMPING_.. and SOCK_TIMESTAMPING_.. in getsockopt/setsockopt.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---

Keeping SOCK_TIMESTAMPING_RX_SOFTWARE for now. It is used in receive
tstamp static key logic. It can be removed and that logic simplified.
Will do that in a separate follow up.
---
 include/net/sock.h | 34 ++++++++++++----------------------
 net/core/sock.c    | 29 ++---------------------------
 net/socket.c       | 10 +++++-----
 3 files changed, 19 insertions(+), 54 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 00abc4c..cf5e75b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -67,6 +67,7 @@
 #include <linux/atomic.h>
 #include <net/dst.h>
 #include <net/checksum.h>
+#include <linux/net_tstamp.h>
 
 struct cgroup;
 struct cgroup_subsys;
@@ -411,6 +412,7 @@ struct sock {
 	void			*sk_protinfo;
 	struct timer_list	sk_timer;
 	ktime_t			sk_stamp;
+	u16			sk_tsflags;
 	struct socket		*sk_socket;
 	void			*sk_user_data;
 	struct page_frag	sk_frag;
@@ -701,13 +703,7 @@ enum sock_flags {
 	SOCK_LOCALROUTE, /* route locally only, %SO_DONTROUTE setting */
 	SOCK_QUEUE_SHRUNK, /* write queue has been shrunk recently */
 	SOCK_MEMALLOC, /* VM depends on this socket for swapping */
-	SOCK_TIMESTAMPING_TX_HARDWARE,  /* %SOF_TIMESTAMPING_TX_HARDWARE */
-	SOCK_TIMESTAMPING_TX_SOFTWARE,  /* %SOF_TIMESTAMPING_TX_SOFTWARE */
-	SOCK_TIMESTAMPING_RX_HARDWARE,  /* %SOF_TIMESTAMPING_RX_HARDWARE */
 	SOCK_TIMESTAMPING_RX_SOFTWARE,  /* %SOF_TIMESTAMPING_RX_SOFTWARE */
-	SOCK_TIMESTAMPING_SOFTWARE,     /* %SOF_TIMESTAMPING_SOFTWARE */
-	SOCK_TIMESTAMPING_RAW_HARDWARE, /* %SOF_TIMESTAMPING_RAW_HARDWARE */
-	SOCK_TIMESTAMPING_SYS_HARDWARE, /* %SOF_TIMESTAMPING_SYS_HARDWARE */
 	SOCK_FASYNC, /* fasync() active */
 	SOCK_RXQ_OVFL,
 	SOCK_ZEROCOPY, /* buffers from userspace */
@@ -2162,22 +2158,18 @@ sock_recv_timestamp(struct msghdr *msg, struct sock *sk, struct sk_buff *skb)
 
 	/*
 	 * generate control messages if
-	 * - receive time stamping in software requested (SOCK_RCVTSTAMP
-	 *   or SOCK_TIMESTAMPING_RX_SOFTWARE)
+	 * - receive time stamping in software requested
 	 * - software time stamp available and wanted
-	 *   (SOCK_TIMESTAMPING_SOFTWARE)
 	 * - hardware time stamps available and wanted
-	 *   (SOCK_TIMESTAMPING_SYS_HARDWARE or
-	 *   SOCK_TIMESTAMPING_RAW_HARDWARE)
 	 */
 	if (sock_flag(sk, SOCK_RCVTSTAMP) ||
-	    sock_flag(sk, SOCK_TIMESTAMPING_RX_SOFTWARE) ||
-	    (kt.tv64 && (sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE) ||
+	    (sk->sk_tsflags & SOF_TIMESTAMPING_RX_SOFTWARE) ||
+	    (kt.tv64 && (sk->sk_tsflags & SOF_TIMESTAMPING_SOFTWARE ||
 	     skb_shinfo(skb)->tx_flags & SKBTX_ANY_SW_TSTAMP)) ||
 	    (hwtstamps->hwtstamp.tv64 &&
-	     sock_flag(sk, SOCK_TIMESTAMPING_RAW_HARDWARE)) ||
+	     (sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE)) ||
 	    (hwtstamps->syststamp.tv64 &&
-	     sock_flag(sk, SOCK_TIMESTAMPING_SYS_HARDWARE)))
+	     (sk->sk_tsflags & SOF_TIMESTAMPING_SYS_HARDWARE)))
 		__sock_recv_timestamp(msg, sk, skb);
 	else
 		sk->sk_stamp = kt;
@@ -2193,12 +2185,12 @@ static inline void sock_recv_ts_and_drops(struct msghdr *msg, struct sock *sk,
 					  struct sk_buff *skb)
 {
 #define FLAGS_TS_OR_DROPS ((1UL << SOCK_RXQ_OVFL)			| \
-			   (1UL << SOCK_RCVTSTAMP)			| \
-			   (1UL << SOCK_TIMESTAMPING_SOFTWARE)		| \
-			   (1UL << SOCK_TIMESTAMPING_RAW_HARDWARE)	| \
-			   (1UL << SOCK_TIMESTAMPING_SYS_HARDWARE))
+			   (1UL << SOCK_RCVTSTAMP))
+#define TSFLAGS_ANY	  (SOF_TIMESTAMPING_SOFTWARE			| \
+			   SOF_TIMESTAMPING_RAW_HARDWARE		| \
+			   SOF_TIMESTAMPING_SYS_HARDWARE)
 
-	if (sk->sk_flags & FLAGS_TS_OR_DROPS)
+	if (sk->sk_flags & FLAGS_TS_OR_DROPS || sk->sk_tsflags & TSFLAGS_ANY)
 		__sock_recv_ts_and_drops(msg, sk, skb);
 	else
 		sk->sk_stamp = skb->tstamp;
@@ -2208,8 +2200,6 @@ static inline void sock_recv_ts_and_drops(struct msghdr *msg, struct sock *sk,
  * sock_tx_timestamp - checks whether the outgoing packet is to be time stamped
  * @sk:		socket sending this packet
  * @tx_flags:	filled with instructions for time stamping
- *
- * Currently only depends on SOCK_TIMESTAMPING* flags.
  */
 void sock_tx_timestamp(struct sock *sk, __u8 *tx_flags);
 
diff --git a/net/core/sock.c b/net/core/sock.c
index 026e01f..bdde15e 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -848,24 +848,13 @@ set_rcvbuf:
 			ret = -EINVAL;
 			break;
 		}
-		sock_valbool_flag(sk, SOCK_TIMESTAMPING_TX_HARDWARE,
-				  val & SOF_TIMESTAMPING_TX_HARDWARE);
-		sock_valbool_flag(sk, SOCK_TIMESTAMPING_TX_SOFTWARE,
-				  val & SOF_TIMESTAMPING_TX_SOFTWARE);
-		sock_valbool_flag(sk, SOCK_TIMESTAMPING_RX_HARDWARE,
-				  val & SOF_TIMESTAMPING_RX_HARDWARE);
+		sk->sk_tsflags = val;
 		if (val & SOF_TIMESTAMPING_RX_SOFTWARE)
 			sock_enable_timestamp(sk,
 					      SOCK_TIMESTAMPING_RX_SOFTWARE);
 		else
 			sock_disable_timestamp(sk,
 					       (1UL << SOCK_TIMESTAMPING_RX_SOFTWARE));
-		sock_valbool_flag(sk, SOCK_TIMESTAMPING_SOFTWARE,
-				  val & SOF_TIMESTAMPING_SOFTWARE);
-		sock_valbool_flag(sk, SOCK_TIMESTAMPING_SYS_HARDWARE,
-				  val & SOF_TIMESTAMPING_SYS_HARDWARE);
-		sock_valbool_flag(sk, SOCK_TIMESTAMPING_RAW_HARDWARE,
-				  val & SOF_TIMESTAMPING_RAW_HARDWARE);
 		break;
 
 	case SO_RCVLOWAT:
@@ -1091,21 +1080,7 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		break;
 
 	case SO_TIMESTAMPING:
-		v.val = 0;
-		if (sock_flag(sk, SOCK_TIMESTAMPING_TX_HARDWARE))
-			v.val |= SOF_TIMESTAMPING_TX_HARDWARE;
-		if (sock_flag(sk, SOCK_TIMESTAMPING_TX_SOFTWARE))
-			v.val |= SOF_TIMESTAMPING_TX_SOFTWARE;
-		if (sock_flag(sk, SOCK_TIMESTAMPING_RX_HARDWARE))
-			v.val |= SOF_TIMESTAMPING_RX_HARDWARE;
-		if (sock_flag(sk, SOCK_TIMESTAMPING_RX_SOFTWARE))
-			v.val |= SOF_TIMESTAMPING_RX_SOFTWARE;
-		if (sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE))
-			v.val |= SOF_TIMESTAMPING_SOFTWARE;
-		if (sock_flag(sk, SOCK_TIMESTAMPING_SYS_HARDWARE))
-			v.val |= SOF_TIMESTAMPING_SYS_HARDWARE;
-		if (sock_flag(sk, SOCK_TIMESTAMPING_RAW_HARDWARE))
-			v.val |= SOF_TIMESTAMPING_RAW_HARDWARE;
+		v.val = sk->sk_tsflags;
 		break;
 
 	case SO_RCVTIMEO:
diff --git a/net/socket.c b/net/socket.c
index 7fc2eef..6c5c913 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -613,9 +613,9 @@ EXPORT_SYMBOL(sock_release);
 void sock_tx_timestamp(struct sock *sk, __u8 *tx_flags)
 {
 	*tx_flags = 0;
-	if (sock_flag(sk, SOCK_TIMESTAMPING_TX_HARDWARE))
+	if (sk->sk_tsflags & SOF_TIMESTAMPING_TX_HARDWARE)
 		*tx_flags |= SKBTX_HW_TSTAMP;
-	if (sock_flag(sk, SOCK_TIMESTAMPING_TX_SOFTWARE))
+	if (sk->sk_tsflags & SOF_TIMESTAMPING_TX_SOFTWARE)
 		*tx_flags |= SKBTX_SW_TSTAMP;
 	if (sock_flag(sk, SOCK_WIFI_STATUS))
 		*tx_flags |= SKBTX_WIFI_STATUS;
@@ -723,15 +723,15 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
 	}
 
 	memset(&tss, 0, sizeof(tss));
-	if ((sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE) ||
+	if ((sk->sk_tsflags & SOF_TIMESTAMPING_SOFTWARE ||
 	     skb_shinfo(skb)->tx_flags & SKBTX_ANY_SW_TSTAMP) &&
 	    ktime_to_timespec_cond(skb->tstamp, tss.ts + 0))
 		empty = 0;
 	if (shhwtstamps) {
-		if (sock_flag(sk, SOCK_TIMESTAMPING_SYS_HARDWARE) &&
+		if (sk->sk_tsflags & SOF_TIMESTAMPING_SYS_HARDWARE &&
 		    ktime_to_timespec_cond(shhwtstamps->syststamp, tss.ts + 1))
 			empty = 0;
-		if (sock_flag(sk, SOCK_TIMESTAMPING_RAW_HARDWARE) &&
+		if (sk->sk_tsflags & SOF_TIMESTAMPING_RAW_HARDWARE &&
 		    ktime_to_timespec_cond(shhwtstamps->hwtstamp, tss.ts + 2))
 			empty = 0;
 	}
-- 
2.0.0.526.g5318336

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH net-next v3 3/5] net-timestamp: ENQ timestamp on enqueue to traffic shaping layer
  2014-07-21 19:35 [PATCH net-next v3 0/5] net-timestamp: additional sw tstamps and bytestream support Willem de Bruijn
  2014-07-21 19:35 ` [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct Willem de Bruijn
  2014-07-21 19:35 ` [PATCH net-next v3 2/5] net-timestamp: move timestamp flags out of sk_flags Willem de Bruijn
@ 2014-07-21 19:35 ` Willem de Bruijn
  2014-07-21 19:35 ` [PATCH net-next v3 4/5] net-timestamp: TCP timestamping Willem de Bruijn
  2014-07-21 19:35 ` [PATCH net-next v3 5/5] net-timestamp: ACK timestamp for bytestreams Willem de Bruijn
  4 siblings, 0 replies; 15+ messages in thread
From: Willem de Bruijn @ 2014-07-21 19:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, richardcochran, Willem de Bruijn

Kernel transmit latency is often incurred in the traffic shaping
layer. This patch adds a new timestamp on transmission just before
entering traffic shaping. When data travels through multiple devices
(bonding, tunneling, ...) each device will export an individual
timestamp.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/skbuff.h          | 11 +++++++++--
 include/uapi/linux/errqueue.h   |  1 +
 include/uapi/linux/net_tstamp.h |  8 +++++---
 net/core/dev.c                  |  4 ++++
 net/core/skbuff.c               | 16 ++++++++++++----
 net/socket.c                    |  3 +++
 6 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index e71224a..8c98dc9 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -242,7 +242,7 @@ enum {
 	/* generate hardware time stamp */
 	SKBTX_HW_TSTAMP = 1 << 0,
 
-	/* generate software time stamp */
+	/* generate software time stamp when queueing packet to NIC */
 	SKBTX_SW_TSTAMP = 1 << 1,
 
 	/* device driver is going to provide hardware time stamp */
@@ -260,9 +260,12 @@ enum {
 	 * all frags to avoid possible bad checksum
 	 */
 	SKBTX_SHARED_FRAG = 1 << 5,
+
+	/* generate software time stamp when queueing packing in TC */
+	SKBTX_ENQ_TSTAMP = 1 << 6,
 };
 
-#define SKBTX_ANY_SW_TSTAMP	SKBTX_SW_TSTAMP
+#define SKBTX_ANY_SW_TSTAMP	(SKBTX_SW_TSTAMP | SKBTX_ENQ_TSTAMP)
 #define SKBTX_ANY_TSTAMP	(SKBTX_HW_TSTAMP | SKBTX_ANY_SW_TSTAMP)
 
 /*
@@ -2707,6 +2710,10 @@ static inline bool skb_defer_rx_timestamp(struct sk_buff *skb)
 void skb_complete_tx_timestamp(struct sk_buff *skb,
 			       struct skb_shared_hwtstamps *hwtstamps);
 
+void __skb_tstamp_tx(struct sk_buff *orig_skb,
+		     struct skb_shared_hwtstamps *hwtstamps,
+		     struct sock *sk, int tstype);
+
 /**
  * skb_tstamp_tx - queue clone of skb with send time stamps
  * @orig_skb:	the original outgoing packet
diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
index 97e1872..d37e5c7 100644
--- a/include/uapi/linux/errqueue.h
+++ b/include/uapi/linux/errqueue.h
@@ -36,6 +36,7 @@ struct scm_timestamping {
 /* type of ts[0], passed in ee_info */
 enum {
 	SCM_TSTAMP_SND = 1,	/* driver passed skb to NIC */
+	SCM_TSTAMP_ENQ,		/* data enqueued in traffic shaping layer */
 };
 
 #endif /* _UAPI_LINUX_ERRQUEUE_H */
diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index f53879c..99c2035 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -20,9 +20,11 @@ enum {
 	SOF_TIMESTAMPING_SOFTWARE = (1<<4),
 	SOF_TIMESTAMPING_SYS_HARDWARE = (1<<5),
 	SOF_TIMESTAMPING_RAW_HARDWARE = (1<<6),
-	SOF_TIMESTAMPING_MASK =
-	(SOF_TIMESTAMPING_RAW_HARDWARE - 1) |
-	SOF_TIMESTAMPING_RAW_HARDWARE
+	SOF_TIMESTAMPING_TX_ENQ = (1<<7),
+
+	SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_TX_ENQ,
+	SOF_TIMESTAMPING_MASK = (SOF_TIMESTAMPING_LAST - 1) |
+				 SOF_TIMESTAMPING_LAST
 };
 
 /**
diff --git a/net/core/dev.c b/net/core/dev.c
index e52a378..8bdf253 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -132,6 +132,7 @@
 #include <linux/hashtable.h>
 #include <linux/vmalloc.h>
 #include <linux/if_macvlan.h>
+#include <linux/errqueue.h>
 
 #include "net-sysfs.h"
 
@@ -2876,6 +2877,9 @@ static int __dev_queue_xmit(struct sk_buff *skb, void *accel_priv)
 
 	skb_reset_mac_header(skb);
 
+	if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_ENQ_TSTAMP))
+		__skb_tstamp_tx(skb, NULL, skb->sk, SCM_TSTAMP_ENQ);
+
 	/* Disable soft irqs for various locks below. Also
 	 * stops preemption for RCU.
 	 */
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index d95b09d..c4b724f 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3490,10 +3490,10 @@ int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)
 }
 EXPORT_SYMBOL(sock_queue_err_skb);
 
-void skb_tstamp_tx(struct sk_buff *orig_skb,
-		struct skb_shared_hwtstamps *hwtstamps)
+void __skb_tstamp_tx(struct sk_buff *orig_skb,
+		     struct skb_shared_hwtstamps *hwtstamps,
+		     struct sock *sk, int tstype)
 {
-	struct sock *sk = orig_skb->sk;
 	struct sock_exterr_skb *serr;
 	struct sk_buff *skb;
 	int err;
@@ -3521,7 +3521,7 @@ void skb_tstamp_tx(struct sk_buff *orig_skb,
 	memset(serr, 0, sizeof(*serr));
 	serr->ee.ee_errno = ENOMSG;
 	serr->ee.ee_origin = SO_EE_ORIGIN_TIMESTAMPING;
-	serr->ee.ee_info = hwtstamps ? 0 : SCM_TSTAMP_SND;
+	serr->ee.ee_info = tstype;
 	serr->ee.ee_data = skb->mark;
 
 	err = sock_queue_err_skb(sk, skb);
@@ -3529,6 +3529,14 @@ void skb_tstamp_tx(struct sk_buff *orig_skb,
 	if (err)
 		kfree_skb(skb);
 }
+EXPORT_SYMBOL_GPL(__skb_tstamp_tx);
+
+void skb_tstamp_tx(struct sk_buff *orig_skb,
+		   struct skb_shared_hwtstamps *hwtstamps)
+{
+	return __skb_tstamp_tx(orig_skb, hwtstamps, orig_skb->sk,
+			       hwtstamps ? 0 : SCM_TSTAMP_SND);
+}
 EXPORT_SYMBOL_GPL(skb_tstamp_tx);
 
 void skb_complete_wifi_ack(struct sk_buff *skb, bool acked)
diff --git a/net/socket.c b/net/socket.c
index 6c5c913..22c7f55 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -617,6 +617,9 @@ void sock_tx_timestamp(struct sock *sk, __u8 *tx_flags)
 		*tx_flags |= SKBTX_HW_TSTAMP;
 	if (sk->sk_tsflags & SOF_TIMESTAMPING_TX_SOFTWARE)
 		*tx_flags |= SKBTX_SW_TSTAMP;
+	if (sk->sk_tsflags & SOF_TIMESTAMPING_TX_ENQ)
+		*tx_flags |= SKBTX_ENQ_TSTAMP;
+
 	if (sock_flag(sk, SOCK_WIFI_STATUS))
 		*tx_flags |= SKBTX_WIFI_STATUS;
 }
-- 
2.0.0.526.g5318336

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH net-next v3 4/5] net-timestamp: TCP timestamping
  2014-07-21 19:35 [PATCH net-next v3 0/5] net-timestamp: additional sw tstamps and bytestream support Willem de Bruijn
                   ` (2 preceding siblings ...)
  2014-07-21 19:35 ` [PATCH net-next v3 3/5] net-timestamp: ENQ timestamp on enqueue to traffic shaping layer Willem de Bruijn
@ 2014-07-21 19:35 ` Willem de Bruijn
  2014-07-21 19:35 ` [PATCH net-next v3 5/5] net-timestamp: ACK timestamp for bytestreams Willem de Bruijn
  4 siblings, 0 replies; 15+ messages in thread
From: Willem de Bruijn @ 2014-07-21 19:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, richardcochran, Willem de Bruijn

TCP timestamping extends datagram MSG_TSTAMP support to bytestreams.

Bytestreams do not have a 1:1 relationship between send() buffers and
network packets. The feature interprets a send call with MSG_TSTAMP on
a bytestream as a request for a timestamp for the last byte in the
buffer.

The choice corresponds to a request for a timestamp when all bytes in
the buffer have been sent. That assumption depends on in-order kernel
transmission. This is the common case. That said, it is possible to
construct a traffic shaping tree that would result in reordering.
The guarantee is strong, then, but not ironclad.

This implementation supports send and sendpages (splice). GSO replaces
one large packet with multiple smaller packets. This patch also copies
the option into the correct smaller packet.

This patch does not yet support timestamping on data in an initial TCP
Fast Open SYN, because that takes a very different data path.

The implementation supports a single timestamp per packet. To avoid
having multiple timestamp requests per sk_buff, the skb is locked
against extension once the flag is set.

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 net/ipv4/tcp.c         | 21 +++++++++++++++++----
 net/ipv4/tcp_offload.c |  4 ++++
 2 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9d2118e..ca0a34a3 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -878,6 +878,11 @@ static int tcp_send_mss(struct sock *sk, int *size_goal, int flags)
 	return mss_now;
 }
 
+static bool tcp_skb_can_extend(struct sk_buff *skb)
+{
+	return !(skb_shinfo(skb)->tx_flags & SKBTX_ANY_SW_TSTAMP);
+}
+
 static ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
 				size_t size, int flags)
 {
@@ -911,7 +916,8 @@ static ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
 		int copy, i;
 		bool can_coalesce;
 
-		if (!tcp_send_head(sk) || (copy = size_goal - skb->len) <= 0) {
+		if (!tcp_send_head(sk) || (copy = size_goal - skb->len) <= 0 ||
+		    !tcp_skb_can_extend(skb)) {
 new_segment:
 			if (!sk_stream_memory_free(sk))
 				goto wait_for_sndbuf;
@@ -959,8 +965,10 @@ new_segment:
 
 		copied += copy;
 		offset += copy;
-		if (!(size -= copy))
+		if (!(size -= copy)) {
+			sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags);
 			goto out;
+		}
 
 		if (skb->len < size_goal || (flags & MSG_OOB))
 			continue;
@@ -1160,7 +1168,7 @@ int tcp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 				copy = max - skb->len;
 			}
 
-			if (copy <= 0) {
+			if (copy <= 0 || !tcp_skb_can_extend(skb)) {
 new_segment:
 				/* Allocate new segment. If the interface is SG,
 				 * allocate skb fitting to single page.
@@ -1252,8 +1260,10 @@ new_segment:
 
 			from += copy;
 			copied += copy;
-			if ((seglen -= copy) == 0 && iovlen == 0)
+			if ((seglen -= copy) == 0 && iovlen == 0) {
+				sock_tx_timestamp(sk, &skb_shinfo(skb)->tx_flags);
 				goto out;
+			}
 
 			if (skb->len < max || (flags & MSG_OOB) || unlikely(tp->repair))
 				continue;
@@ -1617,6 +1627,9 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	struct sk_buff *skb;
 	u32 urg_hole = 0;
 
+	if (unlikely(flags & MSG_ERRQUEUE))
+		return ip_recv_error(sk, msg, len, addr_len);
+
 	if (sk_can_busy_loop(sk) && skb_queue_empty(&sk->sk_receive_queue) &&
 	    (sk->sk_state == TCP_ESTABLISHED))
 		sk_busy_loop(sk, nonblock);
diff --git a/net/ipv4/tcp_offload.c b/net/ipv4/tcp_offload.c
index 4e86c59..730c2f6 100644
--- a/net/ipv4/tcp_offload.c
+++ b/net/ipv4/tcp_offload.c
@@ -134,6 +134,10 @@ struct sk_buff *tcp_gso_segment(struct sk_buff *skb,
 				(__force u32)delta));
 	if (skb->ip_summed != CHECKSUM_PARTIAL)
 		th->check = gso_make_checksum(skb, ~th->check);
+
+	if (unlikely(skb_shinfo(gso_skb)->tx_flags & SKBTX_SW_TSTAMP))
+		skb_shinfo(skb)->tx_flags |= SKBTX_SW_TSTAMP;
+
 out:
 	return segs;
 }
-- 
2.0.0.526.g5318336

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH net-next v3 5/5] net-timestamp: ACK timestamp for bytestreams
  2014-07-21 19:35 [PATCH net-next v3 0/5] net-timestamp: additional sw tstamps and bytestream support Willem de Bruijn
                   ` (3 preceding siblings ...)
  2014-07-21 19:35 ` [PATCH net-next v3 4/5] net-timestamp: TCP timestamping Willem de Bruijn
@ 2014-07-21 19:35 ` Willem de Bruijn
  4 siblings, 0 replies; 15+ messages in thread
From: Willem de Bruijn @ 2014-07-21 19:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, richardcochran, Willem de Bruijn

This patch adds send() flag MSG_TSTAMP_ACK, a request for a timestamp
when the last byte in the send buffer is acknowledged. It implements
the feature for TCP.

The timestamp is generated when the TCP socket cumulative ACK is
moved beyond the tracked seqno for the first time. This corresponds
to the other peer having received all data up until this byte. The
feature ignores SACK and FACK, because those acknowledge the
specific byte, but not necessarily the entire contents of the buffer
passed in send()

Signed-off-by: Willem de Bruijn <willemb@google.com>
---
 include/linux/skbuff.h          | 7 ++++++-
 include/uapi/linux/errqueue.h   | 1 +
 include/uapi/linux/net_tstamp.h | 3 ++-
 net/ipv4/tcp_input.c            | 4 ++++
 net/socket.c                    | 2 ++
 5 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 8c98dc9..13299db 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -263,9 +263,14 @@ enum {
 
 	/* generate software time stamp when queueing packing in TC */
 	SKBTX_ENQ_TSTAMP = 1 << 6,
+
+	/* generate software timestamp on peer data acknowledgment */
+	SKBTX_ACK_TSTAMP = 1 << 7,
 };
 
-#define SKBTX_ANY_SW_TSTAMP	(SKBTX_SW_TSTAMP | SKBTX_ENQ_TSTAMP)
+#define SKBTX_ANY_SW_TSTAMP	(SKBTX_SW_TSTAMP  | \
+				 SKBTX_ENQ_TSTAMP | \
+				 SKBTX_ACK_TSTAMP)
 #define SKBTX_ANY_TSTAMP	(SKBTX_HW_TSTAMP | SKBTX_ANY_SW_TSTAMP)
 
 /*
diff --git a/include/uapi/linux/errqueue.h b/include/uapi/linux/errqueue.h
index d37e5c7..81639bc 100644
--- a/include/uapi/linux/errqueue.h
+++ b/include/uapi/linux/errqueue.h
@@ -37,6 +37,7 @@ struct scm_timestamping {
 enum {
 	SCM_TSTAMP_SND = 1,	/* driver passed skb to NIC */
 	SCM_TSTAMP_ENQ,		/* data enqueued in traffic shaping layer */
+	SCM_TSTAMP_ACK,		/* data acknowledged by peer */
 };
 
 #endif /* _UAPI_LINUX_ERRQUEUE_H */
diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index 99c2035..bf9f338 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -21,8 +21,9 @@ enum {
 	SOF_TIMESTAMPING_SYS_HARDWARE = (1<<5),
 	SOF_TIMESTAMPING_RAW_HARDWARE = (1<<6),
 	SOF_TIMESTAMPING_TX_ENQ = (1<<7),
+	SOF_TIMESTAMPING_TX_ACK = (1<<8),
 
-	SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_TX_ENQ,
+	SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_TX_ACK,
 	SOF_TIMESTAMPING_MASK = (SOF_TIMESTAMPING_LAST - 1) |
 				 SOF_TIMESTAMPING_LAST
 };
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 7832d94..f52f159 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -74,6 +74,7 @@
 #include <linux/ipsec.h>
 #include <asm/unaligned.h>
 #include <net/netdma.h>
+#include <linux/errqueue.h>
 
 int sysctl_tcp_timestamps __read_mostly = 1;
 int sysctl_tcp_window_scaling __read_mostly = 1;
@@ -3102,6 +3103,9 @@ static int tcp_clean_rtx_queue(struct sock *sk, int prior_fackets,
 		if (!fully_acked)
 			break;
 
+		if (unlikely(skb_shinfo(skb)->tx_flags & SKBTX_ACK_TSTAMP))
+			__skb_tstamp_tx(skb, NULL, sk, SCM_TSTAMP_ACK);
+
 		tcp_unlink_write_queue(skb, sk);
 		sk_wmem_free_skb(sk, skb);
 		if (skb == tp->retransmit_skb_hint)
diff --git a/net/socket.c b/net/socket.c
index 22c7f55..7d3f171 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -619,6 +619,8 @@ void sock_tx_timestamp(struct sock *sk, __u8 *tx_flags)
 		*tx_flags |= SKBTX_SW_TSTAMP;
 	if (sk->sk_tsflags & SOF_TIMESTAMPING_TX_ENQ)
 		*tx_flags |= SKBTX_ENQ_TSTAMP;
+	if (sk->sk_tsflags & SOF_TIMESTAMPING_TX_ACK)
+		*tx_flags |= SKBTX_ACK_TSTAMP;
 
 	if (sock_flag(sk, SOCK_WIFI_STATUS))
 		*tx_flags |= SKBTX_WIFI_STATUS;
-- 
2.0.0.526.g5318336

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct
  2014-07-21 19:35 ` [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct Willem de Bruijn
@ 2014-07-21 23:24   ` David Miller
  2014-07-22 15:45     ` Willem de Bruijn
  0 siblings, 1 reply; 15+ messages in thread
From: David Miller @ 2014-07-21 23:24 UTC (permalink / raw)
  To: willemb; +Cc: netdev, eric.dumazet, richardcochran

From: Willem de Bruijn <willemb@google.com>
Date: Mon, 21 Jul 2014 15:35:53 -0400

> Applications that request kernel tx timestamps with SO_TIMESTAMPING
> read timestamps as recvmsg() ancillary data. The response is defined
> implicitly as timespec[3]. Define the struct, and
> 
> 1) Add support for new tstamp types. On tx, scm_timestamping always
> accompanies a sock_extended_err. Define previously unused field
> ee_info to signal the type of ts[0]. Introduce SCM_TSTAMP_SND.
> 
> 2) Add support to discern timestamps. When multiple timestamped
> packets are in flight concurrently, correlating a timestamp with
> the right send() call is non-trivial. Define previously unused field
> ee_data to communicate a send() specific key, skb->mark. It is
> up to the application to optionally set this field to a unique
> value for each send call.
> 
> The reception path is not modified. On rx, no struct similar to
> sock_extended_err is passed along with SCM_TIMESTAMPING.
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>

I'm not so sure about this skb->mark usage, it has other uses for things
such as classification, it influences route lookups, netfilter behavior,
etc.

I do not think you therefore want to add another usage, timestamp packet
keying, which may conflict.

Also:

>  	if (sock_flag(sk, SOCK_RCVTSTAMP) ||
>  	    sock_flag(sk, SOCK_TIMESTAMPING_RX_SOFTWARE) ||
> -	    (kt.tv64 && sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE)) ||
> +	    (kt.tv64 && (sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE) ||
> +	     skb_shinfo(skb)->tx_flags & SKBTX_ANY_SW_TSTAMP)) ||

This isn't indented correctly, the tx_flags test is part of the sock_flag()
test expression right before it so either:

	    (kt.tv64 && (sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE) ||
			 skb_shinfo(skb)->tx_flags & SKBTX_ANY_SW_TSTAMP)) ||

or:


	    (kt.tv64 &&
	     (sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE) ||
	      skb_shinfo(skb)->tx_flags & SKBTX_ANY_SW_TSTAMP)) ||

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct
  2014-07-21 23:24   ` David Miller
@ 2014-07-22 15:45     ` Willem de Bruijn
  2014-07-24  4:12       ` David Miller
  0 siblings, 1 reply; 15+ messages in thread
From: Willem de Bruijn @ 2014-07-22 15:45 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Eric Dumazet, Richard Cochran

>> 2) Add support to discern timestamps. When multiple timestamped
>> packets are in flight concurrently, correlating a timestamp with
>> the right send() call is non-trivial. Define previously unused field
>> ee_data to communicate a send() specific key, skb->mark. It is
>> up to the application to optionally set this field to a unique
>> value for each send call.
>
> I'm not so sure about this skb->mark usage, it has other uses for things
> such as classification, it influences route lookups, netfilter behavior,
> etc.
>
> I do not think you therefore want to add another usage, timestamp packet
> keying, which may conflict.

Ok. I will remove that part.

There are a few alternatives, but none as simple, so this is better
left to a separate patch. With skb_shinfo(skb)->syststamp deprecated
for all but the octeon legacy use case, it is possible to repurpose
those bytes for new uses in a union, including for a new u16 ts_key.
The sk would still have to grow by a new field, though, e.g.,
ts_counter. I have to think about this a bit more first.

> This isn't indented correctly, the tx_flags test is part of the sock_flag()
> test expression right before it so either:

Thanks.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct
  2014-07-22 15:45     ` Willem de Bruijn
@ 2014-07-24  4:12       ` David Miller
  2014-07-24 20:48         ` Willem de Bruijn
  0 siblings, 1 reply; 15+ messages in thread
From: David Miller @ 2014-07-24  4:12 UTC (permalink / raw)
  To: willemb; +Cc: netdev, eric.dumazet, richardcochran

From: Willem de Bruijn <willemb@google.com>
Date: Tue, 22 Jul 2014 11:45:30 -0400

> There are a few alternatives, but none as simple, so this is better
> left to a separate patch. With skb_shinfo(skb)->syststamp deprecated
> for all but the octeon legacy use case, it is possible to repurpose
> those bytes for new uses in a union, including for a new u16 ts_key.
> The sk would still have to grow by a new field, though, e.g.,
> ts_counter. I have to think about this a bit more first.

I can't believe we're going to hold onto syststamp just for one
odd-ball MIPS ethernet driver.  Really?

I think putting in whatever work is necessary to convert Octeon over
to exposing a proper PTP device is much better than having this
syststamp thing rot in our data structures forever.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct
  2014-07-24  4:12       ` David Miller
@ 2014-07-24 20:48         ` Willem de Bruijn
  2014-07-24 21:15           ` David Miller
  0 siblings, 1 reply; 15+ messages in thread
From: Willem de Bruijn @ 2014-07-24 20:48 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Eric Dumazet, Richard Cochran, david.daney, Chad Reese

> I can't believe we're going to hold onto syststamp just for one
> odd-ball MIPS ethernet driver.  Really?
>
> I think putting in whatever work is necessary to convert Octeon over
> to exposing a proper PTP device is much better than having this
> syststamp thing rot in our data structures forever.

Implementing ptp_clock_info gettime/settime/adjtime/adjfreq using
CVMX_MIO_PTP_CLOCK_[CFG|HI|COMP] might be feasible.

That does not address legacy application dependencies on syststamp.
Even with a PTP device, it is unclear whether anyone would invest time
to convert their application to use it. The most relevant code I could
find is cavium ptp-1588v2 on github. That configures SO_TIMESTAMPING
to generate both raw + sys tstamps and preferentially reads the first.
If this is the only known octeon timestamping user, then it would be
just as safe to simply remove syststamp.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct
  2014-07-24 20:48         ` Willem de Bruijn
@ 2014-07-24 21:15           ` David Miller
  2014-07-24 23:05             ` Willem de Bruijn
  0 siblings, 1 reply; 15+ messages in thread
From: David Miller @ 2014-07-24 21:15 UTC (permalink / raw)
  To: willemb; +Cc: netdev, eric.dumazet, richardcochran, david.daney, kreese

From: Willem de Bruijn <willemb@google.com>
Date: Thu, 24 Jul 2014 16:48:04 -0400

>> I can't believe we're going to hold onto syststamp just for one
>> odd-ball MIPS ethernet driver.  Really?
>>
>> I think putting in whatever work is necessary to convert Octeon over
>> to exposing a proper PTP device is much better than having this
>> syststamp thing rot in our data structures forever.
> 
> Implementing ptp_clock_info gettime/settime/adjtime/adjfreq using
> CVMX_MIO_PTP_CLOCK_[CFG|HI|COMP] might be feasible.
> 
> That does not address legacy application dependencies on syststamp.
> Even with a PTP device, it is unclear whether anyone would invest time
> to convert their application to use it. The most relevant code I could
> find is cavium ptp-1588v2 on github. That configures SO_TIMESTAMPING
> to generate both raw + sys tstamps and preferentially reads the first.
> If this is the only known octeon timestamping user, then it would be
> just as safe to simply remove syststamp.

The kernel would simply saw that adjusted HW timestamps are not
supported, the application has to accomodate the possibility of
needing to use one of the other timestamping flavors.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct
  2014-07-24 21:15           ` David Miller
@ 2014-07-24 23:05             ` Willem de Bruijn
  2014-07-24 23:50               ` David Miller
  0 siblings, 1 reply; 15+ messages in thread
From: Willem de Bruijn @ 2014-07-24 23:05 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Eric Dumazet, Richard Cochran, Chad Reese, ddaney

>> That does not address legacy application dependencies on syststamp.
>> Even with a PTP device, it is unclear whether anyone would invest time
>> to convert their application to use it. The most relevant code I could
>> find is cavium ptp-1588v2 on github. That configures SO_TIMESTAMPING
>> to generate both raw + sys tstamps and preferentially reads the first.
>> If this is the only known octeon timestamping user, then it would be
>> just as safe to simply remove syststamp.
>
> The kernel would simply saw that adjusted HW timestamps are not
> supported, the application has to accomodate the possibility of
> needing to use one of the other timestamping flavors.

Cavium's PTP library already programs the device PTP clock,
just not through the standard interface. It appears to map the
memory addresses CVMX_MIO_.. directly. In any case, it
will gracefully handle the loss of syststamp.

I doubt that there are other users of this Octeon interface, but
if so, they can reasonably be expected to do the same and
rely on the same legacy Octeon PTP clock interface +
hwtstamp as alternative. Is that enough justification to remove
syststamp without patching the driver to implement
ptp_clock_info?

Cleanup would be two patches: one to revert the relevant
code in the octeon driver (mostly, ptp_to_ktime), and a
follow-up to remove the core code now that the last user
is gone.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct
  2014-07-24 23:05             ` Willem de Bruijn
@ 2014-07-24 23:50               ` David Miller
  2014-07-25  0:10                 ` Willem de Bruijn
  0 siblings, 1 reply; 15+ messages in thread
From: David Miller @ 2014-07-24 23:50 UTC (permalink / raw)
  To: willemb; +Cc: netdev, eric.dumazet, richardcochran, kreese, ddaney

From: Willem de Bruijn <willemb@google.com>
Date: Thu, 24 Jul 2014 19:05:14 -0400

>>> That does not address legacy application dependencies on syststamp.
>>> Even with a PTP device, it is unclear whether anyone would invest time
>>> to convert their application to use it. The most relevant code I could
>>> find is cavium ptp-1588v2 on github. That configures SO_TIMESTAMPING
>>> to generate both raw + sys tstamps and preferentially reads the first.
>>> If this is the only known octeon timestamping user, then it would be
>>> just as safe to simply remove syststamp.
>>
>> The kernel would simply saw that adjusted HW timestamps are not
>> supported, the application has to accomodate the possibility of
>> needing to use one of the other timestamping flavors.
> 
> Cavium's PTP library already programs the device PTP clock,
> just not through the standard interface. It appears to map the
> memory addresses CVMX_MIO_.. directly. In any case, it
> will gracefully handle the loss of syststamp.
> 
> I doubt that there are other users of this Octeon interface, but
> if so, they can reasonably be expected to do the same and
> rely on the same legacy Octeon PTP clock interface +
> hwtstamp as alternative. Is that enough justification to remove
> syststamp without patching the driver to implement
> ptp_clock_info?
> 
> Cleanup would be two patches: one to revert the relevant
> code in the octeon driver (mostly, ptp_to_ktime), and a
> follow-up to remove the core code now that the last user
> is gone.

I'm not sure I understand, can users still get hardware timestamps
from Octeon cards after such a change?  Unless I misunderstand, the
only thing left available will be software timestamps, right?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct
  2014-07-24 23:50               ` David Miller
@ 2014-07-25  0:10                 ` Willem de Bruijn
  2014-07-25  0:53                   ` David Miller
  0 siblings, 1 reply; 15+ messages in thread
From: Willem de Bruijn @ 2014-07-25  0:10 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, Eric Dumazet, Richard Cochran, Chad Reese, David Daney

>>> The kernel would simply saw that adjusted HW timestamps are not
>>> supported, the application has to accomodate the possibility of
>>> needing to use one of the other timestamping flavors.
>>
>> Cavium's PTP library already programs the device PTP clock,
>> just not through the standard interface. It appears to map the
>> memory addresses CVMX_MIO_.. directly. In any case, it
>> will gracefully handle the loss of syststamp.
>>
>> I doubt that there are other users of this Octeon interface, but
>> if so, they can reasonably be expected to do the same and
>> rely on the same legacy Octeon PTP clock interface +
>> hwtstamp as alternative. Is that enough justification to remove
>> syststamp without patching the driver to implement
>> ptp_clock_info?
>>
>> Cleanup would be two patches: one to revert the relevant
>> code in the octeon driver (mostly, ptp_to_ktime), and a
>> follow-up to remove the core code now that the last user
>> is gone.
>
> I'm not sure I understand, can users still get hardware timestamps
> from Octeon cards after such a change?  Unless I misunderstand, the
> only thing left available will be software timestamps, right?

They will still receive hardware timestamps in raw format. The
only feature that is deprecated is hardware timestamps
converted to system time format (ptp_to_ktime).

By querying the PTP clock source, users can perform  perform
this conversion to system time themselves. The Octeon driver
does not support the standard PTP clock interface, but does
appear to use its own alternative using shared memory. I'm
not exactly sure how that memory is setup.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct
  2014-07-25  0:10                 ` Willem de Bruijn
@ 2014-07-25  0:53                   ` David Miller
  0 siblings, 0 replies; 15+ messages in thread
From: David Miller @ 2014-07-25  0:53 UTC (permalink / raw)
  To: willemb; +Cc: netdev, eric.dumazet, richardcochran, kreese, ddaney

From: Willem de Bruijn <willemb@google.com>
Date: Thu, 24 Jul 2014 20:10:55 -0400

> They will still receive hardware timestamps in raw format. The
> only feature that is deprecated is hardware timestamps
> converted to system time format (ptp_to_ktime).

OK.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-07-25  0:53 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-21 19:35 [PATCH net-next v3 0/5] net-timestamp: additional sw tstamps and bytestream support Willem de Bruijn
2014-07-21 19:35 ` [PATCH net-next v3 1/5] net-timestamp: extend SCM_TIMESTAMPING ancillary data struct Willem de Bruijn
2014-07-21 23:24   ` David Miller
2014-07-22 15:45     ` Willem de Bruijn
2014-07-24  4:12       ` David Miller
2014-07-24 20:48         ` Willem de Bruijn
2014-07-24 21:15           ` David Miller
2014-07-24 23:05             ` Willem de Bruijn
2014-07-24 23:50               ` David Miller
2014-07-25  0:10                 ` Willem de Bruijn
2014-07-25  0:53                   ` David Miller
2014-07-21 19:35 ` [PATCH net-next v3 2/5] net-timestamp: move timestamp flags out of sk_flags Willem de Bruijn
2014-07-21 19:35 ` [PATCH net-next v3 3/5] net-timestamp: ENQ timestamp on enqueue to traffic shaping layer Willem de Bruijn
2014-07-21 19:35 ` [PATCH net-next v3 4/5] net-timestamp: TCP timestamping Willem de Bruijn
2014-07-21 19:35 ` [PATCH net-next v3 5/5] net-timestamp: ACK timestamp for bytestreams Willem de Bruijn

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).