From: Thorsten Scherer <t.scherer@eckelmann.de>
To: Marc Kleine-Budde <mkl@pengutronix.de>
Cc: linux-can@vger.kernel.org,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>
Subject: Re: [PATCH can v2] can: flexcan: flexcan_mailbox_read() fix return value for drop = true
Date: Fri, 12 Aug 2022 12:46:38 +0200 [thread overview]
Message-ID: <20220812104638.b4spriz4jwhdskxk@NB142.eckelmann.group> (raw)
In-Reply-To: <20220811094254.1864367-1-mkl@pengutronix.de>
Hello,
On Thu, Aug 11, 2022 at 11:42:54AM +0200, Marc Kleine-Budde wrote:
> The following happened on an i.MX25 using flexcan with many packets on
> the bus:
>
> The rx-offload queue reached a length more than skb_queue_len_max. In
> can_rx_offload_offload_one() the drop variable was set to true which
> made the call to .mailbox_read() (here: flexcan_mailbox_read()) to
> _always_ return ERR_PTR(-ENOBUFS) and drop the rx'ed CAN frame. So
> can_rx_offload_offload_one() returned ERR_PTR(-ENOBUFS), too.
>
> can_rx_offload_irq_offload_fifo() looks as follows:
>
> | while (1) {
> | skb = can_rx_offload_offload_one(offload, 0);
> | if (IS_ERR(skb))
> | continue;
> | if (!skb)
> | break;
> | ...
> | }
>
> The flexcan driver wrongly always returns ERR_PTR(-ENOBUFS) if drop is
> requested, even if there is no CAN frame pending. As the i.MX25 is a
> single core CPU, while the rx-offload processing is active, there is
> no thread to process packets from the offload queue. So the queue
> doesn't get any shorter and this results is a tight loop.
>
> Instead of always returning ERR_PTR(-ENOBUFS) if drop is requested,
> return NULL if no CAN frame is pending.
>
> Fixes: 4e9c9484b085 ("can: rx-offload: Prepare for CAN FD support")
> Suggested-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
This patch (rebased onto v5.15) fixes the issue on our i.MX25 board.
Tested-by: Thorsten Scherer <t.scherer@eckelmann.de>
Thank you!
Best regards
Thorsten
> ---
> drivers/net/can/flexcan/flexcan-core.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/can/flexcan/flexcan-core.c b/drivers/net/can/flexcan/flexcan-core.c
> index f857968efed7..ccb438eca517 100644
> --- a/drivers/net/can/flexcan/flexcan-core.c
> +++ b/drivers/net/can/flexcan/flexcan-core.c
> @@ -941,11 +941,6 @@ static struct sk_buff *flexcan_mailbox_read(struct can_rx_offload *offload,
> u32 reg_ctrl, reg_id, reg_iflag1;
> int i;
>
> - if (unlikely(drop)) {
> - skb = ERR_PTR(-ENOBUFS);
> - goto mark_as_read;
> - }
> -
> mb = flexcan_get_mb(priv, n);
>
> if (priv->devtype_data.quirks & FLEXCAN_QUIRK_USE_RX_MAILBOX) {
> @@ -974,6 +969,11 @@ static struct sk_buff *flexcan_mailbox_read(struct can_rx_offload *offload,
> reg_ctrl = priv->read(&mb->can_ctrl);
> }
>
> + if (unlikely(drop)) {
> + skb = ERR_PTR(-ENOBUFS);
> + goto mark_as_read;
> + }
> +
> if (reg_ctrl & FLEXCAN_MB_CNT_EDL)
> skb = alloc_canfd_skb(offload->dev, &cfd);
> else
> --
> 2.35.1
>
>
prev parent reply other threads:[~2022-08-12 10:49 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-11 9:42 [PATCH can v2] can: flexcan: flexcan_mailbox_read() fix return value for drop = true Marc Kleine-Budde
2022-08-12 9:19 ` Uwe Kleine-König
2022-08-12 10:46 ` Thorsten Scherer [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=20220812104638.b4spriz4jwhdskxk@NB142.eckelmann.group \
--to=t.scherer@eckelmann.de \
--cc=linux-can@vger.kernel.org \
--cc=mkl@pengutronix.de \
--cc=u.kleine-koenig@pengutronix.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.