From: Eric Dumazet <dada1@cosmosbay.com>
To: Oliver Hartkopp <oliver@hartkopp.net>
Cc: Patrick Ohly <patrick.ohly@intel.com>,
Andi Kleen <ak@linux.intel.com>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
Octavian Purdila <opurdila@ixiacom.com>,
Stephen Hemminger <shemminger@vyatta.com>,
Ingo Oeser <netdev@axxeo.de>,
"Ronciak, John" <john.ronciak@intel.com>
Subject: Re: [RFC PATCH 00/13] hardware time stamping + igb example implementation
Date: Wed, 12 Nov 2008 20:22:43 +0100 [thread overview]
Message-ID: <491B2D03.1090700@cosmosbay.com> (raw)
In-Reply-To: <491B23FE.9000105@hartkopp.net>
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.
Crazy idea here :
Say your NIC is capable of generating hw timestamps at TX or RX time.
Instead of storing them in skb, store them in a local structure (of the driver)
The local structure could contain an array of 4096 (or whatever size) couples of
{pointer to skb, hardware timestamp with whatever format is needed by this NIC}
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)
For TX side, a flag on skb could ask NIC driver to feed transmited skb (or a copy of them)
to a raw socket (kind of a loopback for selected packets), once TX hstamp is collected in local table.
next prev parent reply other threads:[~2008-11-12 19:23 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-11-11 14:44 [RFC PATCH 00/13] hardware time stamping + igb example implementation Patrick Ohly
2008-10-22 8:17 ` [RFC PATCH 02/13] extended semantic of sk_buff::tstamp: lowest bit marks hardware time stamps Patrick Ohly
2008-11-12 7:41 ` Eric Dumazet
2008-11-12 8:09 ` Patrick Ohly
2008-11-12 10:09 ` David Miller
2008-11-12 9:58 ` David Miller
2008-11-19 12:50 ` Patrick Ohly
2008-10-22 12:46 ` [RFC PATCH 01/13] put_cmsg_compat + SO_TIMESTAMP[NS]: use same name for value as caller Patrick Ohly
2008-11-12 9:55 ` David Miller
2008-10-22 15:01 ` [RFC PATCH 03/13] user space API for time stamping of incoming and outgoing packets Patrick Ohly
2008-11-12 10:02 ` David Miller
2008-10-24 13:41 ` [RFC PATCH 04/13] net: implement generic SOF_TIMESTAMPING_TX_* support Patrick Ohly
2008-11-11 23:15 ` Octavian Purdila
2008-11-12 8:38 ` Patrick Ohly
2008-10-24 13:49 ` [RFC PATCH 05/13] ip: support for TX timestamps on UDP and RAW sockets Patrick Ohly
2008-11-12 9:59 ` David Miller
2008-10-29 14:48 ` [RFC PATCH 06/13] workaround: detect time stamp when command flags are expected Patrick Ohly
2008-11-12 10:00 ` David Miller
2008-10-31 11:43 ` [RFC PATCH 07/13] net: add SIOCSHWTSTAMP - hardware time stamping of packets Patrick Ohly
2008-10-31 12:21 ` [RFC PATCH 08/13] igb: stub support for SIOCSHWTSTAMP Patrick Ohly
2008-11-04 9:23 ` [RFC PATCH 09/13] clocksource: allow usage independent of timekeeping.c Patrick Ohly
2008-11-12 10:04 ` David Miller
2008-11-04 9:27 ` [RFC PATCH 10/13] igb: infrastructure for hardware time stamping Patrick Ohly
2008-11-05 9:58 ` [RFC PATCH 11/13] time sync: generic infrastructure to map between time stamps generated by a clock source and system time Patrick Ohly
2008-11-11 16:18 ` Andi Kleen
2008-11-12 8:01 ` Patrick Ohly
2008-11-12 10:08 ` David Miller
2008-11-12 16:14 ` Patrick Ohly
2008-11-12 16:28 ` Eric Dumazet
2008-11-12 10:05 ` David Miller
2008-11-06 11:13 ` [RFC PATCH 12/13] igb: use clocksync to implement hardware time stamping Patrick Ohly
2008-11-07 9:26 ` [RFC PATCH 13/13] skbuff: optionally store hardware time stamps in new field Patrick Ohly
2008-11-12 16:06 ` [RFC PATCH 00/13] hardware time stamping + igb example implementation Andi Kleen
2008-11-12 16:25 ` Patrick Ohly
2008-11-12 18:44 ` Oliver Hartkopp
2008-11-12 19:22 ` Eric Dumazet [this message]
2008-11-12 20:23 ` Andi Kleen
2008-11-12 20:23 ` Andi Kleen
2008-11-12 20:56 ` Eric Dumazet
2008-11-12 21:34 ` Andi Kleen
2008-11-12 22:26 ` Oliver Hartkopp
2008-11-13 15:53 ` Ohly, Patrick
2008-11-13 6:15 ` Oliver Hartkopp
2008-11-13 6:29 ` Eric Dumazet
2008-11-13 16:05 ` Ohly, Patrick
2008-11-16 8:15 ` Andrew Shewmaker
2008-11-12 22:17 ` David Miller
2008-11-19 12:39 ` Patrick Ohly
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=491B2D03.1090700@cosmosbay.com \
--to=dada1@cosmosbay.com \
--cc=ak@linux.intel.com \
--cc=john.ronciak@intel.com \
--cc=netdev@axxeo.de \
--cc=netdev@vger.kernel.org \
--cc=oliver@hartkopp.net \
--cc=opurdila@ixiacom.com \
--cc=patrick.ohly@intel.com \
--cc=shemminger@vyatta.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).