netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Marc Kleine-Budde <mkl@pengutronix.de>
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, kuba@kernel.org, linux-can@vger.kernel.org,
	kernel@pengutronix.de, Marc Kleine-Budde <mkl@pengutronix.de>
Subject: [PATCH net-next 21/21] can: gs_usb: convert to NAPI/rx-offload to avoid OoO reception
Date: Fri, 28 Jul 2023 09:56:14 +0200	[thread overview]
Message-ID: <20230728075614.1014117-22-mkl@pengutronix.de> (raw)
In-Reply-To: <20230728075614.1014117-1-mkl@pengutronix.de>

The driver used to pass received CAN frames/skbs to the network stack
with netif_rx(). In netif_rx() the skbs are queued to the local CPU.
If IRQs are handled in round robin, OoO packets may occur.

To avoid out-of-order reception convert the driver from netif_rx() to
NAPI.

For USB devices with timestamping support use the rx-offload helper
can_rx_offload_queue_timestamp() for the RX, and
can_rx_offload_get_echo_skb_queue_timestamp() for the TX path. Devices
without timestamping support use can_rx_offload_queue_tail() for RX,
and can_rx_offload_get_echo_skb_queue_tail() for the TX path.

Link: https://lore.kernel.org/all/559D628C.5020100@hartkopp.net
Link: https://github.com/candle-usb/candleLight_fw/issues/166
Link: https://lore.kernel.org/all/20230718-gs_usb-rx-offload-v2-3-716e542d14d5@pengutronix.de
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/usb/Kconfig  |  1 +
 drivers/net/can/usb/gs_usb.c | 85 ++++++++++++++++++++++++++++--------
 2 files changed, 67 insertions(+), 19 deletions(-)

diff --git a/drivers/net/can/usb/Kconfig b/drivers/net/can/usb/Kconfig
index 58fcd2b34820..d1450722cb3c 100644
--- a/drivers/net/can/usb/Kconfig
+++ b/drivers/net/can/usb/Kconfig
@@ -52,6 +52,7 @@ config CAN_F81604
 
 config CAN_GS_USB
 	tristate "Geschwister Schneider UG and candleLight compatible interfaces"
+	select CAN_RX_OFFLOAD
 	help
 	  This driver supports the Geschwister Schneider and
 	  bytewerk.org candleLight compatible
diff --git a/drivers/net/can/usb/gs_usb.c b/drivers/net/can/usb/gs_usb.c
index 6caf3974e028..95b0fdb602c8 100644
--- a/drivers/net/can/usb/gs_usb.c
+++ b/drivers/net/can/usb/gs_usb.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2013-2016 Geschwister Schneider Technologie-,
  * Entwicklungs- und Vertriebs UG (Haftungsbeschränkt).
  * Copyright (C) 2016 Hubert Denkmair
+ * Copyright (c) 2023 Pengutronix, Marc Kleine-Budde <kernel@pengutronix.de>
  *
  * Many thanks to all socketcan devs!
  */
@@ -24,6 +25,7 @@
 #include <linux/can.h>
 #include <linux/can/dev.h>
 #include <linux/can/error.h>
+#include <linux/can/rx-offload.h>
 
 /* Device specific constants */
 #define USB_GS_USB_1_VENDOR_ID 0x1d50
