hardware time stamping with extra skb->hwtstamp

This patch series was discussed before on linux-netdev ("hardware time stamping + igb example implementation"). Since then I have rebased against net-next and addressed all comments sent so far, except Octavian's suggestion to include more information in the packet which is bounced back to the application.

As suggested by David, I'm now also including linux-kernel because:
* patch 2 adds a new user space API (documentation and example program included, but no man page patch yet)
* patch 8 extends the clocksource struct (mostly adds code, but also adds one branch to reading the clock, which may affect gettimeofday)
* patch 10 adds generic code for time synchronization (not network specific, so people not subscribed to linux-netdev might have comments)

The open question on linux-netdev was whether struct sk_buff should be extended to hold the additional hardware time stamp. The previous patch avoided that at the cost of much more complicated code and side effects on normal time stamping.

This patch now adds a 8 byte field unconditionally. The implementation is a lot more straight-forward. The user space API was already designed to cover this case, so it remained unchanged.

There's one unsolved problem, though: time synchronization with PTP (the use case I'm working on) requires a transformation of raw hardware time stamps into system time. Currently this is done at the socket level by finding the device which created the time stamp and letting it do the transformation. This fails for incoming packets because skb->rt points to the "lo" device.

Perhaps the interface number can be used to find the real hardware device. Alternatively the conversion could be done when generating the time stamp (might also be more accurate), but then another 8 byte field is needed. Delta encoding won't help because one cannot assume that hardware time stamps track system time closely enough.

I'm posting the patch despite this problem so that the discussion can move forward. There are other TODOs anyway; in particular the igb extension is just a proof-of-concept.

Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

In __sock_recv_timestamp() the additional SCM_TIMESTAMP[NS] is used. This has the same value as SO_TIMESTAMP[NS], so this is a purely cosmetic change.

---
 net/compat.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

index 67fb6a3..6ce1a1c 100644
@@ -226,14 +226,14 @@ int put_cmsg_compat(struct msghdr *kmsg, int level, int type, int len, void *dat
 		return 0; /* XXX: return error? check spec. */
 	}
 
