Commit 24bc41b4 authored by Marc Kleine-Budde's avatar Marc Kleine-Budde

can: gs_usb: convert to NAPI/rx-offload to avoid OoO reception

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