netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH 0/2] net: support for TX timestamps
@ 2008-07-29  0:35 Octavian Purdila
  2008-07-29  0:35 ` [RFC][PATCH 1/2] " Octavian Purdila
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Octavian Purdila @ 2008-07-29  0:35 UTC (permalink / raw)
  To: netdev

Hi everybody,

This patches adds TX timestamping support and is main use case is the
Precision Time Protocol (IEEE 1588). Both software and hardware
timestamping can be used.

Thanks,
tavi



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

* [RFC][PATCH 1/2] net: support for TX timestamps
  2008-07-29  0:35 [RFC][PATCH 0/2] net: support for TX timestamps Octavian Purdila
@ 2008-07-29  0:35 ` Octavian Purdila
  2008-07-30 12:43   ` Patrick Ohly
  2008-07-29  0:35 ` [RFC][PATCH 2/2] ip: support for TX timestamps on UDP and RAW sockets Octavian Purdila
  2008-07-30  9:47 ` [RFC][PATCH 0/2] net: support for TX timestamps Ingo Oeser
  2 siblings, 1 reply; 9+ messages in thread
From: Octavian Purdila @ 2008-07-29  0:35 UTC (permalink / raw)
  To: netdev; +Cc: Octavian Purdila

We make use of the first 16 MSB bits of the skb->tstamp to send
timestamping commands to low layers of the networking stack or network
device drivers.

When a TX timestamp operation is requested, the TX skb will be cloned
and the clone will be timestamped and added to the socket error queue
of the skb, if the skb has a socket associated.

The actual 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.

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 include/asm-powerpc/socket.h |    3 +++
 include/linux/skbuff.h       |    3 +++
 include/net/sock.h           |    9 +++++++++
 include/net/tstamps.h        |   11 +++++++++++
 net/core/dev.c               |   17 +++++++++++++++--
 net/core/skbuff.c            |   33 +++++++++++++++++++++++++++++++++
 net/core/sock.c              |    6 ++++++
 net/socket.c                 |   23 +++++++++++++++++++++++
 8 files changed, 103 insertions(+), 2 deletions(-)

diff --git a/include/asm-powerpc/socket.h b/include/asm-powerpc/socket.h
index be304b6..26a232f 100644
--- a/include/asm-powerpc/socket.h
+++ b/include/asm-powerpc/socket.h
@@ -64,4 +64,7 @@
 #define SO_TIMESTAMPHW		37
 #define SCM_TIMESTAMPHW		SO_TIMESTAMPHW
 
+#define SO_TXTIMESTAMP		38
+#define SCM_TXTIMESTAMP		SO_TXTIMESTAMP
+
 #endif	/* _ASM_POWERPC_SOCKET_H */
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index f19ed43..39b3ffd 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -1697,5 +1697,8 @@ static inline void skb_forward_csum(struct sk_buff *skb)
 }
 
 bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off);
+
+int skb_tx_timestamp(struct sk_buff *orig_skb, ktime_t tstamp);
+
 #endif	/* __KERNEL__ */
 #endif	/* _LINUX_SKBUFF_H */
diff --git a/include/net/sock.h b/include/net/sock.h
index 7ff1d02..784af54 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -415,6 +415,7 @@ enum sock_flags {
 	SOCK_LOCALROUTE, /* route locally only, %SO_DONTROUTE setting */
 	SOCK_QUEUE_SHRUNK, /* write queue has been shrunk recently */
 	SOCK_RCVTSTAMPHW, /* %SO_TIMESTAMPHW setting */
+	SOCK_TXTSTAMP, /* %SO_TXTIMESTAMP setting */
 };
 
 static inline void sock_copy_flags(struct sock *nsk, struct sock *osk)
@@ -1157,6 +1158,11 @@ static inline int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)
 	return 0;
 }
 
+static inline void __net_tx_timestamp(struct sk_buff *skb)
+{
+	skb_tx_timestamp(skb, ktime_get_real());
+}
+
 /*
  *	Recover an error report and clear atomically
  */
@@ -1263,6 +1269,9 @@ sock_recv_timestamp(struct msghdr *msg, struct sock *sk, struct sk_buff *skb)
 		sk->sk_stamp = kt;
 }
 
+extern int sock_send_timestamp(ktime_t *tstamp, struct msghdr *msg,
+			       struct sock *sk);
+
 /**
  * sk_eat_skb - Release a skb if it is no longer needed
  * @sk: socket to eat this skb from
diff --git a/include/net/tstamps.h b/include/net/tstamps.h
index 9e02673..e2faced 100644
--- a/include/net/tstamps.h
+++ b/include/net/tstamps.h
@@ -1,6 +1,17 @@
 #ifndef _NET_TIMESTAMPS_H
 #define _NET_TIMESTAMPS_H
 
+/*
+ * Timestamps for outgoing skbs have special meaning:
+ *
+ * - MSB bits 0-15: command flags (take timestamp, embed timestamp, schedule
+ * packet, etc.)
+ * - other bits: command specific
+ */
+
+/* request TX timestamping */
+#define SKB_TSTAMP_TX_STAMP			(0x8000000000000000ULL)
+
 #ifdef __KERNEL__
 
 #include <linux/skbuff.h>
diff --git a/net/core/dev.c b/net/core/dev.c
index fca23a3..40fd301 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1243,6 +1243,14 @@ static inline void net_timestamp(struct sk_buff *skb)
 		skb->tstamp.tv64 = 0;
 }
 
+static inline void net_tx_timestamp(struct sk_buff *skb)
+{
+	/* TX timestamps reach userspace only if RX timestamps are enabled */
+	if (atomic_read(&netstamp_needed) &&
+	    (skb->tstamp.tv64 & SKB_TSTAMP_TX_STAMP))
+		__net_tx_timestamp(skb);
+}
+
 /*
  *	Support routine. Sends outgoing frames to any network
  *	taps currently in use.
@@ -1568,6 +1576,8 @@ static int dev_gso_segment(struct sk_buff *skb)
 
 int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
 {
+	int rc;
+
 	if (likely(!skb->next)) {
 		if (!list_empty(&ptype_all))
 			dev_queue_xmit_nit(skb, dev);
@@ -1579,13 +1589,15 @@ 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);
+		if (likely(!rc))
+			net_tx_timestamp(skb);
+		return rc;
 	}
 
 gso:
 	do {
 		struct sk_buff *nskb = skb->next;
-		int rc;
 
 		skb->next = nskb->next;
 		nskb->next = NULL;
@@ -1595,6 +1607,7 @@ gso:
 			skb->next = nskb;
 			return rc;
 		}
+		net_tx_timestamp(skb);
 		if (unlikely((netif_queue_stopped(dev) ||
 			     netif_subqueue_stopped(dev, skb)) &&
 			     skb->next))
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 3666216..0a5b49e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2565,6 +2565,39 @@ int skb_cow_data(struct sk_buff *skb, int tailbits, struct sk_buff **trailer)
 }
 
 /**
+ * skb_tx_timestamp - timestamp a transmitted skb
+ * @orig_skb: the skb to be timestamped
+ * @tstamp: the value of the timestamp
+ *
+ * This function will not actually timestamp the skb, but, if the skb has a
+ * socket associated, clone the skb, timestamp it, and queue it to the error
+ * queue of the socket.
+ */
+int skb_tx_timestamp(struct sk_buff *orig_skb, ktime_t tstamp)
+{
+	struct sock *sk = orig_skb->sk;
+	struct sk_buff *skb;
+	int err = -ENOMEM;
+
+	if (!sk)
+		return -ENOTSOCK;
+
+	skb = skb_clone(orig_skb, GFP_ATOMIC);
+	if (!skb)
+		return err;
+
+	skb->tstamp = tstamp;
+
+	err = sock_queue_err_skb(sk, skb);
+	if (err)
+		kfree_skb(skb);
+
+	return err;
+}
+EXPORT_SYMBOL(skb_tx_timestamp);
+
+
+/**
  * skb_partial_csum_set - set up and verify partial csum values for packet
  * @skb: the skb to set
  * @start: the number of bytes after skb->data to start checksumming.
diff --git a/net/core/sock.c b/net/core/sock.c
index d6f05bb..6fa8cbc 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -628,6 +628,12 @@ set_rcvbuf:
 
 		break;
 
+	case SO_TXTIMESTAMP:
+		if (valbool)
+			sock_set_flag(sk, SOCK_TXTSTAMP);
+		else
+			sock_reset_flag(sk, SOCK_TXTSTAMP);
+
 	case SO_RCVLOWAT:
 		if (val < 0)
 			val = INT_MAX;
diff --git a/net/socket.c b/net/socket.c
index 40bc098..4dbc7b6 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -543,6 +543,29 @@ void sock_release(struct socket *sock)
 	sock->file = NULL;
 }
 
+int sock_send_timestamp(ktime_t *tstamp, struct msghdr *msg, struct sock *sk)
+{
+	struct cmsghdr *cmsg;
+
+	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
+		if (!CMSG_OK(msg, cmsg))
+			return -EINVAL;
+		if (cmsg->cmsg_level != SOL_SOCKET ||
+		    cmsg->cmsg_type != SCM_TXTIMESTAMP ||
+		    cmsg->cmsg_len != CMSG_LEN(sizeof(__u64)))
+			continue;
+
+		*tstamp = *((ktime_t *)CMSG_DATA(cmsg));
+		break;
+	}
+
+	if (sk && sock_flag(sk, SOCK_TXTSTAMP))
+		tstamp->tv64 |= SKB_TSTAMP_TX_STAMP;
+
+	return 0;
+}
+EXPORT_SYMBOL(sock_send_timestamp);
+
 static inline int __sock_sendmsg(struct kiocb *iocb, struct socket *sock,
 				 struct msghdr *msg, size_t size)
 {
-- 
1.5.6.2


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

* [RFC][PATCH 2/2] ip: support for TX timestamps on UDP and RAW sockets
  2008-07-29  0:35 [RFC][PATCH 0/2] net: support for TX timestamps Octavian Purdila
  2008-07-29  0:35 ` [RFC][PATCH 1/2] " Octavian Purdila
