netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Bjørn Mork" <bjorn@mork.no>
To: <netdev@vger.kernel.org>
Cc: linux-usb@vger.kernel.org,
	"Alexey Orishko" <alexey.orishko@gmail.com>,
	"Oliver Neukum" <oliver@neukum.org>,
	"Enrico Mioso" <mrkiko.rs@gmail.com>,
	"David Laight" <David.Laight@ACULAB.COM>,
	"Bjørn Mork" <bjorn@mork.no>
Subject: [PATCH net-next 05/11] net: cdc_ncm: use ethtool to tune coalescing settings
Date: Sat, 10 May 2014 17:41:43 +0200	[thread overview]
Message-ID: <1399736509-1159-6-git-send-email-bjorn@mork.no> (raw)
In-Reply-To: <1399736509-1159-1-git-send-email-bjorn@mork.no>

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 suboptimal
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 noticable
performance degredation .

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: Bjørn Mork <bjorn@mork.no>
---
 drivers/net/usb/cdc_ncm.c   | 68 +++++++++++++++++++++++++++++++++++++++++++--
 include/linux/usb/cdc_ncm.h |  6 +++-
 2 files changed, 70 insertions(+), 4 deletions(-)

diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index 466922a8af4d..b20c82c19e02 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -65,6 +65,62 @@ static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx);
 static enum hrtimer_restart cdc_ncm_tx_timer_cb(struct hrtimer *hr_timer);
 static struct usb_driver cdc_ncm_driver;
 
+static int cdc_ncm_get_coalesce(struct net_device *netdev,
+				struct ethtool_coalesce *ec)
+{
+	struct usbnet *dev = netdev_priv(netdev);
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+
+	/* assuming maximum sized dgrams and ignoring NDPs */
+	ec->rx_max_coalesced_frames = ctx->rx_max / ctx->max_datagram_size;
+	ec->tx_max_coalesced_frames = ctx->tx_max / ctx->max_datagram_size;
+
+	/* the timer will fire CDC_NCM_TIMER_PENDING_CNT times in a row */
+	ec->tx_coalesce_usecs = (ctx->timer_interval * CDC_NCM_TIMER_PENDING_CNT) / NSEC_PER_USEC;
+	return 0;
+}
+
+static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx);
+
+static int cdc_ncm_set_coalesce(struct net_device *netdev,
+				struct ethtool_coalesce *ec)
+{
+	struct usbnet *dev = netdev_priv(netdev);
+	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+	u32 new_rx_max = ctx->rx_max;
+	u32 new_tx_max = ctx->tx_max;
+
+	/* assuming maximum sized dgrams and a single NDP */
+	if (ec->rx_max_coalesced_frames)
+		new_rx_max = ec->rx_max_coalesced_frames * ctx->max_datagram_size;
+	if (ec->tx_max_coalesced_frames)
+		new_tx_max = ec->tx_max_coalesced_frames * ctx->max_datagram_size;
+
+	if (ec->tx_coalesce_usecs &&
+	    (ec->tx_coalesce_usecs < CDC_NCM_TIMER_INTERVAL_MIN * CDC_NCM_TIMER_PENDING_CNT ||
+	     ec->tx_coalesce_usecs > CDC_NCM_TIMER_INTERVAL_MAX * CDC_NCM_TIMER_PENDING_CNT))
+		return -EINVAL;
+	ctx->timer_interval = ec->tx_coalesce_usecs * NSEC_PER_USEC / CDC_NCM_TIMER_PENDING_CNT;
+
+	/* inform device of new values */
+	if (new_rx_max != ctx->rx_max || new_tx_max != ctx->tx_max)
+		cdc_ncm_update_rxtx_max(dev, new_rx_max, new_tx_max);
+	return 0;
+}
+
+static const struct ethtool_ops cdc_ncm_ethtool_ops = {
+	.get_settings      = usbnet_get_settings,
+	.set_settings      = usbnet_set_settings,
+	.get_link          = usbnet_get_link,
+	.nway_reset        = usbnet_nway_reset,
+	.get_drvinfo       = usbnet_get_drvinfo,
+	.get_msglevel      = usbnet_get_msglevel,
+	.set_msglevel      = usbnet_set_msglevel,
+	.get_ts_info       = ethtool_op_get_ts_info,
+	.get_coalesce      = cdc_ncm_get_coalesce,
+	.set_coalesce      = cdc_ncm_set_coalesce,
+};
+
 /* handle rx_max and tx_max changes */
 static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
 {
@@ -250,6 +306,9 @@ static int cdc_ncm_init(struct usbnet *dev)
 			(ctx->tx_max_datagrams > CDC_NCM_DPT_DATAGRAMS_MAX))
 		ctx->tx_max_datagrams = CDC_NCM_DPT_DATAGRAMS_MAX;
 
+	/* initial coalescing timer interval */
+	ctx->timer_interval = CDC_NCM_TIMER_INTERVAL_USEC * NSEC_PER_USEC;
+
 	return 0;
 }
 
@@ -589,6 +648,9 @@ advance:
 	/* finish setting up the device specific data */
 	cdc_ncm_setup(dev);
 
+	/* override ethtool_ops */
+	dev->net->ethtool_ops = &cdc_ncm_ethtool_ops;
+
 	return 0;
 
 error2:
@@ -857,7 +919,7 @@ cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign)
 		ctx->tx_curr_skb = skb_out;
 		goto exit_no_skb;
 
-	} else if ((n < ctx->tx_max_datagrams) && (ready2send == 0)) {
+	} else if ((n < ctx->tx_max_datagrams) && (ready2send == 0) && (ctx->timer_interval > 0)) {
 		/* wait for more frames */
 		/* push variables */
 		ctx->tx_curr_skb = skb_out;
@@ -909,7 +971,7 @@ static void cdc_ncm_tx_timeout_start(struct cdc_ncm_ctx *ctx)
 	/* start timer, if not already started */
 	if (!(hrtimer_active(&ctx->tx_timer) || atomic_read(&ctx->stop)))
 		hrtimer_start(&ctx->tx_timer,
-				ktime_set(0, CDC_NCM_TIMER_INTERVAL),
+				ktime_set(0, ctx->timer_interval),
 				HRTIMER_MODE_REL);
 }
 
@@ -929,7 +991,7 @@ static void cdc_ncm_txpath_bh(unsigned long param)
 	struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
 
 	spin_lock_bh(&ctx->mtx);
-	if (ctx->tx_timer_pending != 0) {
+	if (ctx->tx_timer_pending != 0 && ctx->timer_interval > 0) {
 		ctx->tx_timer_pending--;
 		cdc_ncm_tx_timeout_start(ctx);
 		spin_unlock_bh(&ctx->mtx);
diff --git a/include/linux/usb/cdc_ncm.h b/include/linux/usb/cdc_ncm.h
index 44b38b92236a..07ad8c3284a6 100644
--- a/include/linux/usb/cdc_ncm.h
+++ b/include/linux/usb/cdc_ncm.h
@@ -72,7 +72,9 @@
 /* Restart the timer, if amount of datagrams is less than given value */
 #define	CDC_NCM_RESTART_TIMER_DATAGRAM_CNT	3
 #define	CDC_NCM_TIMER_PENDING_CNT		2
-#define CDC_NCM_TIMER_INTERVAL			(400UL * NSEC_PER_USEC)
+#define CDC_NCM_TIMER_INTERVAL_USEC		400UL
+#define CDC_NCM_TIMER_INTERVAL_MIN		5UL
+#define CDC_NCM_TIMER_INTERVAL_MAX		(15UL * USEC_PER_SEC)
 
 /* The following macro defines the minimum header space */
 #define	CDC_NCM_MIN_HDR_SIZE \
@@ -107,6 +109,8 @@ struct cdc_ncm_ctx {
 	spinlock_t mtx;
 	atomic_t stop;
 
+	u64 timer_interval;
+
 	u32 tx_timer_pending;
 	u32 tx_curr_frame_num;
 	u32 rx_max;
-- 
2.0.0.rc2

  parent reply	other threads:[~2014-05-10 15:42 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-10 15:41 [PATCH net-next 00/11] cdc_ncm: add buffer tuning and stats using ethtool Bjørn Mork
2014-05-10 15:41 ` [PATCH net-next 02/11] net: cdc_ncm: factor out one-time device initialization Bjørn Mork
2014-05-10 15:41 ` [PATCH net-next 03/11] net: cdc_ncm: split .bind " Bjørn Mork
2014-05-10 15:41 ` Bjørn Mork [this message]
2014-05-10 15:41 ` [PATCH net-next 06/11] net: cdc_ncm: use true max dgram count for header estimates Bjørn Mork
2014-05-10 15:41 ` [PATCH net-next 07/11] net: cdc_ncm: set reasonable padding limits Bjørn Mork
2014-05-10 15:41 ` [PATCH net-next 08/11] net: cdc_ncm/cdc_mbim: adding NCM protocol statiscics Bjørn Mork
2014-05-10 15:41 ` [PATCH net-next 09/11] net: cdc_ncm: use sane defaults for rx/tx buffers Bjørn Mork
     [not found] ` <1399736509-1159-1-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>
2014-05-10 15:41   ` [PATCH net-next 01/11] net: cdc_ncm: split out rx_max/tx_max update of setup Bjørn Mork
2014-05-13  8:09     ` Oliver Neukum
2014-05-13  8:49       ` Bjørn Mork
2014-05-13  9:09         ` Oliver Neukum
     [not found]           ` <1399972165.8278.11.camel-B2T3B9s34ElbnMAlSieJcQ@public.gmane.org>
2014-05-13  9:25             ` Bjørn Mork
2014-05-13 11:07               ` Oliver Neukum
2014-05-10 15:41   ` [PATCH net-next 04/11] net: cdc_ncm: support rx_max/tx_max updates when running Bjørn Mork
2014-05-10 15:41   ` [PATCH net-next 10/11] net: cdc_ncm: fix argument alignment Bjørn Mork
2014-05-10 15:41 ` [PATCH net-next 11/11] net: cdc_ncm: remove redundant "disconnected" flag Bjørn Mork
2014-05-11  9:14   ` Enrico Mioso (@atlantide)

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=1399736509-1159-6-git-send-email-bjorn@mork.no \
    --to=bjorn@mork.no \
    --cc=David.Laight@ACULAB.COM \
    --cc=alexey.orishko@gmail.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=mrkiko.rs@gmail.com \
    --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).