| Message ID | 20191026184554.32648-1-wdauchy@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | [v2] tcp: add timestamp options fetcher | expand |
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.
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h index 81e697978e8b..2a9685216aef 100644 --- a/include/uapi/linux/tcp.h +++ b/include/uapi/linux/tcp.h @@ -128,12 +128,18 @@ enum { #define TCP_CM_INQ TCP_INQ #define TCP_TX_DELAY 37 /* delay outgoing packets by XX usec */ +#define TCP_TIMESTAMP_OPT 38 /* timestamps option with tsval and tsecr values */ #define TCP_REPAIR_ON 1 #define TCP_REPAIR_OFF 0 #define TCP_REPAIR_OFF_NO_WP -1 /* Turn off without window probes */ +struct tcp_timestamp_opt { + __u32 tcp_tsval; + __u32 tcp_tsecr; +}; + struct tcp_repair_opt { __u32 opt_code; __u32 opt_val; diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 42187a3b82f4..b9c34a5477dd 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -3309,6 +3309,21 @@ void tcp_get_info(struct sock *sk, struct tcp_info *info) } EXPORT_SYMBOL_GPL(tcp_get_info); +/* Return timestamps option of tcp endpoint. */ +static void tcp_get_tsopt(struct sock *sk, struct tcp_timestamp_opt *tsopt) +{ + const struct tcp_sock *tp = tcp_sk(sk); + + memset(tsopt, 0, sizeof(*tsopt)); + if (sk->sk_type != SOCK_STREAM) + return; + + if (tp->rx_opt.tstamp_ok) { + tsopt->tcp_tsval = tp->rx_opt.rcv_tsval; + tsopt->tcp_tsecr = tp->rx_opt.rcv_tsecr; + } +} + static size_t tcp_opt_stats_get_size(void) { return @@ -3668,6 +3683,21 @@ static int do_tcp_getsockopt(struct sock *sk, int level, return err; } #endif + case TCP_TIMESTAMP_OPT: { + struct tcp_timestamp_opt tsopt; + + if (get_user(len, optlen)) + return -EFAULT; + + tcp_get_tsopt(sk, &tsopt); + + len = min_t(unsigned int, len, sizeof(tsopt)); + if (put_user(len, optlen)) + return -EFAULT; + if (copy_to_user(optval, &tsopt, len)) + return -EFAULT; + return 0; + } default: return -ENOPROTOOPT; }
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. Signed-off-by: William Dauchy <wdauchy@gmail.com> --- Changes in v2: - change from tcp_info to a new getsockopt() to avoid making tcp_info bigger for everyone --- include/uapi/linux/tcp.h | 6 ++++++ net/ipv4/tcp.c | 30 ++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+)