netdev - Re: hardware time stamping with extra skb->hwtstamp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <493414A4.8010804@hartkopp.net>
Date:	Mon, 01 Dec 2008 17:45:24 +0100
From:	Oliver Hartkopp <socketcan@...tkopp.net>
To:	Patrick Ohly <patrick.ohly@...el.com>
CC:	Octavian Purdila <opurdila@...acom.com>,
	Oliver Hartkopp <oliver@...tkopp.net>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: hardware time stamping with extra skb->hwtstamp

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

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html