[PATCH net-next] tcp: remove the TCPSmallQueueFailure counter

[PATCH net-next] tcp: remove the TCPSmallQueueFailure counter

[Jakub Kicinski]

This reverts commit aeeecb889165 ("net: snmp: add statistics for
tcp small queue check").

The recently added TSQ-limit-hit metric does not provide clear,
actionable signal and can be confusing to the user as it may
well increment under normal operation (yet it has Failure in
its name). Menglong mentioned that the condition he was
targetting arised due to a bug in the virtio driver.

Link: https://lore.kernel.org/r/20211128060102.6504-1-imagedong@tencent.com/
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
 include/uapi/linux/snmp.h | 1 -
 net/ipv4/proc.c           | 1 -
 net/ipv4/tcp_output.c     | 5 +----
 3 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
index e32ec6932e82..904909d020e2 100644
--- a/include/uapi/linux/snmp.h
+++ b/include/uapi/linux/snmp.h
@@ -292,7 +292,6 @@ enum
 	LINUX_MIB_TCPDSACKIGNOREDDUBIOUS,	/* TCPDSACKIgnoredDubious */
 	LINUX_MIB_TCPMIGRATEREQSUCCESS,		/* TCPMigrateReqSuccess */
 	LINUX_MIB_TCPMIGRATEREQFAILURE,		/* TCPMigrateReqFailure */
-	LINUX_MIB_TCPSMALLQUEUEFAILURE,		/* TCPSmallQueueFailure */
 	__LINUX_MIB_MAX
 };
 
diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
index 43b7a77cd6b4..f30273afb539 100644
--- a/net/ipv4/proc.c
+++ b/net/ipv4/proc.c
@@ -297,7 +297,6 @@ static const struct snmp_mib snmp4_net_list[] = {
 	SNMP_MIB_ITEM("TCPDSACKIgnoredDubious", LINUX_MIB_TCPDSACKIGNOREDDUBIOUS),
 	SNMP_MIB_ITEM("TCPMigrateReqSuccess", LINUX_MIB_TCPMIGRATEREQSUCCESS),
 	SNMP_MIB_ITEM("TCPMigrateReqFailure", LINUX_MIB_TCPMIGRATEREQFAILURE),
-	SNMP_MIB_ITEM("TCPSmallQueueFailure", LINUX_MIB_TCPSMALLQUEUEFAILURE),
 	SNMP_MIB_SENTINEL
 };
 
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index c4ab6c8f0c77..5079832af5c1 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2524,11 +2524,8 @@ static bool tcp_small_queue_check(struct sock *sk, const struct sk_buff *skb,
 		 * test again the condition.
 		 */
 		smp_mb__after_atomic();
-		if (refcount_read(&sk->sk_wmem_alloc) > limit) {
-			NET_INC_STATS(sock_net(sk),
-				      LINUX_MIB_TCPSMALLQUEUEFAILURE);
+		if (refcount_read(&sk->sk_wmem_alloc) > limit)
 			return true;
-		}
 	}
 	return false;
 }
-- 
2.31.1

Re: [PATCH net-next] tcp: remove the TCPSmallQueueFailure counter

[Eric Dumazet]

On 11/30/21 5:51 PM, Jakub Kicinski wrote:
> This reverts commit aeeecb889165 ("net: snmp: add statistics for
> tcp small queue check").
>
> The recently added TSQ-limit-hit metric does not provide clear,
> actionable signal and can be confusing to the user as it may
> well increment under normal operation (yet it has Failure in
> its name). Menglong mentioned that the condition he was
> targetting arised due to a bug in the virtio driver.
>
> Link: https://lore.kernel.org/r/20211128060102.6504-1-imagedong@tencent.com/
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>


Oops, I have seen this a bit late.


Maybe because my primary email address is edumazet@google.com, and that 
my @gmail.com

accounts have some lag.


Reviewed-by: Eric Dumazet <edumazet@google.com>


Thanks.


> ---
>   include/uapi/linux/snmp.h | 1 -
>   net/ipv4/proc.c           | 1 -
>   net/ipv4/tcp_output.c     | 5 +----
>   3 files changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/snmp.h b/include/uapi/linux/snmp.h
> index e32ec6932e82..904909d020e2 100644
> --- a/include/uapi/linux/snmp.h
> +++ b/include/uapi/linux/snmp.h
> @@ -292,7 +292,6 @@ enum
>   	LINUX_MIB_TCPDSACKIGNOREDDUBIOUS,	/* TCPDSACKIgnoredDubious */
>   	LINUX_MIB_TCPMIGRATEREQSUCCESS,		/* TCPMigrateReqSuccess */
>   	LINUX_MIB_TCPMIGRATEREQFAILURE,		/* TCPMigrateReqFailure */
> -	LINUX_MIB_TCPSMALLQUEUEFAILURE,		/* TCPSmallQueueFailure */
>   	__LINUX_MIB_MAX
>   };
>   
> diff --git a/net/ipv4/proc.c b/net/ipv4/proc.c
> index 43b7a77cd6b4..f30273afb539 100644
> --- a/net/ipv4/proc.c
> +++ b/net/ipv4/proc.c
> @@ -297,7 +297,6 @@ static const struct snmp_mib snmp4_net_list[] = {
>   	SNMP_MIB_ITEM("TCPDSACKIgnoredDubious", LINUX_MIB_TCPDSACKIGNOREDDUBIOUS),
>   	SNMP_MIB_ITEM("TCPMigrateReqSuccess", LINUX_MIB_TCPMIGRATEREQSUCCESS),
>   	SNMP_MIB_ITEM("TCPMigrateReqFailure", LINUX_MIB_TCPMIGRATEREQFAILURE),
> -	SNMP_MIB_ITEM("TCPSmallQueueFailure", LINUX_MIB_TCPSMALLQUEUEFAILURE),
>   	SNMP_MIB_SENTINEL
>   };
>   
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index c4ab6c8f0c77..5079832af5c1 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -2524,11 +2524,8 @@ static bool tcp_small_queue_check(struct sock *sk, const struct sk_buff *skb,
>   		 * test again the condition.
>   		 */
>   		smp_mb__after_atomic();
> -		if (refcount_read(&sk->sk_wmem_alloc) > limit) {
> -			NET_INC_STATS(sock_net(sk),
> -				      LINUX_MIB_TCPSMALLQUEUEFAILURE);
> +		if (refcount_read(&sk->sk_wmem_alloc) > limit)
>   			return true;
> -		}
>   	}
>   	return false;
>   }