linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next 1/2] tcp: show number of network segments in some SNMP counters
@ 2018-09-09  3:14 Yafang Shao
  2018-09-09  3:14 ` [PATCH v2 net-next 2/2] tcp: fix the error count of tcpInSegs Yafang Shao
  2018-09-09 18:27 ` [PATCH v2 net-next 1/2] tcp: show number of network segments in some SNMP counters Eric Dumazet
  0 siblings, 2 replies; 5+ messages in thread
From: Yafang Shao @ 2018-09-09  3:14 UTC (permalink / raw)
  To: edumazet, davem; +Cc: netdev, linux-kernel, Yafang Shao

It is better to show the number of network segments in bellow SNMP
counters, because that could be more useful for the user.
For example, the user could easily figure out how mant packets are
dropped and how many packets are queued in the out-of-oder queue.

- LINUX_MIB_TCPRCVQDROP
- LINUX_MIB_TCPZEROWINDOWDROP
- LINUX_MIB_TCPBACKLOGDROP
- LINUX_MIB_TCPMINTTLDROP
- LINUX_MIB_TCPOFODROP
- LINUX_MIB_TCPOFOQUEUE

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 net/ipv4/tcp_input.c | 18 ++++++++++++------
 net/ipv4/tcp_ipv4.c  |  9 ++++++---
 net/ipv6/tcp_ipv6.c  |  6 ++++--
 3 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 62508a2..c2ce334 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -4496,7 +4496,8 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
 	tcp_ecn_check_ce(sk, skb);
 
 	if (unlikely(tcp_try_rmem_schedule(sk, skb, skb->truesize))) {
-		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP);
+		NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP,
+			      max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 		tcp_drop(sk, skb);
 		return;
 	}
@@ -4505,7 +4506,8 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
 	tp->pred_flags = 0;
 	inet_csk_schedule_ack(sk);
 
-	NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFOQUEUE);
+	NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPOFOQUEUE,
+		      max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 	seq = TCP_SKB_CB(skb)->seq;
 	end_seq = TCP_SKB_CB(skb)->end_seq;
 	SOCK_DEBUG(sk, "out of order segment: rcv_next %X seq %X - %X\n",
@@ -4666,7 +4668,8 @@ int tcp_send_rcvq(struct sock *sk, struct msghdr *msg, size_t size)
 	skb->len = size;
 
 	if (tcp_try_rmem_schedule(sk, skb, skb->truesize)) {
-		NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP);
+		NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP,
+			      max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 		goto err_free;
 	}
 
@@ -4725,7 +4728,8 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
 	 */
 	if (TCP_SKB_CB(skb)->seq == tp->rcv_nxt) {
 		if (tcp_receive_window(tp) == 0) {
-			NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
+			NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP,
+				      max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 			goto out_of_window;
 		}
 
@@ -4734,7 +4738,8 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
 		if (skb_queue_len(&sk->sk_receive_queue) == 0)
 			sk_forced_mem_schedule(sk, skb->truesize);
 		else if (tcp_try_rmem_schedule(sk, skb, skb->truesize)) {
-			NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP);
+			NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPRCVQDROP,
+				      max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 			goto drop;
 		}
 
@@ -4796,7 +4801,8 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
 		 * remembering D-SACK for its head made in previous line.
 		 */
 		if (!tcp_receive_window(tp)) {
-			NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP);
+			NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPZEROWINDOWDROP,
+				      max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 			goto out_of_window;
 		}
 		goto queue_and_out;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 09547ef..23d7cb5 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -475,7 +475,8 @@ void tcp_v4_err(struct sk_buff *icmp_skb, u32 info)
 		goto out;
 
 	if (unlikely(iph->ttl < inet_sk(sk)->min_ttl)) {
-		__NET_INC_STATS(net, LINUX_MIB_TCPMINTTLDROP);
+		__NET_ADD_STATS(net, LINUX_MIB_TCPMINTTLDROP,
+				max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 		goto out;
 	}
 
@@ -1633,7 +1634,8 @@ bool tcp_add_backlog(struct sock *sk, struct sk_buff *skb)
 
 	if (unlikely(sk_add_backlog(sk, skb, limit))) {
 		bh_unlock_sock(sk);
-		__NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPBACKLOGDROP);
+		__NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPBACKLOGDROP,
+				max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 		return true;
 	}
 	return false;
