Thread (31 messages) 31 messages, 6 authors, 2008-12-12

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help