Skip to content

L
linux
Switch branch/tag
  1. 21 May, 2014 3 commits
  3. 19 May, 2014 21 commits
  5. 18 May, 2014 3 commits
  7. 17 May, 2014 13 commits
    • David S. Miller's avatar
      Merge branch 'cdc_ncm-coalesce' · 33fcc5e0
      David S. Miller authored 
      Bjørn Mork says:

====================
cdc_ncm: add buffer tuning and stats using ethtool

Quoting the previous description of this series (skip to the
changelog below if you only want a summary of the changes):

"I have got quite a few reports from frustrated users of OpenWRT
hosts trying to use some powerful LTE modem, but not achieving
full speed.  This is typically caused by a combination of
big buffers and little memory, giving in allocation errors and
bad performance as a result.

This series is an attempt to let users adjust the size of these
buffers without having to rebuild the driver.

Patches 1 - 4 are mostly rearranging existing code, in preparing
for the dynamic buffer size changes.

Patch 5 adds userspace control (ab)using the ethtool coalescing
API. This isn't a perfect match, which is the main reason why I
post this series as a RFC.

Patch 6 is an unrelated framing optimization, reducing the
overhead quite a bit and allowing for better use of smaller
buffers.

Patch 7 changes the way we calculate frame padding cutoff. The
problem with big buffers is made much worse by the current padding
strategy where zero padding often can account for more than 90% of
the frames.

Patch 8 add some counters giving some insight into how well the
NCM/MBIM protocol works, supporting further tuning.

Patch 9 reduce the initial maximum buffer size from 32kB to 16kB
in an attempt to make the default better suit all. It is still
possible to tune this up again to the old fixed max, using the
new tuning knobs.

I must admit that I had higher hopes for this series before I
tested it on my own modems.  One really unexpected result was
that one of the MBIM modems accepted the new rx buffer size we
set, but happily continued sending buffers of the same size as
before.  Needless to say:  This did not work very well...

So don't really expect to be able to use any values with any
given device. Firmware implementations are still... I don't
think I have words suitable for a public mailing list.

But I am hoping this will help the many users who have had success
rebuilding the driver with lower fixed limits.

Please test and/or comment!"

Changes:

** RFC -> v1 **

Patch 10 - a follow-up to a comment Joe Perches made in November
           2013.  I don't always forget :-)
Patch 11 - removes the redundant "connected" driver state, and the
           associated .check_connect callbacks.

** v1 -> v2 **

Patch 1  - Better handling of minium rx buffer size, based on feedback
           from Oliver Neukum and Enrico Mioso
Patch 5  - fixed locking around timer interval update
Patch 9  - fixed whitespace error
Patch 12 - new fix related to the tuneable tx timer

...and spelling fixes all over the commit messages.  I have finally
added a spelling hook, which I'm sure may of you will appreciate :-)
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      33fcc5e0
    • Bjørn Mork's avatar
      net: cdc_ncm: do not start timer on an empty skb · 046c6594
      Bjørn Mork authored 
      We can end up with a freshly allocated tx_curr_skb with no frames
in it.  In this case it does not make any sense to start the timer.
This avoids the timer periodically trying to start tx when there
is nothing in the queue.
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      046c6594
    • Bjørn Mork's avatar
      net: cdc_ncm: remove redundant "disconnected" flag · fa83dbee
      Bjørn Mork authored 
      Calling netif_carrier_{on,off} is sufficient.  There is no need
to duplicate the carrier state in a driver specific flag.
Acked-by: default avatarEnrico Mioso <mrkiko.rs@gmail.com>
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      fa83dbee
    • Bjørn Mork's avatar
      net: cdc_ncm: fix argument alignment · 916f7640
      Bjørn Mork authored 
      Reported-by: default avatarJoe Perches <joe@perches.com>
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      916f7640
    • Bjørn Mork's avatar
      net: cdc_ncm: use sane defaults for rx/tx buffers · 50f1cb1c
      Bjørn Mork authored 
      Lots of devices request much larger buffers than reasonable. This
cause real problems for users of hosts with limited resources.

Reducing the default buffer size to 16kB for such devices is
a reasonable trade-off between allowing them to aggregate traffic
and avoiding memory exhaustion on resource restrained hosts.
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      50f1cb1c
    • Bjørn Mork's avatar
      net: cdc_ncm/cdc_mbim: adding NCM protocol statistics · beeecd42
      Bjørn Mork authored 
      To have an idea of the effects of the protocol coalescing
