netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Bjørn Mork" <bjorn@mork.no>
To: Ben Hutchings <ben.hutchings@codethink.co.uk>
Cc: Oliver Neukum <oliver@neukum.org>,
	netdev@vger.kernel.org,
	ct-linux-kernel <linux-kernel@codethink.co.uk>,
	linux-usb@vger.kernel.org,
	Sami Farin <hvtaifwkbgefbaei@gmail.com>,
	Aleksander Morgado <aleksander@aleksander.es>
Subject: Re: [RFC][PATCH] usbnet: Fix tx_bytes statistic running backward in cdc_ncm
Date: Fri, 27 Feb 2015 10:12:27 +0100	[thread overview]
Message-ID: <87h9u73eh0.fsf@nemi.mork.no> (raw)
In-Reply-To: <1424979823.4444.46.camel@xylophone.i.decadent.org.uk> (Ben Hutchings's message of "Thu, 26 Feb 2015 19:43:43 +0000")

Ben Hutchings <ben.hutchings@codethink.co.uk> writes:

> cdc_ncm disagrees with usbnet about how much framing overhead should
> be counted in the tx_bytes statistics, and tries 'fix' this by
> decrementing tx_bytes on the transmit path.  But statistics must never
> be decremented except due to roll-over; this will thoroughly confuse
> user-space.  Also, tx_bytes is only incremented by usbnet in the
> completion path.
>
> Fix this by requiring drivers that set FLAG_MULTI_FRAME to set a
> tx_bytes delta along with the tx_packets count.
>
> Signed-off-by: Ben Hutchings <ben.hutchings@codethink.co.uk>
> ---
> I noticed this bug while trying to fix the tx_packets statistic in asix.
> It depends on the patch I just sent for that.  I don't have any hardware
> to test this with, or any need to make it work.  If you want this fix,
> please test and re-submit it yoursef.

I tested this on an MBIM device, and it worked perfectly as-is.  Please
submit it without the RFC prefix.  This fixes a real and reported
problem with the cdc_ncm driver, so I'd claim it's "net" material along
with a stable Cc and

Fixes: beeecd42c3b4 ("net: cdc_ncm/cdc_mbim: adding NCM protocol statistics")

But you and David decide that, of course...


Fixing this was actually on my TODO-list after a recent report showing
one of the problems with the previous hack: The decremented tx_bytes
counter was never corrected if the URB transmission failed, possibly
ending up with a negative (i.e. very large since it is unsigned)
tx_bytes counter.

Thanks a lot.  I didn't know how to do this without introducing a new
callback or something.  Your solutions is very nice, and so obvious when
I see it.  Just brilliant :-)

Tested-by: Bjørn Mork <bjorn@mork.no>

And you might also want (if this is OK for Sami):

Reported-by: Sami Farin <hvtaifwkbgefbaei@gmail.com>


Sami wrote:

> I have this USB ID 12d1:157d, Sonera Opengate LTE.
> It says all is okay but no packets are moving.
..
> ifconfig
> wwp3s0u1c2: flags=4291<UP,BROADCAST,RUNNING,NOARP,MULTICAST>  mtu 1500
>         inet 46.132.188.224  netmask 255.255.255.192  broadcast 46.132.188.255
>         inet6 fe80::  prefixlen 64  scopeid 0x20<link>
>         ether 32:35:3a:64:2e:25  txqueuelen 1000  (Ethernet)
>         RX packets 0  bytes 0 (0.0 B)
>         RX errors 17764  dropped 0  overruns 0  frame 0
>         TX packets 6714  bytes 18446744073709268911 (1638.3 PiB)
>         TX errors 1427  dropped 0 overruns 0  carrier 0  collisions 0
>




Bjørn

  reply	other threads:[~2015-02-27  9:12 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-26 19:43 [RFC][PATCH] usbnet: Fix tx_bytes statistic running backward in cdc_ncm Ben Hutchings
2015-02-27  9:12 ` Bjørn Mork [this message]
2015-02-27  9:17   ` Oliver Neukum

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h9u73eh0.fsf@nemi.mork.no \
    --to=bjorn@mork.no \
    --cc=aleksander@aleksander.es \
    --cc=ben.hutchings@codethink.co.uk \
    --cc=hvtaifwkbgefbaei@gmail.com \
    --cc=linux-kernel@codethink.co.uk \
    --cc=linux-usb@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=oliver@neukum.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).