| Message ID | 1324511401-2640-1-git-send-email-subramanian.vijay@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Vijay Subramanian <subramanian.vijay@gmail.com> Date: Wed, 21 Dec 2011 15:50:01 -0800 > Linux MIB LINUX_MIB_OFOPRUNED is supposed to count the number of packets > dropped from the out-of-order queue due to socket buffer overrun. Instead > of counting the number of skbs freed, it counts the number of calls make to > __skb_queue_purge() which is not what the user (see f.e. netstat) is expecting. > Fix this by incrementing the counter correctly. > > Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com> I disagree, this is an event, and the counter is counting how many times we prune the out of order queue, not how many packets we prune from that queue. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 21 December 2011 16:33, David Miller <davem@davemloft.net> wrote: > From: Vijay Subramanian <subramanian.vijay@gmail.com> > Date: Wed, 21 Dec 2011 15:50:01 -0800 > >> Linux MIB LINUX_MIB_OFOPRUNED is supposed to count the number of packets >> dropped from the out-of-order queue due to socket buffer overrun. Instead >> of counting the number of skbs freed, it counts the number of calls make to >> __skb_queue_purge() which is not what the user (see f.e. netstat) is expecting. >> Fix this by incrementing the counter correctly. >> >> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com> > > I disagree, this is an event, and the counter is counting how many times > we prune the out of order queue, not how many packets we prune from that > queue. David, Thank you for the review. The reason I felt this was a bug was because of what netstat reports. Here are 2 sample output lines from netstat. 622 packets pruned from receive queue because of socket buffer overrun --> (from LINUX_MIB_RCVPRUNED) 7 packets dropped from out-of-order queue because of socket buffer overrun --> (from LINUX_MIB_OFOPRUNED) netstat is interpreting this incorrectly if the mib counter is supposed to be tracking events. Also, LINUX_MIB_OFOPRUNED is named similarly to counters that track dropped packets (e.g.LINUX_MIB_RCVPRUNED ) than counters that track events such as a function call (e.g LINUX_MIB_PRUNECALLED). I realize the naming scheme is not a clinching argument but taken with what netstat reports, the intent seems to be to track dropped packets. If the counter is tracking the right thing, is it worth fixing netstat? The nstat tool in iproute2 tool does not print any explanatory text in the output, so there is less chance of confusion there. Maybe we just use the newer nstat and forget about this? Thanks for your time. Vijay -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Vijay Subramanian <subramanian.vijay@gmail.com> Date: Wed, 21 Dec 2011 17:14:05 -0800 > If the counter is tracking the right thing, is it worth fixing > netstat? Probably. The thing is, I can see both intepretations being useful. If we count packets, we can't tell that OFO pruning happened 4 times vs. we pruned 4 packets. But if you want to know the "extent" to which OFO pruning occurs, then you want the packet count. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/21/2011 07:15 PM, David Miller wrote: > From: Vijay Subramanian<subramanian.vijay@gmail.com> > Date: Wed, 21 Dec 2011 17:14:05 -0800 > >> If the counter is tracking the right thing, is it worth fixing >> netstat? > > Probably. > > The thing is, I can see both intepretations being useful. > > If we count packets, we can't tell that OFO pruning happened 4 times > vs. we pruned 4 packets. > > But if you want to know the "extent" to which OFO pruning occurs, then > you want the packet count. Starting from the premise that the *primary* goal of netstat is to allow end users/admins to know about packet loss events and how they correlate with retransmissions, were I to have a "vote" it would be "count the packets." rick jones -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c index 2877c3e..0e2c21b 100644 --- a/net/ipv4/tcp_input.c +++ b/net/ipv4/tcp_input.c @@ -4833,7 +4833,8 @@ static int tcp_prune_ofo_queue(struct sock *sk) int res = 0; if (!skb_queue_empty(&tp->out_of_order_queue)) { - NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_OFOPRUNED); + NET_ADD_STATS_BH(sock_net(sk), LINUX_MIB_OFOPRUNED, + tp->out_of_order_queue.qlen); __skb_queue_purge(&tp->out_of_order_queue); /* Reset SACK state. A conforming SACK implementation will
Linux MIB LINUX_MIB_OFOPRUNED is supposed to count the number of packets dropped from the out-of-order queue due to socket buffer overrun. Instead of counting the number of skbs freed, it counts the number of calls make to __skb_queue_purge() which is not what the user (see f.e. netstat) is expecting. Fix this by incrementing the counter correctly. Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com> --- net/ipv4/tcp_input.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-)