-	if (level == SOL_SOCKET && type == SO_TIMESTAMP) {
+	if (level == SOL_SOCKET && type == SCM_TIMESTAMP) {
 		struct timeval *tv = (struct timeval *)data;
 		ctv.tv_sec = tv->tv_sec;
 		ctv.tv_usec = tv->tv_usec;
 		data = &ctv;
 		len = sizeof(ctv);
 	}
-	if (level == SOL_SOCKET && type == SO_TIMESTAMPNS) {
+	if (level == SOL_SOCKET && type == SCM_TIMESTAMPNS) {
 		struct timespec *ts = (struct timespec *)data;
 		cts.tv_sec = ts->tv_sec;
 		cts.tv_nsec = ts->tv_nsec;

1.6.0.4

User space can request hardware and/or software time stamping. Reporting of the result(s) via a new control message is enabled separately for each field in the message because some of the fields may require additional computation and thus cause overhead.

When a TX timestamp operation is requested, the TX skb will be cloned and the clone will be time stamped (in hardware or software) and added to the socket error queue of the skb, if the skb has a socket associated with it.

The actual TX timestamp will reach userspace as a RX timestamp on the cloned packet. If timestamping is requested and no timestamping is done in the device driver (potentially this may use hardware timestamping), it will be done in software after the device's start_hard_xmit routine.

TODO: add SO_TIMESTAMPING define also to other platforms

---
 Documentation/networking/timestamping.txt          |  178 ++++++++
 .../networking/timestamping/timestamping.c         |  469 ++++++++++++++++++++
 arch/x86/include/asm/socket.h                      |    3 +
 include/linux/errqueue.h                           |    1 +
 include/linux/sockios.h                            |    3 +
 include/net/timestamping.h                         |   95 ++++
 6 files changed, 749 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/networking/timestamping.txt
 create mode 100644 Documentation/networking/timestamping/timestamping.c
 create mode 100644 include/net/timestamping.h

new file mode 100644
index 0000000..3ae60b5
@@ -0,0 +1,178 @@
+The existing interfaces for getting network packages time stamped are:
+
+* SO_TIMESTAMP
+  Generate time stamp for each incoming packet using the (not necessarily
+  monotonous!) system time. Result is returned via recv_msg() in a
+  control message as timeval (usec resolution).
+
+* SO_TIMESTAMPNS
+  Same time stamping mechanism as SO_TIMESTAMP, but returns result as
+  timespec (nsec resolution).
+
+* IP_MULTICAST_LOOP + SO_TIMESTAMP[NS]
+  Only for multicasts: approximate send time stamp by receiving the looped
+  packet and using its receive time stamp.
+
+The following interface complements the existing ones: receive time
+stamps can be generated and returned for arbitrary packets and much
+closer to the point where the packet is really sent. Time stamps can
+be generated in software (as before) or in hardware (if the hardware
+has such a feature).
+
+SO_TIMESTAMPING:
+
+Instructs the socket layer which kind of information is wanted. The
+parameter is an integer with some of the following bits set. Setting
+other bits is an error and doesn't change the current state.
+
+SOF_TIMESTAMPING_TX_HARDWARE:  try to obtain send time stamp in hardware
+SOF_TIMESTAMPING_TX_SOFTWARE:  if SOF_TIMESTAMPING_TX_HARDWARE is off or
+                               fails, then do it in software
+SOF_TIMESTAMPING_RX_HARDWARE:  return the original, unmodified time stamp
+                               as generated by the hardware
+SOF_TIMESTAMPING_RX_SOFTWARE:  if SOF_TIMESTAMPING_RX_HARDWARE is off or
+                               fails, then do it in software
+SOF_TIMESTAMPING_RAW_HARDWARE: return original raw hardware time stamp
+SOF_TIMESTAMPING_SYS_HARDWARE: return hardware time stamp transformed to
+                               the system time base
+SOF_TIMESTAMPING_SOFTWARE:     return system time stamp generated in
+                               software
+
+SOF_TIMESTAMPING_TX/RX determine how time stamps are generated.
+SOF_TIMESTAMPING_RAW/SYS determine how they are reported in the
+following control message:
+    struct scm_timestamping {
+           struct timespec systime;
+           struct timespec hwtimetrans;
+           struct timespec hwtimeraw;
+    };
+
+recvmsg() can be used to get this control message for regular incoming
+packets. For send time stamps the outgoing packet is looped back to
+the socket's error queue with the send time stamp(s) attached. It can
+be received with recvmsg(flags=MSG_ERRQUEUE). The call returns the
+original outgoing packet data including all headers preprended down to
+and including the link layer, the scm_timestamping control message and
+a sock_extended_err control message with ee_errno==ENOMSG and
+ee_origin==SO_EE_ORIGIN_TIMESTAMPING. A socket with such a pending
+bounced packet is ready for reading as far as select() is concerned.
+
+All three values correspond to the same event in time, but were
+generated in different ways. Each of these values may be empty (= all
+zero), in which case no such value was available. If the application
+is not interested in some of these values, they can be left blank to
+avoid the potential overhead of calculating them.
+
+systime is the value of the system time at that moment. This
+corresponds to the value also returned via SO_TIMESTAMP[NS]. If the
+time stamp was generated by hardware, then this field is
+empty. Otherwise it is filled in if SOF_TIMESTAMPING_SOFTWARE is
+set.
+
+hwtimeraw is the original hardware time stamp. Filled in if
+SOF_TIMESTAMPING_RAW_HARDWARE is set. No assumptions about its
+relation to system time should be made.
+
+hwtimetrans is the hardware time stamp transformed so that it
+corresponds as good as possible to system time. This correlation is
+not perfect; as a consequence, sorting packets received via different
+NICs by their hwtimetrans may differ from the order in which they were
+received. hwtimetrans may be non-monotonic even for the same NIC.
+Filled in if SOF_TIMESTAMPING_SYS_HARDWARE is set. Requires support
+by the network device and will be empty without that support.
+
+
+SIOCSHWTSTAMP:
+
+Hardware time stamping must also be initialized for each device driver
+that is expected to do hardware time stamping. The parameter is:
+
+struct hwtstamp_config {
+    int flags;           /**< no flags defined right now, must be zero */
+    int tx_type;         /**< HWTSTAMP_TX_* */
+    int rx_filter_type;  /**< HWTSTAMP_RX_* */
+};
+
+Desired behavior is passed into the kernel and to a specific device by
+calling ioctl(SIOCSHWTSTAMP) with a pointer to a struct ifreq whose
+ifr_data points to a struct hwtstamp_config. The tx_type and
+rx_filter_type are hints to the driver what it is expected to do. If
+the requested fine-grained filtering for incoming packets is not
+supported, the driver may time stamp more than just the requested types
+of packets.
+
+A driver which supports hardware time stamping shall update the struct
+with the actual, possibly more permissive configuration. If the
+requested packets cannot be time stamped, then nothing should be
+changed and ERANGE shall be returned (in contrast to EINVAL, which
+indicates that SIOCSHWTSTAMP is not supported at all).
+
+Only a processes with admin rights may change the configuration. User
+space is responsible to ensure that multiple processes don't interfere
+with each other and that the settings are reset.
+
+/** possible values for hwtstamp_config->tx_type */
+enum {
+	/**
+	 * no outgoing packet will need hardware time stamping;
+	 * should a packet arrive which asks for it, no hardware
+	 * time stamping will be done
+	 */
+	HWTSTAMP_TX_OFF,
+
+	/**
+	 * enables hardware time stamping for outgoing packets;
+	 * the sender of the packet decides which are to be
+	 * time stamped by setting SOF_TIMESTAMPING_TX_SOFTWARE
+	 * before sending the packet
+	 */
+	HWTSTAMP_TX_ON,
+};
+
+/** possible values for hwtstamp_config->rx_filter_type */
+enum {
+	/** time stamp no incoming packet at all */
+	HWTSTAMP_FILTER_NONE,
+
+	/** time stamp any incoming packet */
+	HWTSTAMP_FILTER_ALL,
+
+        /** return value: time stamp all packets requested plus some others */
+        HWTSTAMP_FILTER_SOME,
+
+	/** PTP v1, UDP, any kind of event packet */
+	HWTSTAMP_FILTER_PTP_V1_L4_EVENT,
+
+        ...
+};
+
+
+DEVICE IMPLEMENTATION
+
+A driver which supports hardware time stamping must support the
+SIOCSHWTSTAMP ioctl. Time stamps for received packets must be stored
+in the skb with skb_hwtstamp_set().
+
+Time stamps for outgoing packets are to be generated as follows:
+- In hard_start_xmit(), check if skb_hwtstamp_check_tx_hardware()
+  returns non-zero. If yes, then the driver is expected
+  to do hardware time stamping.
+- If this is possible for the skb and requested, then declare
+  that the driver is doing the time stamping by calling
+  skb_hwtstamp_tx_in_progress(). A driver not supporting
+  hardware time stamping doesn't do that. A driver must never
+  touch sk_buff::tstamp! It is used to store how time stamping
+  for an outgoing packets is to be done.
+- As soon as the driver has sent the packet and/or obtained a
+  hardware time stamp for it, it passes the time stamp back by
+  calling skb_hwtstamp_tx() with the original skb, the raw
+  hardware time stamp and a handle to the device (necessary
+  to convert the hardware time stamp to system time). If obtaining
+  the hardware time stamp somehow fails, then the driver should
+  not fall back to software time stamping. The rationale is that
+  this would occur at a later time in the processing pipeline
+  than other software time stamping and therefore could lead
+  to unexpected deltas between time stamps.
+- If the driver did not call skb_hwtstamp_tx_in_progress(), then
+  dev_hard_start_xmit() checks whether software time stamping
+  is wanted as fallback and potentially generates the time stamp.

index 8ab9cc8..79e1f6c 100644
@@ -54,4 +54,7 @@
 
 #define SO_MARK			36
 
+#define SO_TIMESTAMPING         37
+#define SCM_TIMESTAMPING        SO_TIMESTAMPING
+
 #endif /* _ASM_X86_SOCKET_H */

index 92f8d4f..86d88dd 100644
@@ -16,6 +16,7 @@ struct sock_extended_err
 #define SO_EE_ORIGIN_LOCAL	1
 #define SO_EE_ORIGIN_ICMP	2
 #define SO_EE_ORIGIN_ICMP6	3
+#define SO_EE_ORIGIN_TIMESTAMPING 4
 
 #define SO_EE_OFFENDER(ee)	((struct sockaddr*)((ee)+1))
 

index abef759..209ee22 100644
@@ -122,6 +122,9 @@
 #define SIOCBRADDIF	0x89a2		/* add interface to bridge      */
 #define SIOCBRDELIF	0x89a3		/* remove interface from bridge */
 
+/* hardware time stamping: parameters in net/timestamping.h */
+#define SIOCSHWTSTAMP   0x89b0
+
 /* Device private ioctl calls */
 
 /*

1.6.0.4

The new sk_buff->hwtstamp is used to transport time stamping instructions to the device driver (outgoing packets) and to return raw hardware time stamps back to user space (incoming or sent packets).

Implements TX time stamping in software if the device driver doesn't support hardware time stamping.

The new semantic for hardware/software time stamping around net_device->hard_start_xmit() is based on two assumptions about existing network device drivers which don't support hardware time stamping and know nothing about it:
- they leave the skb->hwtstamp field unmodified
- the keep the connection to the originating socket in skb->sk alive, i.e., don't call skb_orphan()

Given that hwtstamp is a new field, the first assumption is safe. The second is only true for some drivers. As a result, software TX time stamping currently works with the bnx2 driver, but not with the unmodified igb driver (the two drivers this patch series was tested with).

---
 include/linux/netdevice.h |   11 ++++
 include/linux/skbuff.h    |  136 ++++++++++++++++++++++++++++++++++++++++++++-
 net/core/dev.c            |   23 +++++++-
 net/core/skbuff.c         |   72 ++++++++++++++++++++++++
 4 files changed, 239 insertions(+), 3 deletions(-)

index 12d7f44..24bea0c 100644
@@ -763,6 +763,17 @@ struct net_device
 	/* for setting kernel sock attribute on TCP connection setup */
 #define GSO_MAX_SIZE		65536
 	unsigned int		gso_max_size;
+
+#define HAVE_HW_TIME_STAMP
+	/* Transforms original raw hardware time stamp to
+	 * system time base. Always required when supporting
+	 * hardware time stamping.
+	 *
+	 * Returns empty stamp (= all zero) if conversion wasn't
+	 * possible.
+	 */
+	ktime_t             (*hwtstamp_raw2sys)(struct net_device *dev,
+						ktime_t hwstamp);
 };
 #define to_net_dev(d) container_of(d, struct net_device, dev)
 

index e08c0fc..b4b8eb8 100644
@@ -1658,6 +1658,8 @@ static int dev_gso_segment(struct sk_buff *skb)
 int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 			struct netdev_queue *txq)
 {
+	int rc;
+
 	if (likely(!skb->next)) {
 		if (!list_empty(&ptype_all))
 			dev_queue_xmit_nit(skb, dev);

@@ -1669,13 +1671,29 @@ int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
 				goto gso;
 		}
 
-		return dev->hard_start_xmit(skb, dev);
+		rc = dev->hard_start_xmit(skb, dev);
+		/*
+		 * TODO: if skb_orphan() was called by
+		 * dev->hard_start_xmit() (for example, the unmodified
+		 * igb driver does that; bnx2 doesn't), then
+		 * skb_tx_software_timestamp() will be unable to send
+		 * back the time stamp.
+		 *
+		 * How can this be prevented? Always create another
+		 * reference to the socket before calling
+		 * dev->hard_start_xmit()? Prevent that skb_orphan()
+		 * does anything in dev->hard_start_xmit() by clearing
+		 * the skb destructor before the call and restoring it
+		 * afterwards, then doing the skb_orphan() ourselves?
+		 */
+		if (likely(!rc))
+			skb_tx_software_timestamp(skb);
+		return rc;
 	}
 
 gso:
 	do {
 		struct sk_buff *nskb = skb->next;
-		int rc;
 
 		skb->next = nskb->next;
 		nskb->next = NULL;

@@ -1685,6 +1703,7 @@ gso:
 			skb->next = nskb;
 			return rc;
 		}
+		skb_tx_software_timestamp(skb);
 		if (unlikely(netif_tx_queue_stopped(txq) && skb->next))
 			return NETDEV_TX_BUSY;
 	} while (skb->next);

1.6.0.4

The overlap with the old SO_TIMESTAMP[NS] options is handled so that time stamping in software (net_enable_timestamp()) is enabled when SO_TIMESTAMP[NS] and/or SO_TIMESTAMPING_RX_SOFTWARE is set. It's disabled if all of these are off.

---
 include/net/sock.h |   37 ++++++++++++++++++++++--
 net/compat.c       |   19 ++++++++----
 net/core/sock.c    |   79 ++++++++++++++++++++++++++++++++++++++++++++--------
 net/socket.c       |   75 ++++++++++++++++++++++++++++++++++++-------------
 4 files changed, 168 insertions(+), 42 deletions(-)

