On Mon, Sep 9, 2019 at 4:30 PM Thomas Higdon <t...@fb.com> wrote:
>
> For receive-heavy cases on the server-side, we want to track the
> connection quality for individual client IPs. This counter, similar to
> the existing system-wide TCPOFOQueue counter in /proc/net/netstat,
> tracks out-of-order packet reception. By providing this counter in
> TCP_INFO, it will allow understanding to what degree receive-heavy
> sockets are experiencing out-of-order delivery and packet drops
> indicating congestion.
>
> Please note that this is similar to the counter in NetBSD TCP_INFO, and
> has the same name.
>
> Signed-off-by: Thomas Higdon <t...@fb.com>
> ---
> include/linux/tcp.h | 2 ++
> include/uapi/linux/tcp.h | 2 ++
> net/ipv4/tcp.c | 1 +
> net/ipv4/tcp_input.c | 1 +
> 4 files changed, 6 insertions(+)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index f3a85a7fb4b1..a01dc78218f1 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -393,6 +393,8 @@ struct tcp_sock {
> */
> struct request_sock *fastopen_rsk;
> u32 *saved_syn;
> +
> + u32 rcv_ooopack; /* Received out-of-order packets, for tcpinfo */
> };
>
> enum tsq_enum {
> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> index b3564f85a762..20237987ccc8 100644
> --- a/include/uapi/linux/tcp.h
> +++ b/include/uapi/linux/tcp.h
> @@ -270,6 +270,8 @@ struct tcp_info {
> __u64 tcpi_bytes_retrans; /* RFC4898 tcpEStatsPerfOctetsRetrans */
> __u32 tcpi_dsack_dups; /* RFC4898 tcpEStatsStackDSACKDups */
> __u32 tcpi_reord_seen; /* reordering events seen */
> +
> + __u32 tcpi_rcv_ooopack; /* Out-of-order packets received */
This is problematic : you create a 32bit hole in this structure that
we will never be able to fill.
We need to add another metric here so that the whole 64bit space is used.
> };
>
> /* netlink attributes types for SCM_TIMESTAMPING_OPT_STATS */
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 94df48bcecc2..d4386f054f18 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -3295,6 +3295,7 @@ void tcp_get_info(struct sock *sk, struct tcp_info
> *info)
> info->tcpi_bytes_retrans = tp->bytes_retrans;
> info->tcpi_dsack_dups = tp->dsack_dups;
> info->tcpi_reord_seen = tp->reord_seen;
> + info->tcpi_rcv_ooopack = tp->rcv_ooopack;
> unlock_sock_fast(sk, slow);
> }
> EXPORT_SYMBOL_GPL(tcp_get_info);
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 706cbb3b2986..2774680c5d05 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4555,6 +4555,7 @@ static void tcp_data_queue_ofo(struct sock *sk, struct
> sk_buff *skb)
> tp->pred_flags = 0;
> inet_csk_schedule_ack(sk);
>
> + tp->rcv_ooopack++;
We count skbs or we count segments ?
(GRO might have aggregated multiple segments)
> NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFOQUEUE);
> seq = TCP_SKB_CB(skb)->seq;
> end_seq = TCP_SKB_CB(skb)->end_seq;
You forgot to clear the field in tcp_disconnect()