@@ -1790,7 +1792,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
 		}
 	}
 	if (unlikely(iph->ttl < inet_sk(sk)->min_ttl)) {
-		__NET_INC_STATS(net, LINUX_MIB_TCPMINTTLDROP);
+		__NET_ADD_STATS(net, LINUX_MIB_TCPMINTTLDROP,
+				max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 		goto discard_and_relse;
 	}
 
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 03e6b7a..bbf7667 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -391,7 +391,8 @@ static void tcp_v6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
 		goto out;
 
 	if (ipv6_hdr(skb)->hop_limit < inet6_sk(sk)->min_hopcount) {
-		__NET_INC_STATS(net, LINUX_MIB_TCPMINTTLDROP);
+		__NET_ADD_STATS(net, LINUX_MIB_TCPMINTTLDROP,
+				max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 		goto out;
 	}
 
@@ -1523,7 +1524,8 @@ static int tcp_v6_rcv(struct sk_buff *skb)
 		}
 	}
 	if (hdr->hop_limit < inet6_sk(sk)->min_hopcount) {
-		__NET_INC_STATS(net, LINUX_MIB_TCPMINTTLDROP);
+		__NET_ADD_STATS(net, LINUX_MIB_TCPMINTTLDROP,
+				max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 		goto discard_and_relse;
 	}
 
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* [PATCH v2 net-next 2/2] tcp: fix the error count of tcpInSegs
  2018-09-09  3:14 [PATCH v2 net-next 1/2] tcp: show number of network segments in some SNMP counters Yafang Shao
@ 2018-09-09  3:14 ` Yafang Shao
  2018-09-09 18:32   ` Eric Dumazet
  2018-09-09 18:27 ` [PATCH v2 net-next 1/2] tcp: show number of network segments in some SNMP counters Eric Dumazet
  1 sibling, 1 reply; 5+ messages in thread
From: Yafang Shao @ 2018-09-09  3:14 UTC (permalink / raw)
  To: edumazet, davem; +Cc: netdev, linux-kernel, Yafang Shao

In RFC1213, the tcpInSegs is the total number of segments received.
While currently it is the total number of SKBs received.
The number of SKBs may be not equal with the numer of segments because of
GRO.
So fix this error count.

Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
 include/net/tcp.h   | 2 ++
 net/ipv4/tcp_ipv4.c | 3 ++-
 net/ipv6/tcp_ipv6.c | 3 ++-
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 770917d..66578f4 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -310,6 +310,8 @@ static inline bool tcp_too_many_orphans(struct sock *sk, int shift)
 #define __TCP_INC_STATS(net, field)	__SNMP_INC_STATS((net)->mib.tcp_statistics, field)
 #define TCP_DEC_STATS(net, field)	SNMP_DEC_STATS((net)->mib.tcp_statistics, field)
 #define TCP_ADD_STATS(net, field, val)	SNMP_ADD_STATS((net)->mib.tcp_statistics, field, val)
+#define __TCP_ADD_STATS(net, field, val)	\
+	__SNMP_ADD_STATS((net)->mib.tcp_statistics, field, val)
 
 void tcp_tasklet_init(void);
 
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 23d7cb5..2b98242 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1703,7 +1703,8 @@ int tcp_v4_rcv(struct sk_buff *skb)
 		goto discard_it;
 
 	/* Count it even if it's bad */
-	__TCP_INC_STATS(net, TCP_MIB_INSEGS);
+	__TCP_ADD_STATS(net, TCP_MIB_INSEGS,
+			max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 
 	if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
 		goto discard_it;
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index bbf7667..8d4ef46 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1441,7 +1441,8 @@ static int tcp_v6_rcv(struct sk_buff *skb)
 	/*
 	 *	Count it even if it's bad.
 	 */
-	__TCP_INC_STATS(net, TCP_MIB_INSEGS);
+	__TCP_ADD_STATS(net, TCP_MIB_INSEGS,
+			max_t(u16, 1, skb_shinfo(skb)->gso_segs));
 
 	if (!pskb_may_pull(skb, sizeof(struct tcphdr)))
 		goto discard_it;
-- 
1.8.3.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH v2 net-next 1/2] tcp: show number of network segments in some SNMP counters
  2018-09-09  3:14 [PATCH v2 net-next 1/2] tcp: show number of network segments in some SNMP counters Yafang Shao
  2018-09-09  3:14 ` [PATCH v2 net-next 2/2] tcp: fix the error count of tcpInSegs Yafang Shao
@ 2018-09-09 18:27 ` Eric Dumazet
  1 sibling, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2018-09-09 18:27 UTC (permalink / raw)
  To: Yafang Shao; +Cc: David Miller, netdev, LKML

On Sat, Sep 8, 2018 at 8:14 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> It is better to show the number of network segments in bellow SNMP
> counters, because that could be more useful for the user.
> For example, the user could easily figure out how mant packets are
> dropped and how many packets are queued in the out-of-oder queue.
>
> - LINUX_MIB_TCPRCVQDROP
> - LINUX_MIB_TCPZEROWINDOWDROP
> - LINUX_MIB_TCPBACKLOGDROP
> - LINUX_MIB_TCPMINTTLDROP
> - LINUX_MIB_TCPOFODROP
> - LINUX_MIB_TCPOFOQUEUE
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
>  net/ipv4/tcp_input.c | 18 ++++++++++++------
>  net/ipv4/tcp_ipv4.c  |  9 ++++++---
>  net/ipv6/tcp_ipv6.c  |  6 ++++--
>  3 files changed, 22 insertions(+), 11 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 62508a2..c2ce334 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4496,7 +4496,8 @@ static void tcp_data_queue_ofo(struct sock *sk, struct sk_buff *skb)
>         tcp_ecn_check_ce(sk, skb);
>
>         if (unlikely(tcp_try_rmem_schedule(sk, skb, skb->truesize))) {
> -               NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP);
> +               NET_ADD_STATS(sock_net(sk), LINUX_MIB_TCPOFODROP,
> +                             max_t(u16, 1, skb_shinfo(skb)->gso_segs));
>

I am nacking this patch. These counters are counting events really.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH v2 net-next 2/2] tcp: fix the error count of tcpInSegs
  2018-09-09  3:14 ` [PATCH v2 net-next 2/2] tcp: fix the error count of tcpInSegs Yafang Shao
@ 2018-09-09 18:32   ` Eric Dumazet
  2018-09-10 10:56     ` Yafang Shao
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2018-09-09 18:32 UTC (permalink / raw)
  To: Yafang Shao; +Cc: David Miller, netdev, LKML

On Sat, Sep 8, 2018 at 8:14 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> In RFC1213, the tcpInSegs is the total number of segments received.
> While currently it is the total number of SKBs received.
> The number of SKBs may be not equal with the numer of segments because of
> GRO.
> So fix this error count.
>

We have discussed this in the past and the consensus was it was too
late to change this.

IP counters have the same issue, so after your patch, we would have
quite a difference between transport and network layers.

Adding all these max_t(u16, 1, skb_shinfo(skb)->gso_segs)) everywhere add a cost

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH v2 net-next 2/2] tcp: fix the error count of tcpInSegs
  2018-09-09 18:32   ` Eric Dumazet
@ 2018-09-10 10:56     ` Yafang Shao
  0 siblings, 0 replies; 5+ messages in thread