@ 2008-07-29  0:35 ` Octavian Purdila
  2008-07-30  9:47 ` [RFC][PATCH 0/2] net: support for TX timestamps Ingo Oeser
  2 siblings, 0 replies; 9+ messages in thread
From: Octavian Purdila @ 2008-07-29  0:35 UTC (permalink / raw)
  To: netdev; +Cc: Octavian Purdila

Signed-off-by: Octavian Purdila <opurdila@ixiacom.com>
---
 include/net/ip.h     |    1 +
 net/can/raw.c        |    8 ++++++++
 net/ipv4/icmp.c      |    2 ++
 net/ipv4/ip_output.c |    2 ++
 net/ipv4/raw.c       |    1 +
 net/ipv4/udp.c       |    4 ++++
 6 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/include/net/ip.h b/include/net/ip.h
index 3b40bc2..b557a7a 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -54,6 +54,7 @@ struct ipcm_cookie
 	__be32			addr;
 	int			oif;
 	struct ip_options	*opt;
+	ktime_t			tstamp;
 };
 
 #define IPCB(skb) ((struct inet_skb_parm*)((skb)->cb))
diff --git a/net/can/raw.c b/net/can/raw.c
index 3e46ee3..e1dff8a 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -618,6 +618,9 @@ 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;
+	ktime_t tstamp = {
+		.tv64 = 0.
+	};
 	int ifindex;
 	int err;
 
