From 2e3df4a3b3178006d530f4c4d0e91f3d96cddb3c Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Fri, 2 Jun 2023 09:36:38 +0200 Subject: [PATCH 1/3] can: rx-offload: rename rx_offload_get_echo_skb() -> can_rx_offload_get_echo_skb_queue_timestamp() Rename the rx_offload_get_echo_skb() function to can_rx_offload_get_echo_skb_queue_timestamp(), since it inserts the echo skb into the rx-offload queue sorted by timestamp. This is a preparation for adding can_rx_offload_get_echo_skb_queue_tail(), which adds the echo skb to the end of the queue. This is intended for devices that do not support timestamps. Link: https://lore.kernel.org/all/20230718-gs_usb-rx-offload-v2-1-716e542d14d5@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/dev/rx-offload.c | 9 +++++---- drivers/net/can/flexcan/flexcan-core.c | 4 ++-- drivers/net/can/m_can/m_can.c | 8 ++++---- drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c | 6 +++--- drivers/net/can/ti_hecc.c | 4 ++-- include/linux/can/rx-offload.h | 6 +++--- 6 files changed, 19 insertions(+), 18 deletions(-) diff --git a/drivers/net/can/dev/rx-offload.c b/drivers/net/can/dev/rx-offload.c index 161e45a7e8c11..6e87ad9083467 100644 --- a/drivers/net/can/dev/rx-offload.c +++ b/drivers/net/can/dev/rx-offload.c @@ -240,9 +240,10 @@ int can_rx_offload_queue_timestamp(struct can_rx_offload *offload, } EXPORT_SYMBOL_GPL(can_rx_offload_queue_timestamp); -unsigned int can_rx_offload_get_echo_skb(struct can_rx_offload *offload, - unsigned int idx, u32 timestamp, - unsigned int *frame_len_ptr) +unsigned int +can_rx_offload_get_echo_skb_queue_timestamp(struct can_rx_offload *offload, + unsigned int idx, u32 timestamp, + unsigned int *frame_len_ptr) { struct net_device *dev = offload->dev; struct net_device_stats *stats = &dev->stats; @@ -262,7 +263,7 @@ unsigned int can_rx_offload_get_echo_skb(struct can_rx_offload *offload, return len; } -EXPORT_SYMBOL_GPL(can_rx_offload_get_echo_skb); +EXPORT_SYMBOL_GPL(can_rx_offload_get_echo_skb_queue_timestamp); int can_rx_offload_queue_tail(struct can_rx_offload *offload, struct sk_buff *skb) diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c index ff0fc18baf133..48273d51f2711 100644 --- a/drivers/net/can/flexcan/flexcan-core.c +++ b/drivers/net/can/flexcan/flexcan-core.c @@ -1097,8 +1097,8 @@ static irqreturn_t flexcan_irq(int irq, void *dev_id) handled = IRQ_HANDLED; stats->tx_bytes += - can_rx_offload_get_echo_skb(&priv->offload, 0, - reg_ctrl << 16, NULL); + can_rx_offload_get_echo_skb_queue_timestamp(&priv->offload, 0, + reg_ctrl << 16, NULL); stats->tx_packets++; /* after sending a RTR frame MB is in RX mode */ diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c index a6263a617ac93..d204703521d28 100644 --- a/drivers/net/can/m_can/m_can.c +++ b/drivers/net/can/m_can/m_can.c @@ -1016,10 +1016,10 @@ static void m_can_tx_update_stats(struct m_can_classdev *cdev, if (cdev->is_peripheral) stats->tx_bytes += - can_rx_offload_get_echo_skb(&cdev->offload, - msg_mark, - timestamp, - NULL); + can_rx_offload_get_echo_skb_queue_timestamp(&cdev->offload, + msg_mark, + timestamp, + NULL); else stats->tx_bytes += can_get_echo_skb(dev, msg_mark, NULL); diff --git a/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c b/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c index 237617b0c125f..e5bd57b65aafe 100644 --- a/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c +++ b/drivers/net/can/spi/mcp251xfd/mcp251xfd-tef.c @@ -111,9 +111,9 @@ mcp251xfd_handle_tefif_one(struct mcp251xfd_priv *priv, if (skb) mcp251xfd_skb_set_timestamp(priv, skb, hw_tef_obj->ts); stats->tx_bytes += - can_rx_offload_get_echo_skb(&priv->offload, - tef_tail, hw_tef_obj->ts, - frame_len_ptr); + can_rx_offload_get_echo_skb_queue_timestamp(&priv->offload, + tef_tail, hw_tef_obj->ts, + frame_len_ptr); stats->tx_packets++; priv->tef->tail++; diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c index a8243acde92d7..5aab440074c69 100644 --- a/drivers/net/can/ti_hecc.c +++ b/drivers/net/can/ti_hecc.c @@ -747,8 +747,8 @@ static irqreturn_t ti_hecc_interrupt(int irq, void *dev_id) spin_unlock_irqrestore(&priv->mbx_lock, flags); stamp = hecc_read_stamp(priv, mbxno); stats->tx_bytes += - can_rx_offload_get_echo_skb(&priv->offload, - mbxno, stamp, NULL); + can_rx_offload_get_echo_skb_queue_timestamp(&priv->offload, + mbxno, stamp, NULL); stats->tx_packets++; --priv->tx_tail; } diff --git a/include/linux/can/rx-offload.h b/include/linux/can/rx-offload.h index c205c51d79c90..e3b4199732c6a 100644 --- a/include/linux/can/rx-offload.h +++ b/include/linux/can/rx-offload.h @@ -44,9 +44,9 @@ int can_rx_offload_irq_offload_timestamp(struct can_rx_offload *offload, int can_rx_offload_irq_offload_fifo(struct can_rx_offload *offload); int can_rx_offload_queue_timestamp(struct can_rx_offload *offload, struct sk_buff *skb, u32 timestamp); -unsigned int can_rx_offload_get_echo_skb(struct can_rx_offload *offload, - unsigned int idx, u32 timestamp, - unsigned int *frame_len_ptr); +unsigned int can_rx_offload_get_echo_skb_queue_timestamp(struct can_rx_offload *offload, + unsigned int idx, u32 timestamp, + unsigned int *frame_len_ptr); int can_rx_offload_queue_tail(struct can_rx_offload *offload, struct sk_buff *skb); void can_rx_offload_irq_finish(struct can_rx_offload *offload); From 8e0e2950c9ef48f7f40a7175048744ec2390b16e Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Mon, 3 Jul 2023 18:18:19 +0200 Subject: [PATCH 2/3] can: rx-offload: add can_rx_offload_get_echo_skb_queue_tail() Add can_rx_offload_get_echo_skb_queue_tail(). This function addds the echo skb at the end of rx-offload the queue. This is intended for devices without timestamp support. Link: https://lore.kernel.org/all/20230718-gs_usb-rx-offload-v2-2-716e542d14d5@pengutronix.de Signed-off-by: Marc Kleine-Budde --- drivers/net/can/dev/rx-offload.c | 27 ++++++++++++++++++++++++++- include/linux/can/rx-offload.h | 5 ++++- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/drivers/net/can/dev/rx-offload.c b/drivers/net/can/dev/rx-offload.c index 6e87ad9083467..77091f7d1fa75 100644 --- a/drivers/net/can/dev/rx-offload.c +++ b/drivers/net/can/dev/rx-offload.c @@ -1,7 +1,7 @@ // SPDX-License-Identifier: GPL-2.0-only /* Copyright (c) 2014 Protonic Holland, * David Jander - * Copyright (C) 2014-2021 Pengutronix, + * Copyright (C) 2014-2021, 2023 Pengutronix, * Marc Kleine-Budde */ @@ -280,6 +280,31 @@ int can_rx_offload_queue_tail(struct can_rx_offload *offload, } EXPORT_SYMBOL_GPL(can_rx_offload_queue_tail); +unsigned int +can_rx_offload_get_echo_skb_queue_tail(struct can_rx_offload *offload, + unsigned int idx, + unsigned int *frame_len_ptr) +{ + struct net_device *dev = offload->dev; + struct net_device_stats *stats = &dev->stats; + struct sk_buff *skb; + unsigned int len; + int err; + + skb = __can_get_echo_skb(dev, idx, &len, frame_len_ptr); + if (!skb) + return 0; + + err = can_rx_offload_queue_tail(offload, skb); + if (err) { + stats->rx_errors++; + stats->tx_fifo_errors++; + } + + return len; +} +EXPORT_SYMBOL_GPL(can_rx_offload_get_echo_skb_queue_tail); + void can_rx_offload_irq_finish(struct can_rx_offload *offload) { unsigned long flags; diff --git a/include/linux/can/rx-offload.h b/include/linux/can/rx-offload.h index e3b4199732c6a..d29bb45219470 100644 --- a/include/linux/can/rx-offload.h +++ b/include/linux/can/rx-offload.h @@ -3,7 +3,7 @@ * linux/can/rx-offload.h * * Copyright (c) 2014 David Jander, Protonic Holland - * Copyright (c) 2014-2017 Pengutronix, Marc Kleine-Budde + * Copyright (c) 2014-2017, 2023 Pengutronix, Marc Kleine-Budde */ #ifndef _CAN_RX_OFFLOAD_H @@ -49,6 +49,9 @@ unsigned int can_rx_offload_get_echo_skb_queue_timestamp(struct can_rx_offload * unsigned int *frame_len_ptr); int can_rx_offload_queue_tail(struct can_rx_offload *offload, struct sk_buff *skb); +unsigned int can_rx_offload_get_echo_skb_queue_tail(struct can_rx_offload *offload, + unsigned int idx, + unsigned int *frame_len_ptr); void can_rx_offload_irq_finish(struct can_rx_offload *offload); void can_rx_offload_threaded_irq_finish(struct can_rx_offload *offload); void can_rx_offload_del(struct can_rx_offload *offload); From 24bc41b4558347672a3db61009c339b1f5692169 Mon Sep 17 00:00:00 2001 From: Marc Kleine-Budde Date: Fri, 2 Jun 2023 09:29:07 +0200 Subject: [PATCH 3/3] 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.de Signed-off-by: Marc Kleine-Budde --- 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 58fcd2b348207..d1450722cb3c8 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 6caf3974e0283..95b0fdb602c8f 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 * * Many thanks to all socketcan devs! */ @@ -24,6 +25,7 @@ #include #include #include +#include /* 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); }