@@ -282,6 +284,8 @@ struct gs_host_frame {
 #define GS_MAX_TX_URBS 10
 /* Only launch a max of GS_MAX_RX_URBS usb requests at a time. */
 #define GS_MAX_RX_URBS 30
+#define GS_NAPI_WEIGHT 32
+
 /* Maximum number of interfaces the driver supports per device.
  * Current hardware only supports 3 interfaces. The future may vary.
  */
@@ -295,6 +299,7 @@ struct gs_tx_context {
 struct gs_can {
 	struct can_priv can; /* must be the first member */
 
+	struct can_rx_offload offload;
 	struct gs_usb *parent;
 
 	struct net_device *netdev;
@@ -506,20 +511,59 @@ static void gs_update_state(struct gs_can *dev, struct can_frame *cf)
 	}
 }
 
-static void gs_usb_set_timestamp(struct gs_can *dev, struct sk_buff *skb,
-				 const struct gs_host_frame *hf)
+static u32 gs_usb_set_timestamp(struct gs_can *dev, struct sk_buff *skb,
+				const struct gs_host_frame *hf)
 {
 	u32 timestamp;
 
-	if (!(dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP))
-		return;
-
 	if (hf->flags & GS_CAN_FLAG_FD)
 		timestamp = le32_to_cpu(hf->canfd_ts->timestamp_us);
 	else
 		timestamp = le32_to_cpu(hf->classic_can_ts->timestamp_us);
 
-	gs_usb_skb_set_timestamp(dev, skb, timestamp);
+	if (skb)
+		gs_usb_skb_set_timestamp(dev, skb, timestamp);
+
+	return timestamp;
+}
+
+static void gs_usb_rx_offload(struct gs_can *dev, struct sk_buff *skb,
+			      const struct gs_host_frame *hf)
+{
+	struct can_rx_offload *offload = &dev->offload;
+	int rc;
+
+	if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP) {
+		const u32 ts = gs_usb_set_timestamp(dev, skb, hf);
+
+		rc = can_rx_offload_queue_timestamp(offload, skb, ts);
+	} else {
+		rc = can_rx_offload_queue_tail(offload, skb);
+	}
+
+	if (rc)
+		dev->netdev->stats.rx_fifo_errors++;
+}
+
+static unsigned int
+gs_usb_get_echo_skb(struct gs_can *dev, struct sk_buff *skb,
+		    const struct gs_host_frame *hf)
+{
+	struct can_rx_offload *offload = &dev->offload;
+	const u32 echo_id = hf->echo_id;
+	unsigned int len;
+
+	if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP) {
+		const u32 ts = gs_usb_set_timestamp(dev, skb, hf);
+
+		len = can_rx_offload_get_echo_skb_queue_timestamp(offload, echo_id,
+								  ts, NULL);
+	} else {
+		len = can_rx_offload_get_echo_skb_queue_tail(offload, echo_id,
+							     NULL);
+	}
+
+	return len;
 }
 
 static void gs_usb_receive_bulk_callback(struct urb *urb)
@@ -592,12 +636,7 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
 				gs_update_state(dev, cf);
 		}
 
-		gs_usb_set_timestamp(dev, skb, hf);
-
-		stats->rx_packets++;
-		stats->rx_bytes += hf->can_dlc;
-
-		netif_rx(skb);
+		gs_usb_rx_offload(dev, skb, hf);
 	} else { /* echo_id == hf->echo_id */
 		if (hf->echo_id >= GS_MAX_TX_URBS) {
 			netdev_err(netdev,
@@ -617,12 +656,8 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
 		}
 
 		skb = dev->can.echo_skb[hf->echo_id];
-		gs_usb_set_timestamp(dev, skb, hf);
-
 		stats->tx_packets++;
-		stats->tx_bytes += can_get_echo_skb(netdev, hf->echo_id,
-						    NULL);
-
+		stats->tx_bytes += gs_usb_get_echo_skb(dev, skb, hf);
 		gs_free_tx_context(txc);
 
 		atomic_dec(&dev->active_tx_urbs);
@@ -641,9 +676,12 @@ static void gs_usb_receive_bulk_callback(struct urb *urb)
 		cf->can_id |= CAN_ERR_CRTL;
 		cf->len = CAN_ERR_DLC;
 		cf->data[1] = CAN_ERR_CRTL_RX_OVERFLOW;
-		netif_rx(skb);
+
+		gs_usb_rx_offload(dev, skb, hf);
 	}
 
+	can_rx_offload_irq_finish(&dev->offload);
+
 resubmit_urb:
 	usb_fill_bulk_urb(urb, parent->udev,
 			  usb_rcvbulkpipe(parent->udev, GS_USB_ENDPOINT_IN),
@@ -857,6 +895,8 @@ static int gs_can_open(struct net_device *netdev)
 			dev->hf_size_tx = struct_size(hf, classic_can, 1);
 	}
 
+	can_rx_offload_enable(&dev->offload);
+
 	if (!parent->active_channels) {
 		if (dev->feature & GS_CAN_FEATURE_HW_TIMESTAMP)
 			gs_usb_timestamp_init(parent);
@@ -965,6 +1005,7 @@ static int gs_can_open(struct net_device *netdev)
 			gs_usb_timestamp_stop(parent);
 	}
 
+	can_rx_offload_disable(&dev->offload);
 	close_candev(netdev);
 
 	return rc;
@@ -1037,6 +1078,8 @@ static int gs_can_close(struct net_device *netdev)
 		dev->tx_context[rc].echo_id = GS_MAX_TX_URBS;
 	}
 