index 0a63894..0f71951 100644
@@ -158,7 +158,7 @@ struct sock_common {
   *	@sk_allocation: allocation mode
   *	@sk_sndbuf: size of send buffer in bytes
   *	@sk_flags: %SO_LINGER (l_onoff), %SO_BROADCAST, %SO_KEEPALIVE,
-  *		   %SO_OOBINLINE settings
+  *		   %SO_OOBINLINE settings, %SO_TIMESTAMPING settings
   *	@sk_no_check: %SO_NO_CHECK setting, wether or not checkup packets
   *	@sk_route_caps: route capabilities (e.g. %NETIF_F_TSO)
   *	@sk_gso_type: GSO type (e.g. %SKB_GSO_TCPV4)

@@ -488,6 +488,13 @@ enum sock_flags {
 	SOCK_RCVTSTAMPNS, /* %SO_TIMESTAMPNS setting */
 	SOCK_LOCALROUTE, /* route locally only, %SO_DONTROUTE setting */
 	SOCK_QUEUE_SHRUNK, /* write queue has been shrunk recently */
+	SOCK_TIMESTAMPING_TX_HARDWARE, /* %SO_TIMESTAMPING %SOF_TIMESTAMPING_TX_HARDWARE */
+	SOCK_TIMESTAMPING_TX_SOFTWARE, /* %SO_TIMESTAMPING %SOF_TIMESTAMPING_TX_SOFTWARE */
+	SOCK_TIMESTAMPING_RX_HARDWARE, /* %SO_TIMESTAMPING %SOF_TIMESTAMPING_RX_HARDWARE */
+	SOCK_TIMESTAMPING_RX_SOFTWARE, /* %SO_TIMESTAMPING %SOF_TIMESTAMPING_RX_SOFTWARE */
+	SOCK_TIMESTAMPING_SOFTWARE,    /* %SO_TIMESTAMPING %SOF_TIMESTAMPING_SOFTWARE */
+	SOCK_TIMESTAMPING_RAW_HARDWARE, /* %SO_TIMESTAMPING %SOF_TIMESTAMPING_RAW_HARDWARE */
+	SOCK_TIMESTAMPING_SYS_HARDWARE, /* %SO_TIMESTAMPING %SOF_TIMESTAMPING_SYS_HARDWARE */
 };
 
 static inline void sock_copy_flags(struct sock *nsk, struct sock *osk)

@@ -1342,13 +1349,37 @@ sock_recv_timestamp(struct msghdr *msg, struct sock *sk, struct sk_buff *skb)
 {
 	ktime_t kt = skb->tstamp;
 
-	if (sock_flag(sk, SOCK_RCVTSTAMP))
+	/*
+	 * generate control messages if receive time stamping requested
+	 * or if time stamp available (RX hardware or TX software/hardware
+	 * case) and reporting via SO_TIMESTAMPING enabled
+	 */
+	if ((sock_flag(sk, SOCK_RCVTSTAMP) ||
+		sock_flag(sk, SOCK_TIMESTAMPING_RX_SOFTWARE)) ||
+		(kt.tv64 && sock_flag(sk, SOCK_TIMESTAMPING_SOFTWARE)) ||
+		(skb_hwtstamp_available(skb) &&
+			(sock_flag(sk, SOCK_TIMESTAMPING_SYS_HARDWARE) ||
+				sock_flag(sk, SOCK_TIMESTAMPING_RAW_HARDWARE))))
 		__sock_recv_timestamp(msg, sk, skb);
 	else
 		sk->sk_stamp = kt;
 }
 
 /**
+ * sock_tx_timestamp - checks whether the outgoing packet is to be time stamped
+ * @msg: outgoing packet
+ * @sk: socket sending this packet
+ * @stamp_tx: filled with instructions for time stamping
+ *
+ * Currently only depends on SOCK_TIMESTAMPING* flags. Returns error code if
+ * parameters are invalid.
+ */
+extern int sock_tx_timestamp(struct msghdr *msg,
+			struct sock *sk,
+			union sk_buff_hwtstamp *stamp_tx);
+
+
+/**
  * sk_eat_skb - Release a skb if it is no longer needed
  * @sk: socket to eat this skb from
  * @skb: socket buffer to eat

@@ -1416,7 +1447,7 @@ static inline struct sock *skb_steal_sock(struct sk_buff *skb)
 	return NULL;
 }
 
-extern void sock_enable_timestamp(struct sock *sk);
+extern void sock_enable_timestamp(struct sock *sk, int flag);
 extern int sock_get_timestamp(struct sock *, struct timeval __user *);
 extern int sock_get_timestampns(struct sock *, struct timespec __user *);
 

index 6ce1a1c..954377e 100644
@@ -216,7 +216,7 @@ Efault:
 int put_cmsg_compat(struct msghdr *kmsg, int level, int type, int len, void *data)
 {
 	struct compat_timeval ctv;
-	struct compat_timespec cts;
+	struct compat_timespec cts[3];
 	struct compat_cmsghdr __user *cm = (struct compat_cmsghdr __user *) kmsg->msg_control;
 	struct compat_cmsghdr cmhdr;
 	int cmlen;

@@ -233,12 +233,17 @@ int put_cmsg_compat(struct msghdr *kmsg, int level, int type, int len, void *dat
 		data = &ctv;
 		len = sizeof(ctv);
 	}
-	if (level == SOL_SOCKET && type == SCM_TIMESTAMPNS) {
+	if (level == SOL_SOCKET &&
+		(type == SCM_TIMESTAMPNS || type == SCM_TIMESTAMPING)) {
+		int count = type == SCM_TIMESTAMPNS ? 1 : 3;
+		int i;
 		struct timespec *ts = (struct timespec *)data;
-		cts.tv_sec = ts->tv_sec;
-		cts.tv_nsec = ts->tv_nsec;
+		for (i = 0; i < count; i++) {
+			cts[i].tv_sec = ts[i].tv_sec;
+			cts[i].tv_nsec = ts[i].tv_nsec;
+		}
 		data = &cts;
-		len = sizeof(cts);
+		len = sizeof(cts[0]) * count;
 	}
 
 	cmlen = CMSG_COMPAT_LEN(len);

@@ -455,7 +460,7 @@ int compat_sock_get_timestamp(struct sock *sk, struct timeval __user *userstamp)
 	struct timeval tv;
 
 	if (!sock_flag(sk, SOCK_TIMESTAMP))
-		sock_enable_timestamp(sk);
+		sock_enable_timestamp(sk, SOCK_TIMESTAMP);
 	tv = ktime_to_timeval(sk->sk_stamp);
 	if (tv.tv_sec == -1)
 		return err;

@@ -479,7 +484,7 @@ int compat_sock_get_timestampns(struct sock *sk, struct timespec __user *usersta
 	struct timespec ts;
 
 	if (!sock_flag(sk, SOCK_TIMESTAMP))
-		sock_enable_timestamp(sk);
+		sock_enable_timestamp(sk, SOCK_TIMESTAMP);
 	ts = ktime_to_timespec(sk->sk_stamp);
 	if (ts.tv_sec == -1)
 		return err;

1.6.0.4

Instructions for time stamping outgoing packets are take from the socket layer and later copied into the new skb.

---
 include/net/ip.h     |    1 +
 net/can/raw.c        |    6 ++++++
 net/ipv4/icmp.c      |    2 ++
 net/ipv4/ip_output.c |    2 ++
 net/ipv4/raw.c       |    1 +
 net/ipv4/udp.c       |    4 ++++
 6 files changed, 16 insertions(+), 0 deletions(-)

index bc026ec..9bc2b65 100644
@@ -55,6 +55,7 @@ struct ipcm_cookie
 	__be32			addr;
 	int			oif;
 	struct ip_options	*opt;
+	union sk_buff_hwtstamp	tstamp_tx;
 };
 
 #define IPCB(skb) ((struct inet_skb_parm*)((skb)->cb))

index 6e0663f..d4a38e3 100644
@@ -618,6 +618,7 @@ static int raw_sendmsg(struct kiocb *iocb, struct socket *sock,
 	struct raw_sock *ro = raw_sk(sk);
 	struct sk_buff *skb;
 	struct net_device *dev;
+	union sk_buff_hwtstamp tstamp_tx;
 	int ifindex;
 	int err;
 

@@ -639,6 +640,10 @@ static int raw_sendmsg(struct kiocb *iocb, struct socket *sock,
 	if (!dev)
 		return -ENXIO;
 
+	err = sock_tx_timestamp(msg, sk, &tstamp_tx);
+	if (err < 0)
+		return err;
+
 	skb = sock_alloc_send_skb(sk, size, msg->msg_flags & MSG_DONTWAIT,
 				  &err);
 	if (!skb) {

@@ -654,6 +659,7 @@ static int raw_sendmsg(struct kiocb *iocb, struct socket *sock,
 	}
 	skb->dev = dev;
 	skb->sk  = sk;
+	skb->hwtstamp = tstamp_tx;
 
 	err = can_send(skb, ro->loopback);
 

index 21e497e..ba739f4 100644
@@ -375,6 +375,7 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
 	inet->tos = ip_hdr(skb)->tos;
 	daddr = ipc.addr = rt->rt_src;
 	ipc.opt = NULL;
+	ipc.tstamp_tx.hwtstamp.tv64 = 0;
 	if (icmp_param->replyopts.optlen) {
 		ipc.opt = &icmp_param->replyopts;
 		if (ipc.opt->srr)

@@ -532,6 +533,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
 	inet_sk(sk)->tos = tos;
 	ipc.addr = iph->saddr;
 	ipc.opt = &icmp_param.replyopts;
+	ipc.tstamp_tx.hwtstamp.tv64 = 0;
 
 	{
 		struct flowi fl = {

index 46d7be2..1498848 100644
@@ -940,6 +940,7 @@ alloc_new_skb:
 			skb->ip_summed = csummode;
 			skb->csum = 0;
 			skb_reserve(skb, hh_len);
+			skb->hwtstamp = ipc->tstamp_tx;
 
 			/*
 			 *	Find where to start putting bytes.

@@ -1354,6 +1355,7 @@ void ip_send_reply(struct sock *sk, struct sk_buff *skb, struct ip_reply_arg *ar
 
 	daddr = ipc.addr = rt->rt_src;
 	ipc.opt = NULL;
+	ipc.tstamp_tx.hwtstamp.tv64 = 0;
 
 	if (replyopts.opt.optlen) {
 		ipc.opt = &replyopts.opt;

index 998fcff..9115ed5 100644
@@ -493,6 +493,7 @@ static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 
 	ipc.addr = inet->saddr;
 	ipc.opt = NULL;
+	ipc.tstamp_tx.hwtstamp.tv64 = 0;
 	ipc.oif = sk->sk_bound_dev_if;
 
 	if (msg->msg_controllen) {

index fea2d87..32c4e98 100644
@@ -573,6 +573,7 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 		return -EOPNOTSUPP;
 
 	ipc.opt = NULL;
+	ipc.tstamp_tx.hwtstamp.tv64 = 0;
 
 	if (up->pending) {
 		/*

@@ -620,6 +621,9 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 	ipc.addr = inet->saddr;
 
 	ipc.oif = sk->sk_bound_dev_if;
+	err = sock_tx_timestamp(msg, sk, &ipc.tstamp_tx);
+	if (err)
+		return err;
 	if (msg->msg_controllen) {
 		err = ip_cmsg_send(sock_net(sk), msg, &ipc);
 		if (err)

1.6.0.4

---
 fs/compat_ioctl.c |    1 +
 net/core/dev.c    |    2 ++
 2 files changed, 3 insertions(+), 0 deletions(-)

index 5235c67..a5001a6 100644
@@ -2555,6 +2555,7 @@ HANDLE_IOCTL(SIOCSIFMAP, dev_ifsioc)
 HANDLE_IOCTL(SIOCGIFADDR, dev_ifsioc)
 HANDLE_IOCTL(SIOCSIFADDR, dev_ifsioc)
 HANDLE_IOCTL(SIOCSIFHWBROADCAST, dev_ifsioc)
+HANDLE_IOCTL(SIOCSHWTSTAMP, dev_ifsioc)
 
 /* ioctls used by appletalk ddp.c */
 HANDLE_IOCTL(SIOCATALKDIFADDR, dev_ifsioc)

index b4b8eb8..4f61f5e 100644
@@ -3650,6 +3650,7 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
 			    cmd == SIOCSMIIREG ||
 			    cmd == SIOCBRADDIF ||
 			    cmd == SIOCBRDELIF ||
+			    cmd == SIOCSHWTSTAMP ||
 			    cmd == SIOCWANDEV) {
 				err = -EOPNOTSUPP;
 				if (dev->do_ioctl) {

@@ -3805,6 +3806,7 @@ int dev_ioctl(struct net *net, unsigned int cmd, void __user *arg)
 		case SIOCBONDCHANGEACTIVE:
 		case SIOCBRADDIF:
 		case SIOCBRDELIF:
+		case SIOCSHWTSTAMP:
 			if (!capable(CAP_NET_ADMIN))
 				return -EPERM;
 			/* fall through */

1.6.0.4

---
 drivers/net/igb/igb_main.c |   30 ++++++++++++++++++++++++++++++
 1 files changed, 30 insertions(+), 0 deletions(-)

index 89ffc07..be8e2b8 100644
@@ -34,6 +34,7 @@
 #include <linux/ipv6.h>
 #include <net/checksum.h>
 #include <net/ip6_checksum.h>
+#include <net/timestamping.h>
 #include <linux/mii.h>
 #include <linux/ethtool.h>
 #include <linux/if_vlan.h>

@@ -4115,6 +4116,33 @@ static int igb_mii_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 }
 
 /**
+ * igb_hwtstamp_ioctl - control hardware time stamping
+ * @netdev:
+ * @ifreq:
+ * @cmd:
+ *
+ * Currently cannot enable any kind of hardware time stamping, but
+ * supports SIOCSHWTSTAMP in general.
+ **/
+static int igb_hwtstamp_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
+{
+	struct hwtstamp_config config;
+
+	if (copy_from_user(&config, ifr->ifr_data, sizeof(config)))
+		return -EFAULT;
+
+	/* reserved for future extensions */
+	if (config.flags)
+		return -EINVAL;
+
+	if (config.tx_type == HWTSTAMP_TX_OFF &&
+		config.rx_filter_type == HWTSTAMP_FILTER_NONE)
+		return 0;
+
+	return -ERANGE;
+}
+
+/**
  * igb_ioctl -
  * @netdev:
  * @ifreq:

@@ -4127,6 +4155,8 @@ static int igb_ioctl(struct net_device *netdev, struct ifreq *ifr, int cmd)
 	case SIOCGMIIREG:
 	case SIOCSMIIREG:
 		return igb_mii_ioctl(netdev, ifr, cmd);
+	case SIOCSHWTSTAMP:
+		return igb_hwtstamp_ioctl(netdev, ifr, cmd);
 	default:
 		return -EOPNOTSUPP;
 	}

1.6.0.4

So far struct clocksource acted as the interface between time/timekeeping and hardware. This patch generalizes the concept so that the same interface can also be used in other contexts.

The only change as far as kernel/time/timekeeping is concerned is that the hardware access can be done either with or without passing the clocksource pointer as context. This is necessary in those cases when there is more than one instance of the hardware.

The extensions in this patch add code which turns the raw cycle count provided by hardware into a continously increasing time value. This reuses fields also used by timekeeping.c. Because of slightly different semantic (__get_nsec_offset does not update cycle_last, clocksource_read_ns does that transparently) timekeeping.c was not modified to use the generalized code.

The new code does no locking of the clocksource. This is the responsibility of the caller.

---
 include/linux/clocksource.h |  119 ++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 118 insertions(+), 1 deletions(-)

index f88d32f..5435bd2 100644
@@ -24,6 +24,9 @@ struct clocksource;
 /**
  * struct clocksource - hardware abstraction for a free running counter
  *	Provides mostly state-free accessors to the underlying hardware.
+ *      Also provides utility functions which convert the underlying
+ *      hardware cycle values into a non-decreasing count of nanoseconds
+ *      ("time").
  *
  * @name:		ptr to clocksource name
  * @list:		list head for registration

@@ -43,6 +46,9 @@ struct clocksource;
  *				The ideal clocksource. A must-use where
  *				available.
  * @read:		returns a cycle value
+ * @read_clock:         alternative to read which gets a pointer to the clock
+ *                      source so that the same code can read different clocks;
+ *                      either read or read_clock must be set
  * @mask:		bitmask for two's complement
  *			subtraction of non 64 bit counters
  * @mult:		cycle to nanosecond multiplier (adjusted by NTP)

@@ -62,6 +68,7 @@ struct clocksource {
 	struct list_head list;
 	int rating;
 	cycle_t (*read)(void);
+	cycle_t (*read_clock)(struct clocksource *cs);
 	cycle_t mask;
 	u32 mult;
 	u32 mult_orig;

@@ -170,7 +177,7 @@ static inline u32 clocksource_hz2mult(u32 hz, u32 shift_constant)
  */
 static inline cycle_t clocksource_read(struct clocksource *cs)
 {
-	return cs->read();
+	return (cs->read ? cs->read() : cs->read_clock(cs));
 }
 
 /**

@@ -190,6 +197,116 @@ static inline s64 cyc2ns(struct clocksource *cs, cycle_t cycles)
 }
 
 /**
+ * clocksource_read_ns - get nanoseconds since last call of this function
+ *                       (never negative)
+ * @cs:         Pointer to clocksource
+ *
+ * When the underlying cycle counter runs over, this will be handled
+ * correctly as long as it does not run over more than once between
+ * calls.
+ *
+ * The first call to this function for a new clock source initializes
+ * the time tracking and returns bogus results.
+ */
+static inline s64 clocksource_read_ns(struct clocksource *cs)
+{
+	cycle_t cycle_now, cycle_delta;
+	s64 ns_offset;
+
+	/* read clocksource: */
+	cycle_now = clocksource_read(cs);
+
+	/* calculate the delta since the last clocksource_read_ns: */
+	cycle_delta = (cycle_now - cs->cycle_last) & cs->mask;
+
+	/* convert to nanoseconds: */
+	ns_offset = cyc2ns(cs, cycle_delta);
+
+	/* update time stamp of clocksource_read_ns call: */
+	cs->cycle_last = cycle_now;
+
+	return ns_offset;
+}
+
+/**
+ * clocksource_init_time - initialize a clock source for use with
+ *                         %clocksource_read_time() and
+ *                         %clocksource_cyc2time()
+ * @cs:            Pointer to clocksource.
+ * @start_tstamp:  Arbitrary initial time stamp.
+ *
+ * After this call the current cycle register (roughly) corresponds to
+ * the initial time stamp. Every call to %clocksource_read_time()
+ * increments the time stamp counter by the number of elapsed
+ * nanoseconds.
+ */
+static inline void clocksource_init_time(struct clocksource *cs,
+					u64 start_tstamp)
+{
+	cs->cycle_last = clocksource_read(cs);
+	cs->xtime_nsec = start_tstamp;
+}
+
+/**
+ * clocksource_read_time - return nanoseconds since %clocksource_init_time()
+ *                         plus the initial time stamp
+ * @cs:          Pointer to clocksource.
+ *
+ * In other words, keeps track of time since the same epoch as
+ * the function which generated the initial time stamp. Don't mix
+ * with calls to %clocksource_read_ns()!
+ */
+static inline u64 clocksource_read_time(struct clocksource *cs)
+{
+	u64 nsec;
+
+	/* increment time by nanoseconds since last call */
+	nsec = clocksource_read_ns(cs);
+	nsec += cs->xtime_nsec;
+	cs->xtime_nsec = nsec;
+
+	return nsec;
+}
+
+/**
+ * clocksource_cyc2time - convert an absolute cycle time stamp to same
+ *                        time base as values returned by
+ *                        %clocksource_read_time()
+ * @cs:            Pointer to clocksource.
+ * @cycle_tstamp:  a value returned by cs->read()
+ *
+ * Cycle time stamps that are converted correctly as long as they
+ * fall into the time interval [-1/2 max cycle count, 1/2 cycle count],
+ * with "max cycle count" == cs->mask+1.
+ *
+ * This avoids situations where a cycle time stamp is generated, the
+ * current cycle counter is updated, and then when transforming the
+ * time stamp the value is treated as if it was in the future. Always
+ * updating the cycle counter would also work, but incurr additional
+ * overhead.
+ */
+static inline u64 clocksource_cyc2time(struct clocksource *cs,
+				cycle_t cycle_tstamp)
+{
+	u64 cycle_delta = (cycle_tstamp - cs->cycle_last) & cs->mask;
+	u64 nsec;
+
+	/*
+	 * Instead of always treating cycle_tstamp as more recent
+	 * than cs->cycle_last, detect when it is too far in the
+	 * future and treat it as old time stamp instead.
+	 */
+	if (cycle_delta > cs->mask / 2) {
+		cycle_delta = (cs->cycle_last - cycle_tstamp) & cs->mask;
+		nsec = cs->xtime_nsec - cyc2ns(cs, cycle_delta);
+	} else {
+		nsec = cyc2ns(cs, cycle_delta) + cs->xtime_nsec;
+	}
+
+	return nsec;
+}
+
+/**
  * clocksource_calculate_interval - Calculates a clocksource interval struct
  *
  * @c:		Pointer to clocksource.

1.6.0.4

Adds register definitions and a clocksource accessing the NIC time.

---
 drivers/net/igb/e1000_regs.h |   28 +++++++++++
 drivers/net/igb/igb.h        |    3 +
 drivers/net/igb/igb_main.c   |  105 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 136 insertions(+), 0 deletions(-)

index 95523af..37f9d55 100644
@@ -75,6 +75,34 @@
 #define E1000_FCRTH    0x02168  /* Flow Control Receive Threshold High - RW */
 #define E1000_RDFPCQ(_n)  (0x02430 + (0x4 * (_n)))
 #define E1000_FCRTV    0x02460  /* Flow Control Refresh Timer Value - RW */
+
+/* IEEE 1588 TIMESYNCH */
+#define E1000_TSYNCTXCTL 0x0B614
+#define E1000_TSYNCRXCTL 0x0B620
+#define E1000_TSYNCRXCFG 0x05F50
+
+#define E1000_SYSTIML 0x0B600
+#define E1000_SYSTIMH 0x0B604
+#define E1000_TIMINCA 0x0B608
+
+#define E1000_RXMTRL     0x0B634
+#define E1000_RXSTMPL 0x0B624
+#define E1000_RXSTMPH 0x0B628
+#define E1000_RXSATRL 0x0B62C
+#define E1000_RXSATRH 0x0B630
+
+#define E1000_TXSTMPL 0x0B618
+#define E1000_TXSTMPH 0x0B61C
+
+#define E1000_ETQF0   0x05CB0
+#define E1000_ETQF1   0x05CB4
+#define E1000_ETQF2   0x05CB8
+#define E1000_ETQF3   0x05CBC
+#define E1000_ETQF4   0x05CC0
+#define E1000_ETQF5   0x05CC4
+#define E1000_ETQF6   0x05CC8
+#define E1000_ETQF7   0x05CCC
+
 /* Split and Replication RX Control - RW */
 /*
  * Convenience macros

index 4ff6f05..2938ab3 100644
@@ -34,6 +34,8 @@
 #include "e1000_mac.h"
 #include "e1000_82575.h"
 
+#include <linux/clocksource.h>
+
 struct igb_adapter;
 
 #ifdef CONFIG_IGB_LRO

@@ -262,6 +264,7 @@ struct igb_adapter {
 	struct napi_struct napi;
 	struct pci_dev *pdev;
 	struct net_device_stats net_stats;
+	struct clocksource clock;
 
 	/* structs defined in e1000_hw.h */
 	struct e1000_hw hw;

1.6.0.4

Currently only mapping from clock source to system time is implemented. The interface could have been made more versatile by not depending on a clock source, but this wasn't done to avoid writing glue code elsewhere.

The method implemented here is the one used and analyzed under the name "assisted PTP" in the LCI PTP paper:
http://www.linuxclustersinstitute.org/conferences/archive/2008/PDF/Ohly_92221.pdf

---
 include/linux/clocksync.h |  139 +++++++++++++++++++++++++++++++++++++++++++++
 kernel/time/Makefile      |    2 +-
 kernel/time/clocksync.c   |  117 +++++++++++++++++++++++++++++++++++++
 3 files changed, 257 insertions(+), 1 deletions(-)
 create mode 100644 include/linux/clocksync.h
 create mode 100644 kernel/time/clocksync.c

new file mode 100644
index 0000000..e8c8fad
@@ -0,0 +1,139 @@
+/*
+ * Utility code which helps transforming between hardware time stamps
+ * generated by a clocksource and system time. The clocksource is
+ * assumed to return monotonically increasing time (but this code does
+ * its best to compensate if that is not the case) whereas system time
+ * may jump.
+ */
+#ifndef _LINUX_CLOCKSYNC_H
+#define _LINUX_CLOCKSYNC_H
+
+#include <linux/clocksource.h>
+#include <linux/ktime.h>
+
+/**
+ * struct clocksync - stores state and configuration for the two clocks
+ *
+ * Initialize to zero, then set clock, systime, num_samples.
+ *
+ * Transformation between HW time and system time is done with:
+ * HW time transformed = HW time + offset +
+ *                       (HW time - last_update) * skew / CLOCKSYNC_SKEW_RESOLUTION
+ *
+ * @clock:           the source for HW time stamps (%clocksource_read_time)
+ * @systime:         function returning current system time (ktime_get
+ *                   for monotonic time, or ktime_get_real for wall clock)
+ * @num_samples:     number of times that HW time and system time are to
+ *                   be compared when determining their offset
+ * @offset:          (system time - HW time) at the time of the last update
+ * @skew:            average (system time - HW time) / delta HW time *
+ *                   CLOCKSYNC_SKEW_RESOLUTION
+ * @last_update:     last HW time stamp when clock offset was measured
+ */
+struct clocksync {
+	struct clocksource *clock;
+	ktime_t (*systime)(void);
+	int num_samples;
+
+	s64 offset;
+	s64 skew;
+	u64 last_update;
+};
+
+/**
+ * CLOCKSYNC_SKEW_RESOLUTION - fixed point arithmetic scale factor for skew
+ *
+ * Usually one would measure skew in ppb (parts per billion, 1e9), but
+ * using a factor of 2 simplifies the math.
+ */
+#define CLOCKSYNC_SKEW_RESOLUTION (((s64)1)<<30)
+
+/**
+ * clocksync_hw2sys - transform HW time stamp into corresponding system time
+ * @sync:             context for clock sync
+ * @hwtstamp:         the result of %clocksource_read_time or
+ *                    %clocksource_cyc2time
+ */
+static inline ktime_t clocksync_hw2sys(struct clocksync *sync,
+					u64 hwtstamp)
+{
+	u64 nsec;
+
+	nsec = hwtstamp + sync->offset;
+	nsec += (s64)(hwtstamp - sync->last_update) * sync->skew /
+		CLOCKSYNC_SKEW_RESOLUTION;
+
+	return ns_to_ktime(nsec);
+}
+
+/**
+ * clocksync_offset - measure current (system time - HW time) offset
+ * @sync:             context for clock sync
+ * @offset:           average offset during sample period returned here
+ * @hwtstamp:         average HW time during sample period returned here
+ *
+ * Returns number of samples used. Might be zero (= no result) in the
+ * unlikely case that system time was monotonically decreasing for all
+ * samples (= broken).
+ */
+int clocksync_offset(struct clocksync *sync,
+		s64 *offset,
+		u64 *hwtstamp);
+
+/**
+ * clocksync_update - update offset and skew by measuring current offset
+ * @sync:             context for clock sync
+ * @hwtstamp:         the result of %clocksource_read_time or
+ *                    %clocksource_cyc2time, pass zero to force update
+ *
+ * Updates are only done at most once per second.
+ */
+static inline void clocksync_update(struct clocksync *sync,
+			u64 hwtstamp)
+{
+	s64 offset;
+	u64 average_time;
+
+	if (hwtstamp &&
+		(s64)(hwtstamp - sync->last_update) < NSEC_PER_SEC)
+		return;
+
+	if (!clocksync_offset(sync, &offset, &average_time))
+		return;
+
+	printk(KERN_DEBUG
+		"average offset: %lld\n", offset);
+
+	if (!sync->last_update) {
+		sync->last_update = average_time;
+		sync->offset = offset;
+		sync->skew = 0;
+	} else {
+		s64 delta_nsec = average_time - sync->last_update;
+
+		/* avoid division by negative or small deltas */
+		if (delta_nsec >= 10000) {
+			s64 delta_offset_nsec = offset - sync->offset;
+			s64 skew = delta_offset_nsec *
+				CLOCKSYNC_SKEW_RESOLUTION /
+				delta_nsec;
+
+			/**
+			 * Calculate new overall skew as 4/16 the
+			 * old value and 12/16 the new one. This is
+			 * a rather arbitrary tradeoff between
+			 * only using the latest measurement (0/16 and
+			 * 16/16) and even more weight on past measurements.
+			 */
+#define CLOCKSYNC_NEW_SKEW_PER_16 12
+			sync->skew =
+				((16 - CLOCKSYNC_NEW_SKEW_PER_16) * sync->skew +
+					CLOCKSYNC_NEW_SKEW_PER_16 * skew) /
+				16;
+			sync->last_update = average_time;
+			sync->offset = offset;
+		}
+	}
+}
+
+#endif /* _LINUX_CLOCKSYNC_H */

index 905b0b5..6279fb0 100644
@@ -1,4 +1,4 @@
-obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o
+obj-y += timekeeping.o ntp.o clocksource.o jiffies.o timer_list.o clocksync.o
 
 obj-$(CONFIG_GENERIC_CLOCKEVENTS_BUILD)		+= clockevents.o
 obj-$(CONFIG_GENERIC_CLOCKEVENTS)		+= tick-common.o

1.6.0.4

Both TX and RX hardware time stamping are implemented. Due to hardware limitations it is not possible to verify reliably which packet was time stamped when multiple were pending for sending; this could be solved by only allowing one packet marked for hardware time stamping into the queue (not implemented yet).

RX time stamping relies on the flag in the packet descriptor which marks packets that were time stamped. In "all packet" mode this flag is not set. TODO: also support that mode (even though it'll suffer from race conditions).

---
 drivers/net/igb/e1000_82575.h   |    1 +
 drivers/net/igb/e1000_defines.h |    1 +
 drivers/net/igb/e1000_regs.h    |   40 +++++++
 drivers/net/igb/igb.h           |    2 +
 drivers/net/igb/igb_main.c      |  239 +++++++++++++++++++++++++++++++++++++--
 5 files changed, 275 insertions(+), 8 deletions(-)

index c1928b5..dd32a6f 100644
@@ -116,6 +116,7 @@ union e1000_adv_tx_desc {
 };
 
 /* Adv Transmit Descriptor Config Masks */
+#define E1000_ADVTXD_MAC_TSTAMP   0x00080000 /* IEEE1588 Timestamp packet */
 #define E1000_ADVTXD_DTYP_CTXT    0x00200000 /* Advanced Context Descriptor */
 #define E1000_ADVTXD_DTYP_DATA    0x00300000 /* Advanced Data Descriptor */
 #define E1000_ADVTXD_DCMD_IFCS    0x02000000 /* Insert FCS (Ethernet CRC) */

index ce70068..2a19698 100644
@@ -104,6 +104,7 @@
 #define E1000_RXD_STAT_UDPCS    0x10    /* UDP xsum calculated */
 #define E1000_RXD_STAT_TCPCS    0x20    /* TCP xsum calculated */
 #define E1000_RXD_STAT_DYNINT   0x800   /* Pkt caused INT via DYNINT */
+#define E1000_RXD_STAT_TS       0x10000 /* Pkt was time stamped */
 #define E1000_RXD_ERR_CE        0x01    /* CRC Error */
 #define E1000_RXD_ERR_SE        0x02    /* Symbol Error */
 #define E1000_RXD_ERR_SEQ       0x04    /* Sequence Error */

index 37f9d55..7b561a1 100644
@@ -78,9 +78,37 @@
 
 /* IEEE 1588 TIMESYNCH */
 #define E1000_TSYNCTXCTL 0x0B614
+#define E1000_TSYNCTXCTL_VALID (1<<0)
+#define E1000_TSYNCTXCTL_ENABLED (1<<4)
 #define E1000_TSYNCRXCTL 0x0B620
+#define E1000_TSYNCRXCTL_VALID (1<<0)
+#define E1000_TSYNCRXCTL_ENABLED (1<<4)
+enum {
+	E1000_TSYNCRXCTL_TYPE_L2_V2 = 0,
+	E1000_TSYNCRXCTL_TYPE_L4_V1 = (1<<1),
+	E1000_TSYNCRXCTL_TYPE_L2_L4_V2 = (1<<2),
+	E1000_TSYNCRXCTL_TYPE_ALL = (1<<3),
+	E1000_TSYNCRXCTL_TYPE_EVENT_V2 = (1<<3) | (1<<1),
+};
 #define E1000_TSYNCRXCFG 0x05F50
+enum {
+	E1000_TSYNCRXCFG_PTP_V1_SYNC_MESSAGE = 0<<0,
+	E1000_TSYNCRXCFG_PTP_V1_DELAY_REQ_MESSAGE = 1<<0,
+	E1000_TSYNCRXCFG_PTP_V1_FOLLOWUP_MESSAGE = 2<<0,
+	E1000_TSYNCRXCFG_PTP_V1_DELAY_RESP_MESSAGE = 3<<0,
+	E1000_TSYNCRXCFG_PTP_V1_MANAGEMENT_MESSAGE = 4<<0,
 
+	E1000_TSYNCRXCFG_PTP_V2_SYNC_MESSAGE = 0<<8,
+	E1000_TSYNCRXCFG_PTP_V2_DELAY_REQ_MESSAGE = 1<<8,
+	E1000_TSYNCRXCFG_PTP_V2_PATH_DELAY_REQ_MESSAGE = 2<<8,
+	E1000_TSYNCRXCFG_PTP_V2_PATH_DELAY_RESP_MESSAGE = 3<<8,
+	E1000_TSYNCRXCFG_PTP_V2_FOLLOWUP_MESSAGE = 8<<8,
+	E1000_TSYNCRXCFG_PTP_V2_DELAY_RESP_MESSAGE = 9<<8,
+	E1000_TSYNCRXCFG_PTP_V2_PATH_DELAY_FOLLOWUP_MESSAGE = 0xA<<8,
+	E1000_TSYNCRXCFG_PTP_V2_ANNOUNCE_MESSAGE = 0xB<<8,
+	E1000_TSYNCRXCFG_PTP_V2_SIGNALLING_MESSAGE = 0xC<<8,
+	E1000_TSYNCRXCFG_PTP_V2_MANAGEMENT_MESSAGE = 0xD<<8,
+};
 #define E1000_SYSTIML 0x0B600
 #define E1000_SYSTIMH 0x0B604
 #define E1000_TIMINCA 0x0B608

@@ -103,6 +131,18 @@
 #define E1000_ETQF6   0x05CC8
 #define E1000_ETQF7   0x05CCC
 
+/* Filtering Registers */
+#define E1000_SAQF(_n) (0x5980 + 4 * (_n))
+#define E1000_DAQF(_n) (0x59A0 + 4 * (_n))
+#define E1000_SPQF(_n) (0x59C0 + 4 * (_n))
+#define E1000_FTQF(_n) (0x59E0 + 4 * (_n))
+#define E1000_SAQF0 E1000_SAQF(0)
+#define E1000_DAQF0 E1000_DAQF(0)
+#define E1000_SPQF0 E1000_SPQF(0)
+#define E1000_FTQF0 E1000_FTQF(0)
+#define E1000_SYNQF(_n) (0x055FC + (4 * (_n))) /* SYN Packet Queue Fltr */
+#define E1000_ETQF(_n)  (0x05CB0 + (4 * (_n))) /* EType Queue Fltr */
+
 /* Split and Replication RX Control - RW */
 /*
  * Convenience macros

index 2938ab3..86ef1a2 100644
@@ -35,6 +35,7 @@
 #include "e1000_82575.h"
 
 #include <linux/clocksource.h>
+#include <linux/clocksync.h>
 
 struct igb_adapter;
 

@@ -265,6 +266,7 @@ struct igb_adapter {
 	struct pci_dev *pdev;
 	struct net_device_stats net_stats;
 	struct clocksource clock;
+	struct clocksync sync;
 
 	/* structs defined in e1000_hw.h */
 	struct e1000_hw hw;

1.6.0.4

These patcehs add a lot of quite large inlined functions.

Right, I'll need to clean this up once it is clear which code is really going to be needed.

In this example, the check is going to avoid a function call when inlined in most of the cases. This was the motivation for making the function inline in the first place. The rest of it should be split of into a non-inline helper function. A "likely()" should be added, too.

Sorry for that, will do. Old habits die hard.

Okay.

Bye, Patrick

Hey Patrick,
Sorry for not noticing this thread earlier!

So as a heads up, the bit about passing the clocksource to the read_clock() function looks very similar to a bit of what Magnus Damm was recently working on.

Hrm.. I'm a little wary here. Your patch basically creates new semantics to how the clocksource structure is used, which will likely cause confusion. I'll agree that the clocksource structure has been somewhat more cluttered with timekeeping-isms then I'd prefer, so maybe your patches give us the need to clean it up and better separate the hardware clocksource accessor information and the timekeeping state.

So to be clear, let me see if I understand your needs from your patch:

1) Need an interface to a counter, who's interface monotonically increases.
2) Need to translate the counter to nanoseconds and nanoseconds back to the counter
3) The counter will likely not be registered for use in timekeeping
4) The counter's sense of time will not be steered via frequency adjustments.