From: Yafang Shao @ 2018-09-10 10:56 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, LKML

On Mon, Sep 10, 2018 at 2:32 AM, Eric Dumazet <edumazet@google.com> wrote:
> On Sat, Sep 8, 2018 at 8:14 PM Yafang Shao <laoar.shao@gmail.com> wrote:
>>
>> In RFC1213, the tcpInSegs is the total number of segments received.
>> While currently it is the total number of SKBs received.
>> The number of SKBs may be not equal with the numer of segments because of
>> GRO.
>> So fix this error count.
>>
>
> We have discussed this in the past and the consensus was it was too
> late to change this.
>
> IP counters have the same issue, so after your patch, we would have
> quite a difference between transport and network layers.
>
> Adding all these max_t(u16, 1, skb_shinfo(skb)->gso_segs)) everywhere add a cost

May be we could give a comment here why we do it like this, otherwise
it may make a misunderstanding.

Thanks
Yafang

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-09-10 10:57 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-09  3:14 [PATCH v2 net-next 1/2] tcp: show number of network segments in some SNMP counters Yafang Shao
2018-09-09  3:14 ` [PATCH v2 net-next 2/2] tcp: fix the error count of tcpInSegs Yafang Shao
2018-09-09 18:32   ` Eric Dumazet
2018-09-10 10:56     ` Yafang Shao
2018-09-09 18:27 ` [PATCH v2 net-next 1/2] tcp: show number of network segments in some SNMP counters Eric Dumazet

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).