netdev - Re: [PATCH net-next] tcp: introduce a new MIB for CLOSE-WAIT sockets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANn89iJQWj75y+QpLGQKZ6jBgSgpi0ZtPf4830O8S0Ld2PpqEg@mail.gmail.com>
Date: Tue, 28 May 2024 07:13:12 +0200
From: Eric Dumazet <edumazet@...gle.com>
To: Jason Xing <kerneljasonxing@...il.com>
Cc: dsahern@...nel.org, kuba@...nel.org, pabeni@...hat.com, 
	davem@...emloft.net, netdev@...r.kernel.org, 
	Jason Xing <kernelxing@...cent.com>, Yongming Liu <yomiliu@...cent.com>, 
	Wangzi Yong <curuwang@...cent.com>
Subject: Re: [PATCH net-next] tcp: introduce a new MIB for CLOSE-WAIT sockets

On Tue, May 28, 2024 at 4:12 AM Jason Xing <kerneljasonxing@...il.com> wrote:
>
> From: Jason Xing <kernelxing@...cent.com>
>
> CLOSE-WAIT is a relatively special state which "represents waiting for
> a connection termination request from the local user" (RFC 793). Some
> issues may happen because of unexpected/too many CLOSE-WAIT sockets,
> like user application mistakenly handling close() syscall.
>
> We want to trace this total number of CLOSE-WAIT sockets fastly and
> frequently instead of resorting to displaying them altogether by using:
>
>   netstat -anlp | grep CLOSE_WAIT

This is horribly expensive.
Why asking af_unix and program names ?
You want to count some TCP sockets in a given state, right ?
iproute2 interface (inet_diag) can do the filtering in the kernel,
saving a lot of cycles.

ss -t state close-wait

>
> or something like this, which does harm to the performance especially in
> heavy load. That's the reason why I chose to introduce this new MIB counter
> like CurrEstab does. It do help us diagnose/find issues in production.
>
> Besides, in the group of TCP_MIB_* defined by RFC 1213, TCP_MIB_CURRESTAB
> should include both ESTABLISHED and CLOSE-WAIT sockets in theory:
>
>   "tcpCurrEstab OBJECT-TYPE
>    ...
>    The number of TCP connections for which the current state
>    is either ESTABLISHED or CLOSE- WAIT."
>
> Apparently, at least since 2005, we don't count CLOSE-WAIT sockets. I think
> there is a need to count it separately to avoid polluting the existing
> TCP_MIB_CURRESTAB counter.
>
> After this patch, we can see the counter by running 'cat /proc/net/netstat'
> or 'nstat -s | grep CloseWait'

I find this counter quite not interesting.
After a few days of uptime, let say it is 52904523
What can you make of this value exactly ?
How do you make any correlation ?

>
> Suggested-by: Yongming Liu <yomiliu@...cent.com>
> Suggested-by: Wangzi Yong <curuwang@...cent.com>
> Signed-off-by: Jason Xing <kernelxing@...cent.com>
> ---
>  include/uapi/linux/snmp.h | 1 +
>  net/ipv4/proc.c           | 1 +
>  net/ipv4/tcp.c            | 2 ++
>  3 files changed, 4 insertions(+)
>
> diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
> index adf5fd78dd50..c0feefb4d88b 100644
> --- a/include/uapi/linux/snmp.h
> +++ b/include/uapi/linux/snmp.h
> @@ -302,6 +302,7 @@ enum
>         LINUX_MIB_TCPAOKEYNOTFOUND,             /* TCPAOKeyNotFound */
>         LINUX_MIB_TCPAOGOOD,                    /* TCPAOGood */
>         LINUX_MIB_TCPAODROPPEDICMPS,            /* TCPAODroppedIcmps */
> +       LINUX_MIB_TCPCLOSEWAIT,                 /* TCPCloseWait */
>         __LINUX_MIB_MAX
>  };
>
> diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
> index 6c4664c681ca..964897dc6eb8 100644
> --- a/net/ipv4/proc.c
> +++ b/net/ipv4/proc.c
> @@ -305,6 +305,7 @@ static const struct snmp_mib snmp4_net_list[] = {
>         SNMP_MIB_ITEM("TCPAOKeyNotFound", LINUX_MIB_TCPAOKEYNOTFOUND),
>         SNMP_MIB_ITEM("TCPAOGood", LINUX_MIB_TCPAOGOOD),
>         SNMP_MIB_ITEM("TCPAODroppedIcmps", LINUX_MIB_TCPAODROPPEDICMPS),
> +       SNMP_MIB_ITEM("TCPCloseWait", LINUX_MIB_TCPCLOSEWAIT),
>         SNMP_MIB_SENTINEL
>  };
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 681b54e1f3a6..7abaa2660cc8 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2659,6 +2659,8 @@ void tcp_set_state(struct sock *sk, int state)
>         default:
>                 if (oldstate == TCP_ESTABLISHED)
>                         TCP_DEC_STATS(sock_net(sk), TCP_MIB_CURRESTAB);
> +               if (state == TCP_CLOSE_WAIT)
> +                       NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPCLOSEWAIT);
>         }
>
>         /* Change state AFTER socket is unhashed to avoid closed
> --
> 2.37.3
>