pull-request: can 2015-03-09

pull-request: can 2015-03-09

[Marc Kleine-Budde]

Hello David,

this is a pull request for net/master for the 4.0 release cycle, it consists of
6 patches:

A patch by Oliver Hartkopp fixes a long outstanding bug in the infrastructure,
which leads to skb_under_panics when CAN interfaces are used by AF_PACKET
sockets e.g. by dhclient. Stephane Grosjean contributes a patch for the
peak_usb driver which adds a missing initialization. Two patches by Ahmed S.
Darwish fix problems in the kvaser_usb driver. Followed by two patches by
myself, updating the MAINTAINERS file.

regards,
Marc
---

The following changes since commit c247f0534cc5a5a547a343903f42295a471844e2:

  ip: fix error queue empty skb handling (2015-03-08 23:01:54 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.0-20150309

for you to fetch changes up to ee2daa00fbe62a491446d3385542d43be233d81c:

  MAINTAINERS: add Marc Kleine-Budde as co maintinaer for CAN networking layer (2015-03-09 10:22:45 +0100)

----------------------------------------------------------------
linux-can-fixes-for-4.0-20150309

----------------------------------------------------------------
Ahmed S. Darwish (2):
      can: kvaser_usb: Avoid double free on URB submission failures
      can: kvaser_usb: Read all messages in a bulk-in URB buffer

Marc Kleine-Budde (2):
      MAINTAINERS: linux-can moved to github
      MAINTAINERS: add Marc Kleine-Budde as co maintinaer for CAN networking layer

Oliver Hartkopp (1):
      can: add missing initialisations in CAN related skbuffs

Stephane Grosjean (1):
      can: peak_usb: fix missing ctrlmode_ init for every dev

 MAINTAINERS                                |  5 ++--
 drivers/net/can/dev.c                      |  8 +++++
 drivers/net/can/usb/kvaser_usb.c           | 48 +++++++++++++++++++-----------
 drivers/net/can/usb/peak_usb/pcan_usb_fd.c |  4 +++
 net/can/af_can.c                           |  3 ++
 5 files changed, 49 insertions(+), 19 deletions(-)

[PATCH 1/6] can: add missing initialisations in CAN related skbuffs

[Marc Kleine-Budde]

From: Oliver Hartkopp <socketcan@hartkopp.net>

When accessing CAN network interfaces with AF_PACKET sockets e.g. by dhclient
this can lead to a skb_under_panic due to missing skb initialisations.

Add the missing initialisations at the CAN skbuff creation times on driver
level (rx path) and in the network layer (tx path).

Reported-by: Austin Schuh <austin@peloton-tech.com>
Reported-by: Daniel Steer <daniel.steer@mclaren.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/dev.c | 8 ++++++++
 net/can/af_can.c      | 3 +++
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 3c82e02e3dae..b0f69248cb71 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -579,6 +579,10 @@ struct sk_buff *alloc_can_skb(struct net_device *dev, struct can_frame **cf)
 	skb->pkt_type = PACKET_BROADCAST;
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 
+	skb_reset_mac_header(skb);
+	skb_reset_network_header(skb);
+	skb_reset_transport_header(skb);
+
 	can_skb_reserve(skb);
 	can_skb_prv(skb)->ifindex = dev->ifindex;
 
@@ -603,6 +607,10 @@ struct sk_buff *alloc_canfd_skb(struct net_device *dev,
 	skb->pkt_type = PACKET_BROADCAST;
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 
+	skb_reset_mac_header(skb);
+	skb_reset_network_header(skb);
+	skb_reset_transport_header(skb);
+
 	can_skb_reserve(skb);
 	can_skb_prv(skb)->ifindex = dev->ifindex;
 
diff --git a/net/can/af_can.c b/net/can/af_can.c
index 66e08040ced7..32d710eaf1fc 100644
--- a/net/can/af_can.c
+++ b/net/can/af_can.c
@@ -259,6 +259,9 @@ int can_send(struct sk_buff *skb, int loop)
 		goto inval_skb;
 	}
 
+	skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+	skb_reset_mac_header(skb);
 	skb_reset_network_header(skb);
 	skb_reset_transport_header(skb);
 
-- 
2.1.4

[PATCH 2/6] can: peak_usb: fix missing ctrlmode_ init for every dev

[Marc Kleine-Budde]

From: Stephane Grosjean <s.grosjean@peak-system.com>

Fixes a missing initialization of ctrlmode and ctrlmode_supported fields,
for all other CAN devices than the first one. This fix only concerns
the PCAN-USB Pro FD dual-channels CAN-FD device made by PEAK-System.

Signed-off-by: Stephane Grosjean <s.grosjean@peak-system.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/peak_usb/pcan_usb_fd.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
index 962c3f027383..0bac0f14edc3 100644
--- a/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
+++ b/drivers/net/can/usb/peak_usb/pcan_usb_fd.c
@@ -879,6 +879,10 @@ static int pcan_usb_fd_init(struct peak_usb_device *dev)
 
 		pdev->usb_if = ppdev->usb_if;
 		pdev->cmd_buffer_addr = ppdev->cmd_buffer_addr;
+
+		/* do a copy of the ctrlmode[_supported] too */
+		dev->can.ctrlmode = ppdev->dev.can.ctrlmode;
+		dev->can.ctrlmode_supported = ppdev->dev.can.ctrlmode_supported;
 	}
 
 	pdev->usb_if->dev[dev->ctrl_idx] = dev;
