Re: hardware time stamping with extra skb->hwtstamp

[Oliver Hartkopp <socketcan@hartkopp.net>]

Patrick Ohly wrote:
>
> Oliver asked:
>   
>> 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:
>   
>> 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