Re: hardware time stamping with extra skb->hwtstamp
From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: 2008-12-01 16:45:27
Patrick Ohly wrote:
Oliver asked:quoted
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.) ?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.
That's really fine. Sorry that i did not go that deep into your code myself :-]
Oliver suggested:quoted
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.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.
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.
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.
I'll be on a business trip until Thursday, so from my side you would get a 'go ahead' right now ;-) Best regards, Oliver