Is that about the right set of assumptions?

So if we break the clocksource structure into two portions (ignore name deatils for now)

 strucut counter{
      char* name,
      u32 mult,
      u32 shift,
      cycle_t mask,
      cycle_t (*read)(struct counter*);
      cycle_t (*vread)(void);

     /* bits needed here for real monotonic interface, more on that below */

    /* other arch specific needs */
}

struct timeclock {
      struct counter* counter,
      u32 adjusted_mult,
      cycle_t cycle_last,
      u32 flags;
      u64 xtime_nsec;
      s64 error;
      /* other timekeeping bits go here */
}

So given that, do you think you'd be ok with using just the first counter structure?

Now there's sort of larger problem I've glossed over. Specifically in assumption #1 up there. The bit about the interface to the monotonic counter. Now many hardware counters wrap, and some wrap fairly quickly. This means we need to have some sort of infrastructure to periodically accumulate cycles into some "cycle store" value. As long as the cycle store is 64bits wide, we probably don't have to worry about overflows (if I recall 64bits at 1GHZ gives us ~500 years).

Now, currently the timekeeping core does this for the active in-use clocksource. However, if we have a number of counter structs that are being used in different contexts, maybe three registered for timekeeping, and a few more for different types of timestamping (maybe audio, networking, maybe even performance counters?), we suddenly have to do the accumulation step on a quite a few counters to avoid wrapping.