-- 
2.1.4

[PATCH 3/6] can: kvaser_usb: Avoid double free on URB submission failures

[Marc Kleine-Budde]

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

Upon a URB submission failure, the driver calls usb_free_urb()
but then manually frees the URB buffer by itself.  Meanwhile
usb_free_urb() has alredy freed out that transfer buffer since
we're the only code path holding a reference to this URB.

Remove two of such invalid manual free().

Signed-off-by: Ahmed S. Darwish <ahmed.darwish@valeo.com>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/kvaser_usb.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/net/can/usb/kvaser_usb.c b/drivers/net/can/usb/kvaser_usb.c
index 2928f7003041..d986fe83c40d 100644
--- a/drivers/net/can/usb/kvaser_usb.c
+++ b/drivers/net/can/usb/kvaser_usb.c
@@ -787,7 +787,6 @@ static int kvaser_usb_simple_msg_async(struct kvaser_usb_net_priv *priv,
 		netdev_err(netdev, "Error transmitting URB\n");
 		usb_unanchor_urb(urb);
 		usb_free_urb(urb);
-		kfree(buf);
 		return err;
 	}
 
@@ -1615,8 +1614,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 	struct urb *urb;
 	void *buf;
 	struct kvaser_msg *msg;
-	int i, err;
-	int ret = NETDEV_TX_OK;
+	int i, err, ret = NETDEV_TX_OK;
 	u8 *msg_tx_can_flags = NULL;		/* GCC */
 
 	if (can_dropped_invalid_skb(netdev, skb))
@@ -1634,7 +1632,7 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 	if (!buf) {
 		stats->tx_dropped++;
 		dev_kfree_skb(skb);
-		goto nobufmem;
+		goto freeurb;
 	}
 
 	msg = buf;
@@ -1681,8 +1679,10 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 	/* This should never happen; it implies a flow control bug */
 	if (!context) {
 		netdev_warn(netdev, "cannot find free context\n");
+
+		kfree(buf);
 		ret =  NETDEV_TX_BUSY;
-		goto releasebuf;
+		goto freeurb;
 	}
 
 	context->priv = priv;
@@ -1719,16 +1719,12 @@ static netdev_tx_t kvaser_usb_start_xmit(struct sk_buff *skb,
 		else
 			netdev_warn(netdev, "Failed tx_urb %d\n", err);
 
-		goto releasebuf;
+		goto freeurb;
 	}
 
-	usb_free_urb(urb);
-
-	return NETDEV_TX_OK;
+	ret = NETDEV_TX_OK;
 
-releasebuf:
-	kfree(buf);
-nobufmem:
+freeurb:
 	usb_free_urb(urb);
 	return ret;
 }
-- 
2.1.4

[PATCH 4/6] can: kvaser_usb: Read all messages in a bulk-in URB buffer

[Marc Kleine-Budde]

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>
Cc: linux-stable <stable@vger.kernel.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 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 d986fe83c40d..a316fa4b91ab 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;
 	}
 
-- 
2.1.4

[PATCH 5/6] MAINTAINERS: linux-can moved to github

[Marc Kleine-Budde]

As gitorious will shut down at the end of May 2015, the linux-can website moved
to github. This patch reflects this change.

Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 MAINTAINERS | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 42f686f2e4b2..ce4380d7ab1a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2370,7 +2370,7 @@ F:	arch/x86/include/asm/tce.h
 CAN NETWORK LAYER
 M:	Oliver Hartkopp <socketcan@hartkopp.net>
 L:	linux-can@vger.kernel.org
-W:	http://gitorious.org/linux-can
+W:	https://github.com/linux-can
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
 S:	Maintained