@@ -639,6 +642,10 @@ static int raw_sendmsg(struct kiocb *iocb, struct socket *sock,
 	if (!dev)
 		return -ENXIO;
 
+	err = sock_send_timestamp(&tstamp, msg, sk);
+	if (err < 0)
+		return err;
+
 	skb = sock_alloc_send_skb(sk, size, msg->msg_flags & MSG_DONTWAIT,
 				  &err);
 	if (!skb) {
@@ -654,6 +661,7 @@ static int raw_sendmsg(struct kiocb *iocb, struct socket *sock,
 	}
 	skb->dev = dev;
 	skb->sk  = sk;
+	skb->tstamp = tstamp;
 
 	err = can_send(skb, ro->loopback);
 
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 8739735..4d65f57 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -377,6 +377,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.tv64 = 0;
 	if (icmp_param->replyopts.optlen) {
 		ipc.opt = &icmp_param->replyopts;
 		if (ipc.opt->srr)
@@ -534,6 +535,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.tv64 = 0;
 
 	{
 		struct flowi fl = {
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index e527628..08be117 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -941,6 +941,7 @@ alloc_new_skb:
 			skb->ip_summed = csummode;
 			skb->csum = 0;
 			skb_reserve(skb, hh_len);
+			skb->tstamp = ipc->tstamp;
 
 			/*
 			 *	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.tv64 = 0;
 
 	if (replyopts.opt.optlen) {
 		ipc.opt = &replyopts.opt;
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index 37a1ecd..0719f88 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -494,6 +494,7 @@ static int raw_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 
 	ipc.addr = inet->saddr;
 	ipc.opt = NULL;
+	ipc.tstamp.tv64 = 0;
 	ipc.oif = sk->sk_bound_dev_if;
 
 	if (msg->msg_controllen) {
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 56fcda3..aea821f 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -560,6 +560,7 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 		return -EOPNOTSUPP;
 
 	ipc.opt = NULL;
+	ipc.tstamp.tv64 = 0;
 
 	if (up->pending) {
 		/*
@@ -608,6 +609,9 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
 
 	ipc.oif = sk->sk_bound_dev_if;
 	if (msg->msg_controllen) {
+		err = sock_send_timestamp(&ipc.tstamp, msg, sk);
+		if (err)
+			return err;
 		err = ip_cmsg_send(sock_net(sk), msg, &ipc);
 		if (err)
 			return err;
-- 
1.5.6.2


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

* Re: [RFC][PATCH 0/2] net: support for TX timestamps
  2008-07-29  0:35 [RFC][PATCH 0/2] net: support for TX timestamps Octavian Purdila
  2008-07-29  0:35 ` [RFC][PATCH 1/2] " Octavian Purdila
  2008-07-29  0:35 ` [RFC][PATCH 2/2] ip: support for TX timestamps on UDP and RAW sockets Octavian Purdila
@ 2008-07-30  9:47 ` Ingo Oeser
  2008-07-30 13:02   ` Octavian Purdila
  2 siblings, 1 reply; 9+ messages in thread
From: Ingo Oeser @ 2008-07-30  9:47 UTC (permalink / raw)
  To: Octavian Purdila; +Cc: netdev

Hi Octavian,

Octavian Purdila schrieb:
> This patches adds TX timestamping support and is main use case is the
> Precision Time Protocol (IEEE 1588). Both software and hardware
> timestamping can be used.

Could you please include an example on how to use
this in userspace?

A proper place for this would be Documentation/net/

At the moment I cannot imagine how to use this and how
to correlate the sent packet with the timestamp.

That would also be a way to test your implementation and
test any changes done later to it. This will avoid future bitrot.

Many Thanks!


Best Regards

Ingo Oeser

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

* Re: [RFC][PATCH 1/2] net: support for TX timestamps
  2008-07-29  0:35 ` [RFC][PATCH 1/2] " Octavian Purdila
@ 2008-07-30 12:43   ` Patrick Ohly
  2008-07-30 13:15     ` Octavian Purdila
  0 siblings, 1 reply; 9+ messages in thread
From: Patrick Ohly @ 2008-07-30 12:43 UTC (permalink / raw)
  To: Octavian Purdila; +Cc: netdev

On Tue, 2008-07-29 at 03:35 +0300, Octavian Purdila wrote:
> The actual 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.

This needs to be augmented to not fall back to software time stamping,
as discussed in "[RFC][PATCH 1/1] net: support for hardware
timestamping".

Apart from that the API looks okay. It should be fairly simple to adapt
PTPd.

> @@ -1568,6 +1576,8 @@ static int dev_gso_segment(struct sk_buff *skb)
>  
>  int dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev)
>  {
> +	int rc;
> +
>  	if (likely(!skb->next)) {
>  		if (!list_empty(&ptype_all))
>  			dev_queue_xmit_nit(skb, dev);
> @@ -1579,13 +1589,15 @@ 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);
> +		if (likely(!rc))
> +			net_tx_timestamp(skb);
> +		return rc;
>  	}

How do you recognize whether the driver did hardware time stamping? Hmm,
okay, I think I can answer that myself: if the driver supports hardware
time stamping, it must clear tstamp in its hard_start_xmit(). Later on
when it has transmitted the packet, it extracts the corresponding
hardware time stamp and calls skb_tx_timestamp(). Right? If so, this
should go into a comment somewhere.

The implicit assumption here is that drivers do not touch tstamp,
because otherwise software time stamping might accidentally be disabled
by a driver. That seems to be the case, at least for in-kernel drivers.

Is skb->sk always guaranteed to be set in hard_start_xmit?
skb_tx_timestamp() depends on it. In 2.6.23 the field always seemed to
be valid, but in 2.6.26 I think I have seen NULL pointers there for PTP
UDP broadcasts.

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


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

* Re: [RFC][PATCH 0/2] net: support for TX timestamps
  2008-07-30  9:47 ` [RFC][PATCH 0/2] net: support for TX timestamps Ingo Oeser
@ 2008-07-30 13:02   ` Octavian Purdila
  0 siblings, 0 replies; 9+ messages in thread
From: Octavian Purdila @ 2008-07-30 13:02 UTC (permalink / raw)
  To: Ingo Oeser; +Cc: netdev

On Wednesday 30 July 2008, Ingo Oeser wrote:

> > This patches adds TX timestamping support and is main use case is the
> > Precision Time Protocol (IEEE 1588). Both software and hardware
> > timestamping can be used.
>
> Could you please include an example on how to use
> this in userspace?
>
> A proper place for this would be Documentation/net/
>
> At the moment I cannot imagine how to use this and how
> to correlate the sent packet with the timestamp.
>
> That would also be a way to test your implementation and
> test any changes done later to it. This will avoid future bitrot.
>

Yes, of course. 

I am also planning to add mac/network/transport layer pointers in the 
sock_extended_err, so that the application can easily "navigate"  the packet 
in the error queue and map it to the sent packet. 

Thanks for the input!

tavi

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

* Re: [RFC][PATCH 1/2] net: support for TX timestamps
  2008-07-30 12:43   ` Patrick Ohly
@ 2008-07-30 13:15     ` Octavian Purdila
  2008-07-30 13:34       ` Patrick Ohly
  0 siblings, 1 reply; 9+ messages in thread
From: Octavian Purdila @ 2008-07-30 13:15 UTC (permalink / raw)
  To: Patrick Ohly; +Cc: netdev

On Wednesday 30 July 2008, Patrick Ohly wrote:

> > The actual 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.
>
> This needs to be augmented to not fall back to software time stamping,
> as discussed in "[RFC][PATCH 1/1] net: support for hardware
> timestamping".
>

Ok, I'll keep this in mind for the next take.

> How do you recognize whether the driver did hardware time stamping? Hmm,
> okay, I think I can answer that myself: if the driver supports hardware
> time stamping, it must clear tstamp in its hard_start_xmit(). Later on
> when it has transmitted the packet, it extracts the corresponding
> hardware time stamp and calls skb_tx_timestamp(). Right? If so, this
> should go into a comment somewhere.
>

Right. 

> The implicit assumption here is that drivers do not touch tstamp,
> because otherwise software time stamping might accidentally be disabled
> by a driver. That seems to be the case, at least for in-kernel drivers.
>

Yes, that is the assumption and I think it should be reasonable.

> Is skb->sk always guaranteed to be set in hard_start_xmit?
> skb_tx_timestamp() depends on it. In 2.6.23 the field always seemed to
> be valid, but in 2.6.26 I think I have seen NULL pointers there for PTP
> UDP broadcasts.

I don't think that skb->sk is guaranteed to be around in hard_start_xmit. But 
we should not need it, if we overload the skb->tstamp, right?

Thanks,
tavi





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

* Re: [RFC][PATCH 1/2] net: support for TX timestamps
  2008-07-30 13:15     ` Octavian Purdila
@ 2008-07-30 13:34       ` Patrick Ohly
  2008-07-30 13:36         ` Octavian Purdila
  0 siblings, 1 reply; 9+ messages in thread
From: Patrick Ohly @ 2008-07-30 13:34 UTC (permalink / raw)
  To: Octavian Purdila; +Cc: netdev

On Wed, 2008-07-30 at 16:15 +0300, Octavian Purdila wrote:
> On Wednesday 30 July 2008, Patrick Ohly wrote:
> > Is skb->sk always guaranteed to be set in hard_start_xmit?
> > skb_tx_timestamp() depends on it. In 2.6.23 the field always seemed to
> > be valid, but in 2.6.26 I think I have seen NULL pointers there for PTP
> > UDP broadcasts.
> 
> I don't think that skb->sk is guaranteed to be around in hard_start_xmit. But 
> we should not need it, if we overload the skb->tstamp, right?

skb->sk is needed in skb_tx_timestamp() to send the copied skb with the
time stamp back to the application, isn't it? skb_tx_timestamp() won't
crash, but it won't be able to do the desired action either. Overloading
skb->tstamp doesn't help with that.

I don't know where the connection to the originating socket is lost, but
in linux-2.6 as of one week ago (not 2.6.26, as I said above) the
skb->sk pointer is indeed NULL for the PTP broadcast packets - I just
checked again.

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


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

* Re: [RFC][PATCH 1/2] net: support for TX timestamps
  2008-07-30 13:34       ` Patrick Ohly
@ 2008-07-30 13:36         ` Octavian Purdila
  0 siblings, 0 replies; 9+ messages in thread
From: Octavian Purdila @ 2008-07-30 13:36 UTC (permalink / raw)
  To: Patrick Ohly; +Cc: netdev

On Wednesday 30 July 2008, Patrick Ohly wrote:

> > I don't think that skb->sk is guaranteed to be around in hard_start_xmit.
> > But we should not need it, if we overload the skb->tstamp, right?
>
> skb->sk is needed in skb_tx_timestamp() to send the copied skb with the
> time stamp back to the application, isn't it? skb_tx_timestamp() won't
> crash, but it won't be able to do the desired action either. Overloading
> skb->tstamp doesn't help with that.
>

Yes you are right :)

> I don't know where the connection to the originating socket is lost, but
> in linux-2.6 as of one week ago (not 2.6.26, as I said above) the
> skb->sk pointer is indeed NULL for the PTP broadcast packets - I just
> checked again.

Thanks for letting me know, I will look into it.

tavi


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

end of thread, other threads:[~2008-07-30 13:39 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-29  0:35 [RFC][PATCH 0/2] net: support for TX timestamps Octavian Purdila
2008-07-29  0:35 ` [RFC][PATCH 1/2] " Octavian Purdila
2008-07-30 12:43   ` Patrick Ohly
2008-07-30 13:15     ` Octavian Purdila
2008-07-30 13:34       ` Patrick Ohly
2008-07-30 13:36         ` Octavian Purdila
2008-07-29  0:35 ` [RFC][PATCH 2/2] ip: support for TX timestamps on UDP and RAW sockets Octavian Purdila
2008-07-30  9:47 ` [RFC][PATCH 0/2] net: support for TX timestamps Ingo Oeser
2008-07-30 13:02   ` Octavian Purdila

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