Patchwork [net] net: usbnet: fix tx_dropped statistics

login
register
mail settings
Submitter Bjørn Mork
Date Jan. 31, 2013, 6:36 p.m.
Message ID <1359657365-2879-1-git-send-email-bjorn@mork.no>
Download mbox | patch
Permalink /patch/217238/
State Accepted
Delegated to: David Miller
Headers show

Comments

Bjørn Mork - Jan. 31, 2013, 6:36 p.m.
It is normal for minidrivers accumulating frames to return NULL
from their tx_fixup function. We do not want to count this as a
drop, or log any debug messages.  A different exit path is
therefore chosen for such drivers, skipping the debug message
and the tx_dropped increment.

The test for accumulating drivers was however completely bogus,
making the exit path selection depend on whether the user had
enabled tx_err logging or not. This would arbitrarily mess up
accounting for both accumulating and non-accumulating minidrivers,
and would result in unwanted debug messages for the accumulating
drivers.

Fix by testing for FLAG_MULTI_PACKET instead, which probably was
the intention from the beginning.  This usage match the documented
behaviour of this flag:

 Indicates to usbnet, that USB driver accumulates multiple IP packets.
 Affects statistic (counters) and short packet handling.

Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/usbnet.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
David Miller - Feb. 4, 2013, 6:07 p.m.
From: Bjørn Mork <bjorn@mork.no>
Date: Thu, 31 Jan 2013 19:36:05 +0100

> It is normal for minidrivers accumulating frames to return NULL
> from their tx_fixup function. We do not want to count this as a
> drop, or log any debug messages.  A different exit path is
> therefore chosen for such drivers, skipping the debug message
> and the tx_dropped increment.
> 
> The test for accumulating drivers was however completely bogus,
> making the exit path selection depend on whether the user had
> enabled tx_err logging or not. This would arbitrarily mess up
> accounting for both accumulating and non-accumulating minidrivers,
> and would result in unwanted debug messages for the accumulating
> drivers.
> 
> Fix by testing for FLAG_MULTI_PACKET instead, which probably was
> the intention from the beginning.  This usage match the documented
> behaviour of this flag:
> 
>  Indicates to usbnet, that USB driver accumulates multiple IP packets.
>  Affects statistic (counters) and short packet handling.
> 
> Signed-off-by: Bjørn Mork <bjorn@mork.no>

Applied.
--
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

Patch

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 9778377..5e33606 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1125,13 +1125,11 @@  netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
 	if (info->tx_fixup) {
 		skb = info->tx_fixup (dev, skb, GFP_ATOMIC);
 		if (!skb) {
-			if (netif_msg_tx_err(dev)) {
-				netif_dbg(dev, tx_err, dev->net, "can't tx_fixup skb\n");
-				goto drop;
-			} else {
-				/* cdc_ncm collected packet; waits for more */
+			/* packet collected; minidriver waiting for more */
+			if (info->flags & FLAG_MULTI_PACKET)
 				goto not_drop;
-			}
+			netif_dbg(dev, tx_err, dev->net, "can't tx_fixup skb\n");
+			goto drop;
 		}
 	}
 	length = skb->len;