@@ -2386,7 +2386,7 @@ CAN NETWORK DRIVERS
 M:	Wolfgang Grandegger <wg@grandegger.com>
 M:	Marc Kleine-Budde <mkl@pengutronix.de>
 L:	linux-can@vger.kernel.org
-W:	http://gitorious.org/linux-can
+W:	https://github.com/linux-can
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can-next.git
 S:	Maintained
-- 
2.1.4

[PATCH 6/6] MAINTAINERS: add Marc Kleine-Budde as co maintinaer for CAN networking layer

[Marc Kleine-Budde]

This patch adds Marc Kleine-Budde as a co maintinaer for CAN networking layer.

Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index ce4380d7ab1a..ba57e5d3ed5c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2369,6 +2369,7 @@ F:	arch/x86/include/asm/tce.h
 
 CAN NETWORK LAYER
 M:	Oliver Hartkopp <socketcan@hartkopp.net>
+M:	Marc Kleine-Budde <mkl@pengutronix.de>
 L:	linux-can@vger.kernel.org
 W:	https://github.com/linux-can
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git
-- 
2.1.4

Re: pull-request: can 2015-03-09

[Marc Kleine-Budde]

[-- Attachment #1: Type: text/plain, Size: 2751 bytes --]

On 03/09/2015 10:36 AM, Marc Kleine-Budde wrote:
> Hello David,

I've just noticed a typo in one of the subjects.

>> Marc Kleine-Budde (2):
>>       MAINTAINERS: linux-can moved to github
>>       MAINTAINERS: add Marc Kleine-Budde as co maintinaer for CAN networking layer
                                                  ^^^^^^^^^^

The series is updates, the tag re-created and I've force-pushed it to
the linux-can repo:

this is a pull request for net/master for the 4.0 release cycle, it consists of
6 patches:

A patch by Oliver Hartkopp fixes a long outstanding bug in the infrastructure,
which leads to skb_under_panics when CAN interfaces are used by AF_PACKET
sockets e.g. by dhclient. Stephane Grosjean contributes a patch for the
peak_usb driver which adds a missing initialization. Two patches by Ahmed S.
Darwish fix problems in the kvaser_usb driver. Followed by two patches by
myself, updating the MAINTAINERS file.

Marc

---

The following changes since commit c247f0534cc5a5a547a343903f42295a471844e2:

  ip: fix error queue empty skb handling (2015-03-08 23:01:54 -0400)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mkl/linux-can.git tags/linux-can-fixes-for-4.0-20150309

for you to fetch changes up to f7214cf29ca6c977ad2c428f2b832e9c66f2ee1b:

  MAINTAINERS: add Marc Kleine-Budde as co maintainer for CAN networking layer (2015-03-09 10:41:45 +0100)

----------------------------------------------------------------
linux-can-fixes-for-4.0-20150309

----------------------------------------------------------------
Ahmed S. Darwish (2):
      can: kvaser_usb: Avoid double free on URB submission failures
      can: kvaser_usb: Read all messages in a bulk-in URB buffer

Marc Kleine-Budde (2):
      MAINTAINERS: linux-can moved to github
      MAINTAINERS: add Marc Kleine-Budde as co maintainer for CAN networking layer

Oliver Hartkopp (1):
      can: add missing initialisations in CAN related skbuffs

Stephane Grosjean (1):
      can: peak_usb: fix missing ctrlmode_ init for every dev

 MAINTAINERS                                |  5 ++--
 drivers/net/can/dev.c                      |  8 +++++
 drivers/net/can/usb/kvaser_usb.c           | 48 +++++++++++++++++++-----------
 drivers/net/can/usb/peak_usb/pcan_usb_fd.c |  4 +++
 net/can/af_can.c                           |  3 ++
 5 files changed, 49 insertions(+), 19 deletions(-)

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: pull-request: can 2015-03-09

[David Miller]

From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Mon,  9 Mar 2015 10:36:15 +0100

> this is a pull request for net/master for the 4.0 release cycle, it consists of
> 6 patches:
> 
> A patch by Oliver Hartkopp fixes a long outstanding bug in the infrastructure,
> which leads to skb_under_panics when CAN interfaces are used by AF_PACKET
> sockets e.g. by dhclient. Stephane Grosjean contributes a patch for the
> peak_usb driver which adds a missing initialization. Two patches by Ahmed S.
> Darwish fix problems in the kvaser_usb driver. Followed by two patches by
> myself, updating the MAINTAINERS file.

Applied, thanks Marc.