From: Eric Dumazet <eric.dumazet@gmail.com>
To: William Dauchy <wdauchy@gmail.com>, netdev@vger.kernel.org
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Subject: Re: [PATCH v2] tcp: add timestamp options fetcher
Date: Sat, 26 Oct 2019 13:42:19 -0700 [thread overview]
Message-ID: <d7d15ef6-bc88-7cfa-3d3c-b220b13924ae@gmail.com> (raw)
In-Reply-To: <20191026184554.32648-1-wdauchy@gmail.com>
On 10/26/19 11:45 AM, William Dauchy wrote:
> tsval and tsecr are useful in some cases to diagnose TCP issues from the
> sender point of view where unexplained RTT values are seen. Getting the
> the timestamps from both ends will help understand those issues more
> easily.
> It can be mostly use in some specific cases, e.g a http server where
> requests are tagged with such informations, which later helps to
> diagnose some issues and create some useful metrics to give a general
> signal.
>
William, I am sorry but you do not describe what is supposed to be returned
in the structure.
Is it something updated for every incoming packet, every outgoing packet ?
What about out of order packets ?
Is the tcp_tsval our view of the tcp timestamps, or the value from the remote peer ?
What time unit is used for the values ?
What happens if TCP receives a packet that is discarded ? (for example by PAWS check)
tcp_parse_aligned_timestamp() would have updated tp->rx_opt.rcv_tsval and tp->rx_opt.rcv_tsecr
with garbage.
This means tp->rx_opt.rcv_tsval and tp->rx_opt.rcv_tsecr and really some working space
that could contain garbage. We could even imagine that a future version of TCP
no longer uses stable storage in the tcp socket for these, but some temporary room in the stack.
You really need to put more thinking into this, because otherwise every possible user
will have to dig the answers into linux kernel source.
In short, I really believe this tsval/tsecr thing is very hard to define and would
add more complexity in TCP just to make sure this is correctly implemented
and wont regress in the future.
You will have to convince us that this is a super useful feature, maybe publishing research results.
prev parent reply other threads:[~2019-10-26 20:42 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-02 22:10 [PATCH] tcp: add tsval and tsecr to TCP_INFO William Dauchy
2019-10-02 22:33 ` Eric Dumazet
2019-10-02 22:54 ` William Dauchy
2019-10-02 23:14 ` Eric Dumazet
2019-10-03 5:47 ` William Dauchy
2019-10-26 18:45 ` [PATCH v2] tcp: add timestamp options fetcher William Dauchy
2019-10-26 20:42 ` Eric Dumazet [this message]
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=d7d15ef6-bc88-7cfa-3d3c-b220b13924ae@gmail.com \
--to=eric.dumazet@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=wdauchy@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).