You dodged this accumulation infrastructure in your patch, by just accumulating at read time. This works, as long as you can guarantee that read events occur more often then the wrap frequency. And in most cases that's probably not too hard, but with some in-developement work, like the -rt patches, kernel work (even most interrupt processing) can be deferred by high priority tasks for an unlimited amount of time.

So this requires thinking this through maybe a bit more, trying to figure out how to create a guaranteed accumulation frequency, but only do so on counters that are really actively in use (we don't want to accumulate on counters that no one cares about). Its probably not too much work, but we may want to consider other approaches as well.

Another issue that multiple clocksources can cause, is dealing with time intervals between clocksources. Different clocksources may be driven by different crystals, so they will drift apart. Also since the clocksource used for timekeeping is adjusted by adjtimex(), you'll likely have to deal with small differences in system time intervals and clocksource time intervals.

I see you've maybe tried to address some of this with the following time_sync patch, but I'm not sure I've totally grokked that patch yet.

Anyway, sorry to ramble on so much. I'm really interested in your work, its really interesting! But we might want to make sure the right changes are being made to the right place so we don't get too much confusion with the same variables meaning different things in different contexts.

thanks
-john

No problem, it's not holding up anything. The question of how to extend skb hasn't been settled either. Thanks for taking the time to consider it.

That's true. I could keep the code separate, if that helps. I just didn't want to duplicate the whole structure definition.

There are two additional ways of using the counter:
* Get nanosecond delay measurements (clocksource_read_ns). Calling this "resets" the counter.
* Get a continously increasing timer value (clocksource_init_time/clocksource_read_time). The clock is only reset when calling clocksource_init_time().

The two are mutually exclusive because clocksource_read_time() depends on clocksource_read_ns(). If this is too confusing, then clocksource_read_ns() could be turned into an internal helper function. I left it in the header because there might be other uses for it. The rest of the patches only needs clocksource_read_time().

Nanoseconds never have to be converted back to the counter. That wouldn't be possible anyway (hardware counter might roll over, whereas the clock counts nanoseconds in a 64 bit value and thus will last longer than the hardware it runs on).

About right ;-)