it's useful to have some counters showing the different
aspects.

Due to the asymmetrical usbnet interface the netdev
rx_bytes counter has been counting real received payload,
while the tx_bytes counter has included the NCM/MBIM
framing overhead. This overhead can be many times the
payload because of the aggressive padding strategy of
this driver, and will vary a lot depending on device
and traffic.

With very few exceptions, users are only interested in
the payload size.  Having an somewhat accurate payload
byte counter is particularly important for mobile
broadband devices, which many NCM devices and of course
all MBIM devices are. Users and userspace applications
will use this counter to monitor account quotas.

Having protocol specific counters for the overhead, we are
now able to correct the tx_bytes netdev counter so that
it shows the real payload
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      beeecd42
    • Bjørn Mork's avatar
      net: cdc_ncm: set reasonable padding limits · 43e4c6df
      Bjørn Mork authored 
      We pad frames larger than X to maximum size for devices which
don't need a ZLP after maximum sized frames. This allows the
device to optimize its transfers for one fixed buffer size.

X was arbitrarily set at 512 bytes regardless of real buffer
maximum, causing extreme overheads due to excessive padding of
larger tx buffers. Limit the padding to at most 3 full USB
packets, still allowing the overhead to payload ratio of 3/1.
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      43e4c6df
    • Bjørn Mork's avatar
      net: cdc_ncm: use true max dgram count for header estimates · 70559b89
      Bjørn Mork authored 
      Many newer NCM and MBIM devices will request a maximum tx
datagram count which is much smaller than our hard-coded
absolute max. We can reduce the overhead without sacrificing
any of the simplicity for these devices, by simply using the
true negotiated count in when calculated the maximum NTH and
NDP header sizes.
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      70559b89
    • Bjørn Mork's avatar
      net: cdc_ncm: use ethtool to tune coalescing settings · 6c4e548f
      Bjørn Mork authored 
      Datagram coalescing is an integral part of the NCM and MBIM
protocols, intended to reduce the interrupt load primarily
on the device end of the USB link.  As with all coalescing
solutions, there is a trade-off between buffering and
interrupts.

The current defaults are based on the assumption that device
side buffers should be the limiting factor.  However, many
modern high speed LTE modems suffers from buffer-bloat,
making this assumption fail. This results in sub-optimal
performance due to excessive coalescing.  And in cases where
such modems are connected to cheap embedded hosts there is
often severe buffer allocation issues, giving very noticeable
performance degradation .

A start on improving this is going from build time hard
coded limits to per device user configurable limits.  The
ethtool coalescing API was selected as user interface
because, although the tuned values are buffer sizes, these
settings directly control datagram coalescing.
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      6c4e548f
    • Bjørn Mork's avatar
      net: cdc_ncm: support rx_max/tx_max updates when running · 68864abf
      Bjørn Mork authored 
      Finish the rx_max/tx_max setup by flushing buffers and
informing usbnet about the changes.  This way, the settings
can be modified while the netdev is up and running.
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      68864abf
    • Bjørn Mork's avatar
      net: cdc_ncm: split .bind device initialization · 08c74fc9
      Bjørn Mork authored 
      Now that we have split out the part of the device setup
which MUST be done with the data interface in altsetting 0,
we can delay the rest of the initialization. This allows us
to move some of post-init buffer size config from bind to
the appropriate setup function.

The purpose of this refactoring is to collect all code
adjusting the rx_max and tx_max buffers in one place, so
that it is easier to call it from multiple call sites.
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      08c74fc9
    • Bjørn Mork's avatar
      net: cdc_ncm: factor out one-time device initialization · f8afb73d
      Bjørn Mork authored 
      Split the parts of setup dealing with device initialization from
parts just setting defaults for attributes which might be
changed after initialization.

Some commands of the device initialization are only allowed when
the data interface is in its disabled altsetting, so we must
separate them out of we are to allow rerunning parts of setup.
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      f8afb73d
    • Bjørn Mork's avatar
      net: cdc_ncm: split out rx_max/tx_max update of setup · 5aa73d5d
      Bjørn Mork authored 
      Split out the part of setup dealing with updating the rx_max
and tx_max buffer sizes so that this code can be reused for
dynamically updating the limits.
Signed-off-by: default avatarBjørn Mork <bjorn@mork.no>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
      5aa73d5d
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7