LKML Archive on lore.kernel.org
 help / color / Atom feed
From: "Ahmed S. Darwish" <darwish.07@gmail.com>
To: Olivier Sobrie <olivier@sobrie.be>,
	Oliver Hartkopp <socketcan@hartkopp.net>,
	Wolfgang Grandegger <wg@grandegger.com>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	Andri Yngvason <andri.yngvason@marel.com>
Cc: Linux-CAN <linux-can@vger.kernel.org>,
	Linux-USB <linux-usb@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>
Subject: [PATCH 2/5] can: kvaser_usb: Read all messages in a bulk-in URB buffer
Date: Thu, 26 Feb 2015 10:22:02 -0500
Message-ID: <20150226152202.GB6075@linux> (raw)
In-Reply-To: <20150226152011.GA6075@linux>

From: Ahmed S. Darwish <ahmed.darwish@valeo.com>

The Kvaser firmware can only read and write messages that are
not crossing the USB endpoint's wMaxPacketSize boundary. While
receiving commands from the CAN device, if the next command in
the same URB buffer crossed that max packet size boundary, the
firmware puts a zero-length placeholder command in its place
then moves the real command to the next boundary mark.

The driver did not recognize such behavior, leading to missing
a good number of rx events during a heavy rx load session.

Moreover, a tx URB context only gets freed upon receiving its
respective tx ACK event. Over time, the free tx URB contexts
pool gets depleted due to the missing ACK events. Consequently,
the netif transmission queue gets __permanently__ stopped; no
frames could be sent again except after restarting the CAN
newtwork interface.

Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
---
 drivers/net/can/usb/kvaser_usb.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index d986fe8..a316fa4 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -14,6 +14,7 @@
  * Copyright (C) 2015 Valeo S.A.
  */
 
+#include <linux/kernel.h>
 #include <linux/completion.h>
 #include <linux/module.h>
 #include <linux/netdevice.h>