Some additional members must be moved to struct counter:
* cycle_last (for the overflow handling)
* xtime_nsec (for the continously increasing timer)

Except from those the first struct is okay.

[...]

Exactly. My plan was that the user of such a custom clocksource is responsible for querying it often enough so that clocksource_read_ns() can detect the wrap around. This works in the context of PTP (which causes regular events). Network driver developers must be a bit careful when there is no active PTP daemon: either they reinitialize the timer when it starts to get used or probe it automatically after certain delays.

I'm not sure what can be done in such a case. Use decent hardware which doesn't wrap around so quickly, I guess. It's not an issue with the Intel NIC (sorry for the advertising... ;-)

The clocksource API extension and the time sync code are independent at the moment: the time sync code assumes that it gets two, usually increasing timer values and tries to match them by measuring skew and drift between them. If the timer values jump, then the sync code adapts these values accordingly.

I don't think it will be necessary to add something like adjtimex() to a clocksource. Either the hardware supports it natively (like the Intel NIC does, sorry again), or the current time sync deals with frequency changes by adapting the drift factor.

Thanks for your comments. I agree that splitting the structures would help. But the variables really have the same meaning. They are just used in different functions.

Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

I think either keeping it separate, using your own structure, or properly splitting out the counter / time-clock interface would be the way to go.

