Messages in this thread | | | From | Yafang Shao <> | Date | Thu, 26 Jul 2018 12:42:25 +0800 | Subject | Re: [PATCH net-next] tcp: add SNMP counter for the number of packets pruned from ofo queue |
| |
On Thu, Jul 26, 2018 at 12:36 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > > > On 07/25/2018 09:31 PM, Yafang Shao wrote: >> On Thu, Jul 26, 2018 at 12:06 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: >>> >>> >>> On 07/25/2018 06:42 PM, Yafang Shao wrote: >>> >>>> >>>> Hi Eric, >>>> >>>> LINUX_MIB_TCPOFOQUEUE, LINUX_MIB_TCPOFODROP and LINUX_MIB_TCPOFOMERGE >>>> are all for the number of SKBs, but only LINUX_MIB_OFOPRUNED is for >>>> the event, that could lead misunderstading. >>>> So I think introducing a counter for the number of SKB pruned could be >>>> better, that could help us to track the whole behavior of ofo queue. >>>> That is why I submit this patch. >>> >>> Sure, and I said your patch had issues. >>> You mixed 'packets' and 'skbs' but apparently you do not get my point. >>> >> >> I had noticed that I made this mistake. >> >>> I would rather not add another SNMP counter, and refine the current one, >>> trying to track something more meaningful. >>> >>> The notion of 'skb' is internal to the kernel, and can not be mapped easily >>> to 'number of network segments' which probably is more useful for the user. >>> >>> I will send this instead : >>> >>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c >>> index d51fa358b2b196d0f9c258b24354813f2128a675..141a062abd0660c8f6d049de1dc7c7ecf7a7230d 100644 >>> --- a/net/ipv4/tcp_input.c >>> +++ b/net/ipv4/tcp_input.c >>> @@ -5001,18 +5001,19 @@ static bool tcp_prune_ofo_queue(struct sock *sk) >>> { >>> struct tcp_sock *tp = tcp_sk(sk); >>> struct rb_node *node, *prev; >>> + unsigned int segs = 0; >>> int goal; >>> >>> if (RB_EMPTY_ROOT(&tp->out_of_order_queue)) >>> return false; >>> >>> - NET_INC_STATS(sock_net(sk), LINUX_MIB_OFOPRUNED); >>> goal = sk->sk_rcvbuf >> 3; >>> node = &tp->ooo_last_skb->rbnode; >>> do { >>> prev = rb_prev(node); >>> rb_erase(node, &tp->out_of_order_queue); >>> goal -= rb_to_skb(node)->truesize; >>> + segs += max_t(u16, 1, skb_shinfo(rb_to_skb(node))->gso_segs); >>> tcp_drop(sk, rb_to_skb(node)); >>> if (!prev || goal <= 0) { >>> sk_mem_reclaim(sk); >>> @@ -5023,6 +5024,7 @@ static bool tcp_prune_ofo_queue(struct sock *sk) >>> } >>> node = prev; >>> } while (node); >>> + NET_ADD_STATS(sock_net(sk), LINUX_MIB_OFOPRUNED, segs); >>> tp->ooo_last_skb = rb_to_skb(prev); >>> >>> /* Reset SACK state. A conforming SACK implementation will >>> >> >> You patch will make it more meaningful. >> How about the other ones? > > Same thing really. > > Note that tcp_collapse() does not currently propagate skb_shinfo(skb)->gso_segs > when rebuilding the skbs, so this will need a fix. >
Got it. Thank you very much for your comment.
Thanks Yafang
| |