+	can_rx_offload_disable(&dev->offload);
+
 	/* close the netdev */
 	close_candev(netdev);
 
@@ -1336,6 +1379,7 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 		dev->can.data_bittiming_const = &dev->data_bt_const;
 	}
 
+	can_rx_offload_add_manual(netdev, &dev->offload, GS_NAPI_WEIGHT);
 	SET_NETDEV_DEV(netdev, &intf->dev);
 
 	rc = register_candev(dev->netdev);
@@ -1343,11 +1387,13 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 		dev_err(&intf->dev,
 			"Couldn't register candev for channel %d (%pe)\n",
 			channel, ERR_PTR(rc));
-		goto out_free_candev;
+		goto out_can_rx_offload_del;
 	}
 
 	return dev;
 
+out_can_rx_offload_del:
+	can_rx_offload_del(&dev->offload);
 out_free_candev:
 	free_candev(dev->netdev);
 	return ERR_PTR(rc);
@@ -1356,6 +1402,7 @@ static struct gs_can *gs_make_candev(unsigned int channel,
 static void gs_destroy_candev(struct gs_can *dev)
 {
 	unregister_candev(dev->netdev);
+	can_rx_offload_del(&dev->offload);
 	free_candev(dev->netdev);
 }
 
-- 
2.40.1



      parent reply	other threads:[~2023-07-28  7:59 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-28  7:55 [PATCH net-next 0/21] pull-request: can-next 2023-07-28 Marc Kleine-Budde
2023-07-28  7:55 ` [PATCH net-next 01/21] dt-bindings: net: can: Add support for Allwinner D1 CAN controller Marc Kleine-Budde
2023-07-28  9:00   ` patchwork-bot+netdevbpf
2023-07-28  7:55 ` [PATCH net-next 02/21] riscv: dts: allwinner: d1: Add CAN controller nodes Marc Kleine-Budde
2023-07-28  7:55 ` [PATCH net-next 03/21] can: sun4i_can: Add acceptance register quirk Marc Kleine-Budde
2023-07-28  7:55 ` [PATCH net-next 04/21] MAINTAINERS: Add myself as maintainer of the ems_pci.c driver Marc Kleine-Budde
2023-07-28  7:55 ` [PATCH net-next 05/21] can: sun4i_can: Add support for the Allwinner D1 Marc Kleine-Budde
2023-07-28  7:55 ` [PATCH net-next 06/21] can: peak_usb: remove unused/legacy peak_usb_netif_rx() function Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 07/21] can: Explicitly include correct DT includes, part 2 Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 08/21] can: gs_usb: remove leading space from goto labels Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 09/21] can: gs_usb: gs_usb_probe(): align block comment Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 10/21] can: gs_usb: gs_usb_set_timestamp(): remove return statements form void function Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 11/21] can: gs_usb: uniformly use "parent" as variable name for struct gs_usb Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 12/21] can: gs_usb: gs_usb_receive_bulk_callback(): make use of netdev Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 13/21] can: gs_usb: gs_usb_receive_bulk_callback(): make use of stats Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 14/21] can: gs_usb: gs_usb_receive_bulk_callback(): count RX overflow errors also in case of OOM Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 15/21] can: gs_usb: gs_can_start_xmit(), gs_can_open(): clean up printouts in error path Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 16/21] can: gs_usb: gs_can_close(): don't complain about failed device reset during ndo_stop Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 17/21] can: gs_usb: gs_destroy_candev(): remove not needed usb_kill_anchored_urbs() Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 18/21] can: gs_usb: gs_usb_disconnect(): " Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 19/21] can: rx-offload: rename rx_offload_get_echo_skb() -> can_rx_offload_get_echo_skb_queue_timestamp() Marc Kleine-Budde
2023-07-28  7:56 ` [PATCH net-next 20/21] can: rx-offload: add can_rx_offload_get_echo_skb_queue_tail() Marc Kleine-Budde
2023-07-28  7:56 ` Marc Kleine-Budde [this message]

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=20230728075614.1014117-22-mkl@pengutronix.de \
    --to=mkl@pengutronix.de \
    --cc=davem@davemloft.net \
    --cc=kernel@pengutronix.de \
    --cc=kuba@kernel.org \
    --cc=linux-can@vger.kernel.org \
    --cc=netdev@vger.kernel.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).