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: <491B3B49.7070402@linux.intel.com>
Date:	Wed, 12 Nov 2008 21:23:37 +0100
From:	Andi Kleen <ak@...ux.intel.com>
To:	Eric Dumazet <dada1@...mosbay.com>
CC:	Oliver Hartkopp <oliver@...tkopp.net>,
	Patrick Ohly <patrick.ohly@...el.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	Octavian Purdila <opurdila@...acom.com>,
	Stephen Hemminger <shemminger@...tta.com>,
	Ingo Oeser <netdev@...eo.de>,
	"Ronciak, John" <john.ronciak@...el.com>
Subject: Re: [RFC PATCH 00/13] hardware time stamping + igb example	implementation

Eric Dumazet wrote:
> Oliver Hartkopp a écrit :
>> Patrick Ohly wrote:
>>> On Wed, 2008-11-12 at 16:06 +0000, Andi Kleen wrote:
>>>  
>>>> As a general comment on the patch series I'm still a little sceptical
>>>> the time stamp offset method is a good idea. Since it tries to 
>>>> approximate
>>>> several unsynchronized clocks the result will always be of a little 
>>>> poor
>>>> quality, which will likely lead to problems sooner or later (or rather
>>>> require ugly workarounds in the user).
>>>>
>>>> I think it would be better to just bite the bullet and add new fields
>>>> for this to the skbs. Hardware timestamps are useful enough to justify
>>>> this.
>>>>     
>>>
>>> I'm all for it, as long as it doesn't keep this feature out of the
>>> mainline.
>>>
>>> At least one additional ktime_t field would be needed for the raw
>>> hardware time stamp. Transformation to system time (as needed by PTP)
>>> would have to be delayed until the packet is delivered via a socket. The
>>> code would be easier (and a bit more accurate) if also another ktime_t
>>> was added to store the transformed value directly after generating it.
>>>
>>> An extra field would also solve one of the open problems (tstamp set to
>>> time stamp when dev_start_xmit_hard is called for IP_MULTICAST_LOOP).
>>>
>>>   
>>
>> I really wondered if you posted the series to get an impression why 
>> adding a new field is a good idea ;-)
>> Ok, i'm not that experienced on timestamps but i really got confused 
>> reading the patches and it's documentation (even together with the 
>> discussion on the ML). I would also vote for having a new field in the 
>> skb instead of this current 'bit-compression' approach which smells 
>> quite expensive at runtime and in code size. Not talking about the 
>> mentioned potential locking issues ...
> 
> New fields in skb are probably the easy way to handle the problem, we
> all know that.
> 
> But adding fields on such heavy duty structure for less than 0.001 % of
> handled frames is disgusting.

You have a strange definition of "disgusting".

But if that's true that applies to the existing timestamp in there then too
(and a couple of other fields in there too)

Also I suspect that your percent numbers are wrong, depending on the workload.

Personally I think hardware time stamps should replace the existing
time stamp and I suspect more and more applications will move to that eventually.


> If an application needs skb hw timestamp, get it when reading message, 
> with appropriate
> API, that calls NIC driver method, giving skb pointer as an argument. 
> NIC driver
> search in its local table a match of skb pointer (giving the most recent 
> match of course),
> and converts hwtimestamp into "generic application format". No need for 
> a fast search, just
> a linear search in the table, so that feeding it is really easy (maybe 
> lockless)

This will probably be a disaster on e.g. high speed network sniffing
(which is one of the primary use cases of the hardware
As soon as there is any reordering in the queue (and that is inevitable
if you scale over multiple CPUs) your linear searches could get quite
long and bounce cache lines like mad. Also I doubt it can be really
done lockless.

Also to be honest such a complicated and likely badly performing scheme just to save 4-8 bytes
would match my own definition of "disgusting".

-Andi
--
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