Just so I understand, do you mean clocksource_read_ns() returns the number of nanoseconds since the last call to clocksource_read_ns() ?

That seems like an odd interface to define, since effectively you're storing state inside the interface.

Why exactly is this useful, as opposed to creating a monotonically increasing function which can be sampled and the state is managed by the users of the interface?

So a monotonic 64bit wide counter. Close to what I described above. Is there actually a need for it to reset ever?

Yea. It seems like an odd interface, as the internal state seems to limit its use.

Right, but if its a monotonically increasing 64bit counter, rollover isn't likely an issue. I think we're basically communicating the same idea here, just the question is do you want to make the interface provide nanoseconds or cycles.

Hmm. I'd still prefer those values to be stored elsewhere. As you add state to the structure, that limits how the structure can be used. For instance, if cycles_last and xtime_nsec are in the counter structure, then that means one counter could not be used for both timekeeping and the hardware time-stamping you're doing.

Instead that state should be stored in the timekeeping and timestamping structures respectively.

Right, however my point quoted below was that this will likely break in the -rt kernel, since those users may be deferred for a undefined amount of time. So we'll need to do something here.

Well, I think it would be good to create a infrastructure that will work on most hardware.

And I think it can work, but in order to make it work cleanly, we'll have to have some form of accumulation infrastructure, which will not be able to be deferred.

However, some careful thought will be needed here, so that we don't create latencies by wasting time sampling unused hardware counters in the hardirq context.

Ok. I'll have to spend some more time on that patch, but it sounds like you're handling the issue.

Err, you might be misunderstanding their current meaning. However, its not your fault, as the naming is not as clear as I like. For instance, xtime_nsec stores the sub-nanoseconds (shifted up by clocksource->shift) not represented in the xtime value.

So yes, while you likely want to keep similar state as the timekeeping core does, I really think splitting it out fully is going to be the way to go.

Thanks for the consideration of my comments! I look forward to your future patches!
-john

Okay, will do that. I'll try to do it so that later the clocksource can be rewritten so that it uses the same definition.

Yes.

The monotonically increasing function already is based on a stateful function which calculcates the delta; calculating the original delta based on a derived value didn't seem right. But I don't really care much about this part of the API, so I'll just make it internal.

Perhaps. A device might decide to reset the time each time hardware time stamping is activated.

The clean solution would be
* struct cyclecounter: abstract API to access hardware cycle counter The cycle counter may roll over relatively quickly. The implementor needs to provide information about the width of the counter and its frequency.
* struct timecounter: turns cycles from one cyclecounter into a nanosecond count
Must detect and deal with cycle counter overflows. Uses a 64 bit counter for time, so it itself doesn't overflow (unless we build hardware that runs for a *really* long time).

Now, should struct timecounter contain a struct cyclecounter or a pointer to it? A pointer is more flexible, but overkill for the usage I had in mind. I'll use a pointer anyway, just in case.

I'm not sure whether timestamping can be separated from timekeeping: it depends on the same cycle counter state as the timekeeping.

If the code isn't called often enough to deal with the regular PTP Sync messages (sent every two seconds), then such a system would already have quite a few other problems.

Most hardware doesn't have hardware time stamping. Is there any hardware which has hardware time stamping, but only with such a limited counter that we run into this problem?

I agree that this problem needs to be taken into account now (while designing these data structures) and be addressed as soon as it becomes necessary - but not sooner. Otherwise we might end up with dead code that isn't used at all.

Currently the structures are owned by the device driver which owns the hardware. Perhaps the device driver could register the structure with such an accumulation infrastructure if the driver itself cannot guarantee that it will check the cycle counter often enough. Concurrent access to the cycle counter hardware and state could make this tricky.

This goes into areas where I have no experience at all, so I would depend on others to provide that code.

Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

[...]

Before someone else mentions it: this was meant to be "sizeof(*serr)" of course.

Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

Hello Patrick,

(..)

This looks really better to me :-)

I still have two questions of understanding regarding these (offset) transformations that make it still difficult:

1. As no one has an insight of how the specific hardware generates the hw time stamp anyway, why can't you put the already transformed values into the hw timestamp field? I won't stick on the fact that the hw time stamp is the value that has been read from specific controller registers that are very hardware depended.

2. From what i've read in the discussion, i understood that the hardware clock and the system clock skew. Assuming both to be strong monotonic(?), is there a linear skew observable from your testing experiences?

If so i would suggest to have some kind of sysfs entry like e.g.

/sys/class/net/eth0/hwstamp_skew_ns

that gives something like

-86234765@12277656012584334095

as output. In general:

<skew_in_ns>@<systemtime>

So if you would check this sysfs entry two or more times, you would get an impression about the hw time stamp behaviour of your hardware by simply calculating the linear (timedepended) offset based on several 'skew-sample-points', right?

I hope these suggestions are not completely damaged ;-)

Best regards,
Oliver

