| Message ID | 1226415407.31699.1.camel@ecld0pohly |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
Patrick Ohly a écrit : > If generated in hardware, then the driver must convert to system > time before storing the transformed value in sk_buff with skb_hwtstamp_set(). > If conversion back to the original hardware time stamp is desired, > then the driver needs to implement the hwtstamp_raw() callback, which > is called by skb_hwtstamp_raw(). > > The purpose of the new skb_* methods is the hiding of how hardware > time stamps are really stored. Later they might be stored in an extra > field instead of mangling the existing tstamp. > > Signed-off-by: Patrick Ohly <patrick.ohly@intel.com> > --- > include/linux/netdevice.h | 12 +++++++ > include/linux/skbuff.h | 76 +++++++++++++++++++++++++++++++++++++++++++- > net/core/skbuff.c | 32 +++++++++++++++++++ > 3 files changed, 118 insertions(+), 2 deletions(-) > > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h > index 488c56e..4da51cb 100644 > --- a/include/linux/netdevice.h > +++ b/include/linux/netdevice.h > @@ -749,6 +749,18 @@ struct net_device > /* for setting kernel sock attribute on TCP connection setup */ > #define GSO_MAX_SIZE 65536 > unsigned int gso_max_size; > + > + /* hardware time stamping support */ > +#define HAVE_HW_TIME_STAMP > + /* Transforms skb->tstamp back to the original, raw hardware > + * time stamp. The value must have been generated by the > + * device. Implementing this is optional, but necessary for > + * SO_TIMESTAMP_HARDWARE. > + * > + * Returns 1 if value could be retrieved, 0 otherwise. > + */ > + int (*hwtstamp_raw)(const struct sk_buff *skb, > + struct timespec *stamp); > }; > #define to_net_dev(d) container_of(d, struct net_device, dev) > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > index 9099237..0b3b36a 100644 > --- a/include/linux/skbuff.h > +++ b/include/linux/skbuff.h > @@ -199,7 +199,10 @@ typedef unsigned char *sk_buff_data_t; > * @next: Next buffer in list > * @prev: Previous buffer in list > * @sk: Socket we are owned by > - * @tstamp: Time we arrived > + * @tstamp: Time we arrived: usually generated by ktime_get_real() and > + * thus is recorded in system time. If the lowest bit is set, > + * then the value was originally generated by a different clock > + * in the receiving hardware and then transformed to system time. > * @dev: Device we arrived on/are leaving by > * @transport_header: Transport layer header > * @network_header: Network layer header > @@ -1524,23 +1527,52 @@ static inline void skb_copy_to_linear_data_offset(struct sk_buff *skb, > > extern void skb_init(void); Please use ktime_t instead of "union ktime" > > +/** returns skb->tstamp without the bit which marks hardware time stamps */ > +static inline union ktime skb_get_ktime(const struct sk_buff *skb) > +{ > +#if BITS_PER_LONG != 64 && !defined(CONFIG_KTIME_SCALAR) > + return ktime_set(skb->tstamp.tv.sec, > + skb->tstamp.tv.nsec & ~1); > +#else > + return (ktime_t) { .tv64 = skb->tstamp.tv64 & ~1UL }; > +#endif > +} > + > /** > * skb_get_timestamp - get timestamp from a skb > * @skb: skb to get stamp from > * @stamp: pointer to struct timeval to store stamp in > * > * Timestamps are stored in the skb as offsets to a base timestamp. > + * The lowest bit is set if and only if the time stamp was originally > + * created by hardware when processing the packet. > + * > * This function converts the offset back to a struct timeval and stores > * it in stamp. > */ > static inline void skb_get_timestamp(const struct sk_buff *skb, struct timeval *stamp) > { > - *stamp = ktime_to_timeval(skb->tstamp); > + *stamp = ktime_to_timeval(skb_get_ktime(skb)); > +} > + > +static inline void skb_get_timestampns(const struct sk_buff *skb, struct timespec *stamp) > +{ > + *stamp = ktime_to_timespec(skb_get_ktime(skb)); > } > > static inline void __net_timestamp(struct sk_buff *skb) > { > skb->tstamp = ktime_get_real(); > + > + /* > + * make sure that lowest bit is never set: it marks hardware > + * time stamps > + */ > +#if BITS_PER_LONG != 64 && !defined(CONFIG_KTIME_SCALAR) > + skb->tstamp.tv.sec = skb->tstamp.tv.sec / 2 * 2; .tv.sec ? are you sure you dont want .tv.nsec ? > +#else > + skb->tstamp.tv64 = skb->tstamp.tv64 / 2 * 2; > +#endif > } > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2008-11-12 at 07:41 +0000, Eric Dumazet wrote: > Please use ktime_t instead of "union ktime" Are you sure? include/linux/ktime.h says typedef union ktime ktime_t; /* Kill this */ and the CodingStyle also seems to be against it. I thought it would be good to avoid using the typedef in new code, but if consistency with the existing code is preferred, then I'll change it. > > + > > + /* > > + * make sure that lowest bit is never set: it marks hardware > > + * time stamps > > + */ > > +#if BITS_PER_LONG != 64 && !defined(CONFIG_KTIME_SCALAR) > > + skb->tstamp.tv.sec = skb->tstamp.tv.sec / 2 * 2; > > .tv.sec ? are you sure you dont want .tv.nsec ? Eek! Right. I'm pretty sure I compiled this in 32 bit mode, but I haven't actually tried the result.
From: Patrick Ohly <patrick.ohly@intel.com> Date: Wed, 22 Oct 2008 10:17:24 +0200 > +int skb_hwtstamp_raw(const struct sk_buff *skb, struct timespec *stamp) > +{ > + struct rtable *rt; > + struct in_device *idev; > + struct net_device *netdev; > + > + if (skb_hwtstamp_available(skb) && > + (rt = skb->rtable) != NULL && > + (idev = rt->idev) != NULL && > + (netdev = idev->dev) != NULL && > + netdev->hwtstamp_raw) { > + return netdev->hwtstamp_raw(skb, stamp); > + } else { > + return 0; > + } > +} > + > +EXPORT_SYMBOL_GPL(skb_hwtstamp_raw); You can't be accessing the generic destination cache entry attached to the SKB, here in generic SKB code, as a pointer to an ipv4 specific route object. What if this is an IPV6 or DECNET packet? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Patrick Ohly <patrick.ohly@intel.com> Date: Wed, 12 Nov 2008 09:09:04 +0100 > On Wed, 2008-11-12 at 07:41 +0000, Eric Dumazet wrote: > > Please use ktime_t instead of "union ktime" > > Are you sure? > > include/linux/ktime.h says > typedef union ktime ktime_t; /* Kill this */ > and the CodingStyle also seems to be against it. > > I thought it would be good to avoid using the typedef in new code, but > if consistency with the existing code is preferred, then I'll change it. Well you then go ahead and cast return values to "ktime_t" so this code is not even being consistent about the choice. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2008-11-12 at 09:58 +0000, David Miller wrote: > From: Patrick Ohly <patrick.ohly@intel.com> > Date: Wed, 22 Oct 2008 10:17:24 +0200 > > > +int skb_hwtstamp_raw(const struct sk_buff *skb, struct timespec *stamp) > > +{ > > + struct rtable *rt; > > + struct in_device *idev; > > + struct net_device *netdev; > > + > > + if (skb_hwtstamp_available(skb) && > > + (rt = skb->rtable) != NULL && > > + (idev = rt->idev) != NULL && > > + (netdev = idev->dev) != NULL && > > + netdev->hwtstamp_raw) { > > + return netdev->hwtstamp_raw(skb, stamp); > > + } else { > > + return 0; > > + } > > +} > > + > > +EXPORT_SYMBOL_GPL(skb_hwtstamp_raw); > > You can't be accessing the generic destination cache entry attached to > the SKB, here in generic SKB code, as a pointer to an ipv4 specific > route object. What if this is an IPV6 or DECNET packet? Yes, this is problematic. The revised patch still depends on this pointer chain, now to convert the raw hardware time stamp to system time. I don't see any clean solution right now except adding both raw and transformed value to the skb (16 additional bytes instead of just 8, or zero as in the original patch).
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 488c56e..4da51cb 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -749,6 +749,18 @@ struct net_device /* for setting kernel sock attribute on TCP connection setup */ #define GSO_MAX_SIZE 65536 unsigned int gso_max_size; + + /* hardware time stamping support */ +#define HAVE_HW_TIME_STAMP + /* Transforms skb->tstamp back to the original, raw hardware + * time stamp. The value must have been generated by the + * device. Implementing this is optional, but necessary for + * SO_TIMESTAMP_HARDWARE. + * + * Returns 1 if value could be retrieved, 0 otherwise. + */ + int (*hwtstamp_raw)(const struct sk_buff *skb, + struct timespec *stamp); }; #define to_net_dev(d) container_of(d, struct net_device, dev) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 9099237..0b3b36a 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -199,7 +199,10 @@ typedef unsigned char *sk_buff_data_t; * @next: Next buffer in list * @prev: Previous buffer in list * @sk: Socket we are owned by - * @tstamp: Time we arrived + * @tstamp: Time we arrived: usually generated by ktime_get_real() and + * thus is recorded in system time. If the lowest bit is set, + * then the value was originally generated by a different clock + * in the receiving hardware and then transformed to system time. * @dev: Device we arrived on/are leaving by * @transport_header: Transport layer header * @network_header: Network layer header @@ -1524,23 +1527,52 @@ static inline void skb_copy_to_linear_data_offset(struct sk_buff *skb, extern void skb_init(void); +/** returns skb->tstamp without the bit which marks hardware time stamps */ +static inline union ktime skb_get_ktime(const struct sk_buff *skb) +{ +#if BITS_PER_LONG != 64 && !defined(CONFIG_KTIME_SCALAR) + return ktime_set(skb->tstamp.tv.sec, + skb->tstamp.tv.nsec & ~1); +#else + return (ktime_t) { .tv64 = skb->tstamp.tv64 & ~1UL }; +#endif +} + /** * skb_get_timestamp - get timestamp from a skb * @skb: skb to get stamp from * @stamp: pointer to struct timeval to store stamp in * * Timestamps are stored in the skb as offsets to a base timestamp. + * The lowest bit is set if and only if the time stamp was originally + * created by hardware when processing the packet. + * * This function converts the offset back to a struct timeval and stores * it in stamp. */ static inline void skb_get_timestamp(const struct sk_buff *skb, struct timeval *stamp) { - *stamp = ktime_to_timeval(skb->tstamp); + *stamp = ktime_to_timeval(skb_get_ktime(skb)); +} + +static inline void skb_get_timestampns(const struct sk_buff *skb, struct timespec *stamp) +{ + *stamp = ktime_to_timespec(skb_get_ktime(skb)); } static inline void __net_timestamp(struct sk_buff *skb) { skb->tstamp = ktime_get_real(); + + /* + * make sure that lowest bit is never set: it marks hardware + * time stamps + */ +#if BITS_PER_LONG != 64 && !defined(CONFIG_KTIME_SCALAR) + skb->tstamp.tv.sec = skb->tstamp.tv.sec / 2 * 2; +#else + skb->tstamp.tv64 = skb->tstamp.tv64 / 2 * 2; +#endif } static inline ktime_t net_timedelta(ktime_t t) @@ -1553,6 +1585,46 @@ static inline ktime_t net_invalid_timestamp(void) return ktime_set(0, 0); } +/** + * checks whether the time stamp value has been set (= non-zero) + * and really came from hardware + */ +static inline int skb_hwtstamp_available(const struct sk_buff *skb) +{ + return skb->tstamp.tv64 & 1; +} + +static inline void skb_hwtstamp_set(struct sk_buff *skb, + union ktime stamp) +{ +#if BITS_PER_LONG != 64 && !defined(CONFIG_KTIME_SCALAR) + skb->tstamp.tv.sec = stamp.tv.sec; + skb->tstamp.tv.nsec = stamp.tv.nsec | 1; +#else + skb->tstamp.tv64 = stamp.tv64 | 1; +#endif +} + +/** + * Fills the timespec with the original, "raw" time stamp as generated + * by the hardware when it processed the packet and returns 1 if such + * a hardware time stamp is unavailable or cannot be inferred. Otherwise + * it returns 0; + */ +int skb_hwtstamp_raw(const struct sk_buff *skb, struct timespec *stamp); + +/** + * Fills the timespec with the hardware time stamp generated when the + * hardware processed the packet, transformed to system time. Beware + * that this transformation is not perfect: packet A received on + * interface 1 before packet B on interface 2 might have a higher + * transformed time stamp. + * + * Returns 1 if a transformed hardware time stamp is available, 0 + * otherwise. + */ +int skb_hwtstamp_transformed(const struct sk_buff *skb, struct timespec *stamp); + extern __sum16 __skb_checksum_complete_head(struct sk_buff *skb, int len); extern __sum16 __skb_checksum_complete(struct sk_buff *skb); diff --git a/net/core/skbuff.c b/net/core/skbuff.c index ca1ccdf..7a95062 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -44,6 +44,7 @@ #include <linux/in.h> #include <linux/inet.h> #include <linux/slab.h> +#include <linux/inetdevice.h> #include <linux/netdevice.h> #ifdef CONFIG_NET_CLS_ACT #include <net/pkt_sched.h> @@ -2323,6 +2324,37 @@ err: EXPORT_SYMBOL_GPL(skb_segment); +int skb_hwtstamp_raw(const struct sk_buff *skb, struct timespec *stamp) +{ + struct rtable *rt; + struct in_device *idev; + struct net_device *netdev; + + if (skb_hwtstamp_available(skb) && + (rt = skb->rtable) != NULL && + (idev = rt->idev) != NULL && + (netdev = idev->dev) != NULL && + netdev->hwtstamp_raw) { + return netdev->hwtstamp_raw(skb, stamp); + } else { + return 0; + } +} + +EXPORT_SYMBOL_GPL(skb_hwtstamp_raw); + +int skb_hwtstamp_transformed(const struct sk_buff *skb, struct timespec *stamp) +{ + if (skb_hwtstamp_available(skb)) { + skb_get_timestampns(skb, stamp); + return 1; + } else { + return 0; + } +} + +EXPORT_SYMBOL_GPL(skb_hwtstamp_transformed); + void __init skb_init(void) { skbuff_head_cache = kmem_cache_create("skbuff_head_cache",
If generated in hardware, then the driver must convert to system time before storing the transformed value in sk_buff with skb_hwtstamp_set(). If conversion back to the original hardware time stamp is desired, then the driver needs to implement the hwtstamp_raw() callback, which is called by skb_hwtstamp_raw(). The purpose of the new skb_* methods is the hiding of how hardware time stamps are really stored. Later they might be stored in an extra field instead of mangling the existing tstamp. Signed-off-by: Patrick Ohly <patrick.ohly@intel.com> --- include/linux/netdevice.h | 12 +++++++ include/linux/skbuff.h | 76 +++++++++++++++++++++++++++++++++++++++++++- net/core/skbuff.c | 32 +++++++++++++++++++ 3 files changed, 118 insertions(+), 2 deletions(-)