[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <d7d15ef6-bc88-7cfa-3d3c-b220b13924ae@gmail.com>
Date: Sat, 26 Oct 2019 13:42:19 -0700
From: Eric Dumazet <eric.dumazet@...il.com>
To: William Dauchy <wdauchy@...il.com>, netdev@...r.kernel.org
Cc: Eric Dumazet <eric.dumazet@...il.com>
Subject: Re: [PATCH v2] tcp: add timestamp options fetcher
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.
Powered by blists - more mailing lists