netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miroslav Lichvar <mlichvar@redhat.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: "Keller, Jacob E" <jacob.e.keller@intel.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Richard Cochran <richardcochran@gmail.com>,
	Jiri Benc <jbenc@redhat.com>, Denny Page <dennypage@me.com>,
	Willem de Bruijn <willemb@google.com>
Subject: Re: Extending socket timestamping API for NTP
Date: Tue, 28 Feb 2017 09:26:32 +0100	[thread overview]
Message-ID: <20170228082632.GA14857@localhost> (raw)
In-Reply-To: <CAF=yD-JJ2nMYg1soRzCv2CKH0c_i2+Ku-UK8-mMYFtbfbn__kw@mail.gmail.com>

On Mon, Feb 27, 2017 at 07:01:54PM -0500, Willem de Bruijn wrote:
> On Mon, Feb 27, 2017 at 10:23 AM, Miroslav Lichvar <mlichvar@redhat.com> wrote:
> > On Tue, Feb 07, 2017 at 02:32:04PM -0800, Willem de Bruijn wrote:
> >> >> 4) allow sockets to use both SW and HW TX timestamping at the same time

> > Do we need a new option for this?
> 
> Similar to OPT_TSONLY or OPT_ID, but to signal the intent of
> receiving both timestamps. Yes, agreed.

Ok. Thanks.

> > With this change I'm getting two error messages per transmission, but
> > it looks like it may need some additional changes.
> >
> > If the first error message is received after the HW timestamp was
> > captured,
> 
> When does this happen? The first timestamp is generated from
> skb_tx_timestamp in the device driver's ndo_start_xmit before
> passing the packet to the NIC, the second when the device
> driver cleans the tx descriptor on completion.

As I understand it, it happens when the first skb (created by the
skb_tx_timestamp() call) is received by the application after the
driver called skb_tstamp_tx() with the HW timestamp. The SW timestamps
are separate, but the HW timestamp is shared between clones. It
probably doesn't happen with the TSONLY option as it allocates a new
skb. When I print timestamps from scm_timestamping I see a mix of two
cases:

TX 1488268812.193945472 0.000000000 1488286813.273760139
TX 0.000000000 0.000000000 1488286813.273760139
RX 1488268812.354356188 0.000000000 1488286813.434096389

TX 1488268816.364407934 0.000000000 0.000000000
TX 0.000000000 0.000000000 1488286817.444251014
RX 1488268816.525150589 0.000000000 1488286817.604749889

In the first case I assume the HW timestamp was saved before the first
error message was received, so both error messages have the same HW
timestamp, but only one has the SW timestamp. In the second case, the
HW timestamp was saved later, so there is one message with SW
timestamp and one message with HW timestamp.

>From the application point of view it would make sense if in the first
case there was only one error message containing both timestamps. I'm
not sure how easy/safe it would be to drop the second skb. The other
approach would be to not put HW timestamp in the first message when
this "dual TX timestamping" option is enabled, so each error message
has only one timestamp.

> Is this for drivers that do not have skb_tx_timestamp, as you
> mention below? Then the solution is to add that call.

FWIW, I saw that with the e1000e driver after I made the
skb_tx_timestamp() call unconditional.

> > it contains both timestamps as the HW timestamp is in the
> > shared info of the skb. Is it possible it could contain a partially
> > updated HW timestamp? I'm not sure how locking works here. Is
> > scm_timestamping actually allowed to contain more than one timestamp?
> > The timestamping.txt document says "Only one field is non-zero at any
> > time.", but that wasn't true even before if both SW and HW RX
> > timestamping was enabled.

-- 
Miroslav Lichvar

  reply	other threads:[~2017-02-28  8:27 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-07 14:01 Extending socket timestamping API for NTP Miroslav Lichvar
2017-02-07 17:45 ` Keller, Jacob E
2017-02-07 22:32   ` Willem de Bruijn
2017-02-08 14:18     ` Soheil Hassas Yeganeh
2017-02-27 15:23     ` Miroslav Lichvar
2017-02-28  0:01       ` Willem de Bruijn
2017-02-28  8:26         ` Miroslav Lichvar [this message]
2017-02-28 21:05           ` Willem de Bruijn
2017-02-08  1:52   ` Denny Page
2017-02-08  5:27     ` Richard Cochran
2017-02-08  5:48       ` Denny Page
2017-02-08 17:27       ` Denny Page
2017-02-07 18:54 ` Soheil Hassas Yeganeh
2017-02-08 10:14   ` Miroslav Lichvar
2017-02-07 20:37 ` sdncurious
2017-02-08 10:26   ` Miroslav Lichvar
2017-02-08 23:27     ` sdncurious
2017-02-08 23:34     ` sdncurious
2017-02-08  1:18 ` Denny Page
     [not found] ` <CAHoNx58u=Fze4e5V2Wb_LiBhka1Mzny3zOVNfvuzjnmQ4wBO=Q@mail.gmail.com>
2017-02-08  3:06   ` Denny Page
2017-02-09  0:45 ` Denny Page
2017-02-09 11:15   ` Miroslav Lichvar
2017-02-09 20:25   ` Denny Page
2017-02-09  8:02 ` Richard Cochran
2017-02-09 11:09   ` Miroslav Lichvar
2017-02-09 19:42     ` sdncurious
2017-02-09 20:37       ` Denny Page
2017-02-10  0:33       ` Denny Page
2017-02-10 18:55         ` Denny Page
2017-03-23 16:21     ` Miroslav Lichvar
2017-03-23 18:54       ` Denny Page
2017-03-23 19:07       ` Richard Cochran
2017-03-24  7:25         ` Miroslav Lichvar
     [not found]       ` <6121D504-288F-4C9B-9AB3-D1C8292965D5@me.com>
2017-03-24  9:45         ` Miroslav Lichvar
2017-03-24 17:17           ` Denny Page
2017-03-24 18:52             ` Keller, Jacob E
2017-03-27 10:13             ` Miroslav Lichvar
2017-03-27 14:29               ` Richard Cochran
2017-03-27 16:25                 ` Denny Page
2017-03-27 18:28                   ` Richard Cochran
2017-03-27 19:18                     ` Denny Page
2017-03-27 20:58                       ` Richard Cochran
2017-03-27 21:20                         ` Denny Page
2017-03-27 19:21                     ` Denny Page
2017-03-27 19:21                     ` Denny Page
     [not found]                     ` <5FD283AB-39DE-4A9D-902A-BA5F0F0B62A3@me.com>
2017-03-27 21:00                       ` Richard Cochran
2017-03-24  9:55         ` Jiri Benc

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=20170228082632.GA14857@localhost \
    --to=mlichvar@redhat.com \
    --cc=dennypage@me.com \
    --cc=jacob.e.keller@intel.com \
    --cc=jbenc@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=richardcochran@gmail.com \
    --cc=willemb@google.com \
    --cc=willemdebruijn.kernel@gmail.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).