@@ -584,8 +585,15 @@ static int kvaser_usb_wait_msg(const struct kvaser_usb *dev, u8 id,
 		while (pos <= actual_len - MSG_HEADER_LEN) {
 			tmp = buf + pos;
 
-			if (!tmp->len)
-				break;
+			/* Handle messages crossing the USB endpoint max packet
+			 * size boundary. Check kvaser_usb_read_bulk_callback()
+			 * for further details.
+			 */
+			if (tmp->len == 0) {
+				pos = round_up(pos,
+					       dev->bulk_in->wMaxPacketSize);
+				continue;
+			}
 
 			if (pos + tmp->len > actual_len) {
 				dev_err(dev->udev->dev.parent,
@@ -1316,8 +1324,19 @@ static void kvaser_usb_read_bulk_callback(struct urb *urb)
 	while (pos <= urb->actual_length - MSG_HEADER_LEN) {
 		msg = urb->transfer_buffer + pos;
 
-		if (!msg->len)
-			break;
+		/* The Kvaser firmware can only read and write messages that
+		 * does not cross the USB's endpoint wMaxPacketSize boundary.
+		 * If a follow-up command crosses such boundary, firmware puts
+		 * a placeholder zero-length command in its place then aligns
+		 * the real command to the next max packet size.
+		 *
+		 * Handle such cases or we're going to miss a significant
+		 * number of events in case of a heavy rx load on the bus.
+		 */
+		if (msg->len == 0) {
+			pos = round_up(pos, dev->bulk_in->wMaxPacketSize);
+			continue;
+		}
 
 		if (pos + msg->len > urb->actual_length) {
 			dev_err(dev->udev->dev.parent, "Format error\n");
@@ -1325,7 +1344,6 @@ static void kvaser_usb_read_bulk_callback(struct urb *urb)
 		}
 
 		kvaser_usb_handle_message(dev, msg);
-
 		pos += msg->len;
 	}
 
-- 
1.9.1


  reply index

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-26 15:20 [PATCH 1/5] can: kvaser_usb: Avoid double free on URB submission failures Ahmed S. Darwish
2015-02-26 15:22 ` Ahmed S. Darwish [this message]
2015-02-26 15:24   ` [PATCH 3/5] can: kvaser_usb: Utilize all possible tx URBs Ahmed S. Darwish
2015-02-26 15:25     ` [PATCH 4/5] can: kvaser_usb: Use can-dev unregistration mechanism Ahmed S. Darwish
2015-02-26 15:29       ` [PATCH 5/5] can: kvaser_usb: Fix tx queue start/stop race conditions Ahmed S. Darwish
2015-03-14 14:26   ` [PATCH 2/5] can: kvaser_usb: Read all messages in a bulk-in URB buffer Marc Kleine-Budde
2015-03-04  9:15 ` [PATCH 1/5] can: kvaser_usb: Avoid double free on URB submission failures Marc Kleine-Budde
2015-03-09 12:32   ` Ahmed S. Darwish
2015-03-09 12:56     ` Marc Kleine-Budde
2015-03-11 15:23 ` [PATCH v2 1/3] can: kvaser_usb: Fix tx queue start/stop race conditions Ahmed S. Darwish
2015-03-11 15:28   ` [PATCH v2 2/3] can: kvaser_usb: Utilize all possible tx URBs Ahmed S. Darwish
2015-03-11 15:30     ` [PATCH v2 3/3] can: kvaser_usb: Use can-dev unregistration mechanism Ahmed S. Darwish
2015-03-11 15:36   ` [PATCH v2 1/3] can: kvaser_usb: Fix tx queue start/stop race conditions Marc Kleine-Budde
2015-03-11 15:57     ` Ahmed S. Darwish
2015-03-11 17:37 ` [PATCH v3 " Ahmed S. Darwish
2015-03-11 17:39   ` [PATCH v3 2/3] can: kvaser_usb: Utilize all possible tx URBs Ahmed S. Darwish
2015-03-11 17:39     ` [PATCH v3 3/3] can: kvaser_usb: Use can-dev unregistration mechanism Ahmed S. Darwish
2015-03-11 21:53     ` [PATCH v3 2/3] can: kvaser_usb: Utilize all possible tx URBs Marc Kleine-Budde
2015-03-12 10:52       ` Ahmed S. Darwish
2015-03-12 11:29         ` Marc Kleine-Budde
2015-03-11 21:43   ` [PATCH v3 1/3] can: kvaser_usb: Fix tx queue start/stop race conditions Marc Kleine-Budde
2015-03-12 19:30     ` Ahmed S. Darwish
2015-03-14 13:02 ` [PATCH v4 " Ahmed S. Darwish
2015-03-14 13:09   ` [PATCH v4 2/3] can: kvaser_usb: Utilize all possible tx URBs Ahmed S. Darwish
2015-03-14 13:11     ` [PATCH v4 3/3] can: kvaser_usb: Use can-dev unregistration mechanism Ahmed S. Darwish
2015-03-14 15:26       ` Marc Kleine-Budde
2015-03-14 15:41         ` Ahmed S. Darwish
2015-03-14 15:55           ` Marc Kleine-Budde
2015-03-14 16:06             ` Ahmed S. Darwish
2015-03-14 13:41   ` [PATCH v4 1/3] can: kvaser_usb: Fix tx queue start/stop race conditions Marc Kleine-Budde
2015-03-14 14:38     ` Ahmed S. Darwish
2015-03-14 14:58       ` Marc Kleine-Budde
2015-03-14 15:19         ` Ahmed S. Darwish
2015-03-15 15:03 ` [PATCH v5 1/2] can: kvaser_usb: Comply with firmware max tx URBs value Ahmed S. Darwish
2015-03-15 15:10   ` [PATCH v5 2/2] can: kvaser_usb: Fix sparse warning __le16 degrades to integer Ahmed S. Darwish
2015-03-15 18:08   ` [PATCH v5 1/2] can: kvaser_usb: Comply with firmware max tx URBs value Marc Kleine-Budde
2015-03-16 12:16     ` Ahmed S. Darwish
2015-03-16 12:56       ` Marc Kleine-Budde

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=20150226152202.GB6075@linux \
    --to=darwish.07@gmail.com \
    --cc=andri.yngvason@marel.com \
    --cc=linux-can@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=olivier@sobrie.be \
    --cc=socketcan@hartkopp.net \
    --cc=wg@grandegger.com \
    /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

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lkml.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lkml.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lkml.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lkml.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lkml.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lkml.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lkml.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lkml.kernel.org/lkml/7 lkml/git/7.git
	git clone --mirror https://lkml.kernel.org/lkml/8 lkml/git/8.git
	git clone --mirror https://lkml.kernel.org/lkml/9 lkml/git/9.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lkml.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git