Re: hardware time stamping with extra skb->hwtstamp

[Oliver Hartkopp <oliver@hartkopp.net>]

Octavian Purdila wrote:
> From: Oliver Hartkopp <oliver@hartkopp.net>
> Date: Thu, 27 Nov 2008 23:13:08 +0100
>
>   
>>> How about this twist: we add a new option at the socket level, to get the
>>> whole skb->head - skb->end data into a user buffer. Then, we call an
>>> device ioctl and pass this buffer. The device will extract the hw
>>> timestamp and give it to the user.
>>>
>>> We might not need to get the whole skb->head - skb->end buffer, maybe
>>> just skb->head - skb->mac if we know that skb->mac is sane at the socket 
>>> level and we use the convention that the device driver must put the 
>>> timestamp below the mac header.
>>>
>>> One potential problem I see with this approach is leaking sensitive
>>> information into userspace, which means we will have to restrict this to
>>> privileged processes only.
>>>       
>> Ugh.
>>
>> Not every protocol that uses skbuffs, has a mac header (e.g. the CAN
>> protocol doesn't have mac addresses). 
>>     
>
> OK, then what about this: we use a device ioctl to get the number of bytes to 
> copy from skb->head. We then pass this to the socket level option.
>
> This is more complicated than option 3 or 4, but it should address the 
> concerns raised here - no performance impact when not using this feature. The 
> trade-off is moving work from core kernel into userspace and device driver. 
>
>   
>> This twist does not look very maintainable to me ...
>>     
>
> Could you elaborate on the maintainability issues, they are not clear to me.
>   

Do you know what all the people are doing with skbuffs in the kernel? 
I'm not aware of it.
So whenever we touch a vital data structure like the skbuff we must be 
sure that our handling is a wise approach for all.

Btw. your answer brought me to a completely different approach:

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.

This struct hwstamp may consist of 'everthing we really need for 
timestamping' and is added somewhere at the head or the tail of the 
skbuff data section by the device driver.
And if the socket sees this pointer to be not NULL it knows where to 
look for our fancy struct hwstamp ...

I don't know, where this hwstamp data could be pasted into the data 
section in the best way - but surely others know. Especially this 
approach meets the requirement that the additional hwstamp data is only 
allocated (per interface!), when it's really needed - and not for 
everyone by default.

What do you thing about this idea?

Regards,
Oliver