Good, so we are making progress :-)

It certainly would be sufficient for PTP with the "assisted" PTP mode of synchronizing system time. But my understanding is that Octavian and possibly others want to have access to the raw, unmodified hardware time stamps. Octavian, perhaps you can confirm/elaborate what your use case is?

I should point out that currently the "raw" hardware time stamp is already "cooked" a bit in the sense that the new field is spec'ed to contain nanoseconds, not some opaque 64 bit blob. This is intentional because even though the time when the clock started to run is unknown, at least deltas can be calculated. The expectation is that this delta is close to world and system time, with some inaccuracies caused by using different crystals to drive the underlying hardware and thus varying frequencies.

The "two-way" PTP mode could use these values to synchronize NIC clocks (using a yet to be defined ioctl call which speeds up or slows down NIC clocks, like adjtimex does for system time) and system time on top of that NIC time. At the NIC level synchronization is going to be more accurate than at the system level because it removes the NIC/system time comparison from the equation. I believe this is similar to what Octavian wants to do.

The clock drift changes over time, so on a big scale the skew is not linear. For short periods (minute range) it is reasonable to assume that the drift is constant, so a linear interpolation works reasonably well. For your proposal that means that the user application needs to reread the sysfs file at least once in a while, without really knowing how often that should be.

[sysfs entry which exposes skew for interface]

This would work, but it exposes internal mechanisms (skew measurement) to user space. If a driver developer has a better method that is more suitable for his hardware (perhaps something non-linear or based on more frequent skew measurements), then the application doesn't benefit from it.

To summarize, I see the following options at this time:

1 only store transformed time stamp...
1a ... without mapping back to raw hardware time stamp
(advantage: simple implementation,
kernel itself can compare hardware time stamps from different NICs;
disadvantage: not suitable for some use cases)
1b ... with backward mapping at socket level (advantage: user space has
full access to whatever it needs; disadvantage: backward transformation most likely inexact, correct way of accessing network device from socket unknown)
1c ... with extra user space API for transformation (advantage:
side-steps the implementation issues in the kernel; disadvantage:
biggger user space API, exposes internals)

2 only store raw time stamp...
2a ... without transformation (disadvantage: not good enough for PTP) 2b ... with transformation to system time at socket level (advantage:
no information lost due to transformation; disadvantage: same implementation issues as 1b)
2c ... with extra user space API for transformation (similar to 1c)

3 store both raw and transformed time stamps in skb (advantage: no
extra APIs (in kernel or to user space) necessary for transformation,
disadvantage: 16 additional bytes instead of just 8)

4 a compile-time configuration option which chooses between 1a (only
store transformed time, should be the default) and 3 (store both)

My personal preference is, in this order: 3, 4, 2b (current patch, but needs clean way to find network device), 1a.

Any other opinions?

Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

From: Patrick Ohly <[email protected]>
Date: Thu, 27 Nov 2008 11:07:07 +0100

We use the hardware timestamp to determine one-way delay:
- hardware synchronize the NIC clocks (20ns resolution)
- insert the hw timestamp in the packet as its going out the wire (this is done in hw)
- received packets will be tagged with a hardware timestamp

One way delay is computed by subtracting the TX hw timestamp from the RX hw timestamp.

[snip]

I also vote for 3 (storing hw timestamps in the skb).

Let me throw in another idea: when enabling hw timestamps could we allocate a bigger skb and store the hw timestamp somewhere in the skb data buffer?

We can then modify sock_recv_timestamp to call a new netdev method which should return the hw timestamp. This should take care of RX hw timestamps.

For TX, we can we get a new skb with skb_copy_expand to make room for the TX timestamp, and queue that in the error socket.

There is still the problem of requesting TX timestamps per packets. At this point it seems that on the TX path the tstamp field is not used, so we could use that space.

Or, maybe we can use the same dynamic approach: can we modify the hard_header_len after device registration (e.g. when TX timestamps are enabled)?

Thanks,
tavi

How does the socket layer detect that the HW timestamp is available in the larger skb data buffer, and where?

Finding the netdev is non-trivial, see David's comment about the current hacky approach via the route. Besides, if the time stamp is in the skb data buffer, why is the netdev still needed? Do I miss something?

Agreed in general, but there was one corner case where the tstamp field was set for looped multicast packets.

I don't know.

Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

From: Patrick Ohly <[email protected]>
Date: Thu, 27 Nov 2008 16:31:07 +0100

It doesn't know, thats why we need help from the device (the new netdev method).

OK I see now, it seems that we can't get to the device in a generic way.

How about this twist: we add a new option at the socket level, to get the whole skb->head - skb->end data into a user buffer. Then, we call an device ioctl and pass this buffer. The device will extract the hw timestamp and give it to the user.

We might not need to get the whole skb->head - skb->end buffer, maybe just skb-

use the convention that the device driver must put the timestamp below the mac header.

One potential problem I see with this approach is leaking sensitive information into userspace, which means we will have to restrict this to privileged processes only.

tavi

4 would be mine.

I assume, we would get kicked somewhere when we are trying to push *another* 8 bytes into the skb by default ;-]

Ugh.

Not every protocol that uses skbuffs, has a mac header (e.g. the CAN protocol doesn't have mac addresses). This twist does not look very maintainable to me ...

One additional question for Patrick:
As you wrote that your hw timestamp contained in the new skbuff-field is already cocked ... is there any identifier that tells the userspace application about the type of hw timestamp he gets (e.g. cocked, raw registers, offset to whatever, etc.) ?

Regards,
Oliver

From: Oliver Hartkopp <[email protected]>
Date: Thu, 27 Nov 2008 23:13:08 +0100

OK, then what about this: we use a device ioctl to get the number of bytes to copy from skb->head. We then pass this to the socket level option.

This is more complicated than option 3 or 4, but it should address the concerns raised here - no performance impact when not using this feature. The trade-off is moving work from core kernel into userspace and device driver.

Could you elaborate on the maintainability issues, they are not clear to me.

Thanks,
tavi

Do you know what all the people are doing with skbuffs in the kernel? I'm not aware of it.
So whenever we touch a vital data structure like the skbuff we must be sure that our handling is a wise approach for all.

Btw. your answer brought me to a completely different approach:

What about just creating a new pointer in the struct skbuff that points to a struct hwstamp when it is available OR the pointer is NULL when no hwstamps are available.

This struct hwstamp may consist of 'everthing we really need for timestamping' and is added somewhere at the head or the tail of the skbuff data section by the device driver. And if the socket sees this pointer to be not NULL it knows where to look for our fancy struct hwstamp ...

I don't know, where this hwstamp data could be pasted into the data section in the best way - but surely others know. Especially this approach meets the requirement that the additional hwstamp data is only allocated (per interface!), when it's really needed - and not for everyone by default.

What do you thing about this idea?

Regards,
Oliver

From: Oliver Hartkopp <[email protected]>
Date: Fri, 28 Nov 2008 16:38:16 +0100

I agree. But what in this approach do you think its not wise? :)

The skb is not touched at all, we just need to allocate a larger skb when using hw timestamps (or other extensions) and reserve some portion of it.

That is still adding a member into the skb. For what I am proposing we don't have to touch the skb and it has all of your approach benefits.

I would rather use this approach since it allows us to "extend" the skb in this manner int the future for other hardware features which requires passing per-skb information to userspace.

Thanks,
tavi

That just pushes the problem into the device driver: when it gets passed a skb pointer, how can it tell reliably that the data buffer contains the HW timestamping information?

I agree with Oliver, this approach doesn't look maintainable to me. I don't know enough about the kernel to tell whether it works reliably at the moment, much less whether it will work in the future.

Oliver asked:

In the proposed API the userspace application gets three time stamps:
software, "cooked" hardware time stamp (converted to nanoseconds by the driver, but not tampered with in any other way), hardware time stamp converted to system time. Each of these may be missing (not available, couldn't be calculated). So yes, the userspace application knows what it got and can pick the value that it needs.

Oliver suggested:

I like this better than tampering with the data buffer pointers implicitly because it enables usages of the additional information inside the kernel itself. It's similar to skb_shared_info, except that it is not allocated for all skbs.

The skb_shared_info is always at the end of the data buffer. Assume that we have a new __netdev_alloc_hw_skb() which increases the allocated data buffer to make room for the additional struct hwtstamp (either before skb_shared_info or after). I cannot think of a way how the rest of the kernel can tell that this additional data is available by just looking at the existing head/data/end fields in a skb - if I missed something, please let me know.

So it seems to me that we need the additional 32 bit offset (or pointer, on 32 bit architectures) in skb which points towards the struct hwtstamp. But that's actually less than the additional 64 bit which hold the time stamp value, as in the current patch. I'll give it a few more days for further debate, then try out this approach.

Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

[...]

It doesn't even need a 32 bit offset. If it is always at the same location in the buffer (e.g., directly after skb_shared_info), then a single bit is sufficient. The same mechanism could be used to also store other optional structs/fields.

Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.

That's really fine. Sorry that i did not go that deep into your code myself :-]

I'm not very familiar with skb fields but hiding this with a new __netdev_alloc_hw_skb() looks very good to me and creates a proper way to add hw-specific netdevice information in a generic way and - that's probably the best news - only when it's really needed.

I'll be on a business trip until Thursday, so from my side you would get a 'go ahead' right now ;-)

Best regards,
Oliver