[PATCH v2 1/3] can: ctucanfd: enable NAPI before starting the controller

[PATCH v2 1/3] can: ctucanfd: enable NAPI before starting the controller

[Patrick Zhang]

ctucan_open() starts the controller and unmasks RBNEI in
ctucan_chip_start(), prints "ctu_can_fd device registered", and only
then calls napi_enable(). A frame that arrives in that window raises
RBNEI. ctucan_interrupt() masks RBNEI and calls napi_schedule(). NAPI is
still disabled, so napi_schedule_prep() finds NAPI_STATE_SCHED set and
the schedule is dropped. napi_enable() then clears SCHED without
scheduling anything. Only ctucan_rx_poll() unmasks RBNEI again, and it
never runs. The interface receives nothing until the next close.
Transmit still works. The RX FIFO fills up and the core sets DOR, but
the overrun is only accounted in ctucan_rx_poll(), so rx_errors and
rx_over_errors stay at zero and nothing is logged. Every re-open (ip
link down/up, a driver rebind) runs the same race again.

The netdev_info() in the window makes it much wider on a slow console.
This can be reproduced by sending continuous traffic with cangen from
another node and repeating "ip link set down/up" on the test device.

Enable NAPI before the controller starts, so that an early RX interrupt
can schedule the poll. NAPI can be enabled while the controller is in
reset, because nothing schedules it until an interrupt arrives. If
ctucan_chip_start() fails, disable NAPI again. Otherwise the next open
would call napi_enable() on an already enabled instance and trip the
BUG_ON() in napi_enable_locked().

Tested on QEMU with the ctucan_pci device.

Fixes: 2dcb8e8782d8 ("can: ctucanfd: add support for CTU CAN FD open-source IP core - bus independent part.")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Patrick Zhang <patrzhan@amazon.com>
Acked-by: Pavel Pisa <pisa@fel.cvut.cz>
Tested-by: Pavel Pisa <pisa@fel.cvut.cz>
---

Notes:
    v2:
    - Add Acked-by and Tested-by from Pavel
    - v1: https://lore.kernel.org/r/20261021205924.2538511-2-patrzhan@amazon.com

 drivers/net/can/ctucanfd/ctucanfd_base.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
index 07d4aa43c..82bf6c645 100644
--- a/drivers/net/can/ctucanfd/ctucanfd_base.c
+++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
@@ -1231,6 +1231,10 @@ static int ctucan_open(struct net_device *ndev)
 		goto err_irq;
 	}
 
+	/* Keep before ctucan_chip_start(): the ISR masks RBNEI and only the
+	 * poll unmasks it.
+	 */
+	napi_enable(&priv->napi);
 	ret = ctucan_chip_start(ndev);
 	if (ret < 0) {
 		netdev_err(ndev, "ctucan_chip_start failed!\n");
@@ -1238,12 +1242,12 @@ static int ctucan_open(struct net_device *ndev)
 	}
 
 	netdev_info(ndev, "ctu_can_fd device registered\n");
-	napi_enable(&priv->napi);
 	netif_start_queue(ndev);
 
 	return 0;
 
 err_chip_start:
+	napi_disable(&priv->napi);
 	free_irq(ndev->irq, ndev);
 err_irq:
 	close_candev(ndev);

base-commit: abeacca61eea4f6803df4a595b656ea45c2d3249
-- 
2.50.1

[PATCH v2 2/3] can: xilinx_can: enable NAPI before starting the controller

[Patrick Zhang]

xcan_open() calls xcan_chip_start(), which writes XCAN_IER_OFFSET to
unmask the RX interrupts and then sets XCAN_SRR_CEN_MASK to enable the
controller, and only then calls napi_enable(). A frame that arrives in
that window raises the RX interrupt. xcan_interrupt() clears the RX
bits in XCAN_IER_OFFSET and calls napi_schedule(). NAPI is still
disabled, so napi_schedule_prep() finds NAPI_STATE_SCHED set and the
schedule is dropped. Only xcan_rx_poll() unmasks RX again, and it never
runs. The interface receives no data frames until the next close.
Transmit still works. The RX overflow interrupt stays enabled, so once
the RX FIFO fills, xcan_err_interrupt() counts rx_over_errors and
reports overflow error frames, but nothing reads the FIFO. Every
re-open (ip link down/up, a driver rebind) runs the same race again.

The window is only the tail of xcan_chip_start(), so the failure is
rare. Adding an mdelay(50) at the end of xcan_chip_start() makes it
reproducible with continuous traffic from another node and a single
"ip link set down/up" on the test device.

Enable NAPI before the controller starts, so that an early RX interrupt
can schedule the poll. NAPI can be enabled while the controller is in
reset, because nothing schedules it until an interrupt arrives. If
xcan_chip_start() fails, disable NAPI again. Otherwise the next open
would call napi_enable() on an already enabled instance and trip the
BUG_ON() in napi_enable_locked().

This is the same race as in "can: ctucanfd: enable NAPI before starting
the controller".

Tested on a Versal board.

Fixes: b1201e44f50b ("can: xilinx CAN controller support")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Patrick Zhang <patrzhan@amazon.com>
---

Notes:
    v2:
    - Correct the commit message: the RX overflow interrupt stays enabled
      during the stall, so rx_over_errors is counted once the FIFO fills
    - Name the ctucanfd fix by subject instead of "the previous patch"
    - v1: https://lore.kernel.org/r/20261021205924.2538511-3-patrzhan@amazon.com

 drivers/net/can/xilinx_can.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 43d7f2282..5c2412bf9 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -1570,18 +1570,22 @@ static int xcan_open(struct net_device *ndev)
 	if (ret)
 		goto err_irq;
 
+	/* Keep before xcan_chip_start(): the ISR masks RX interrupts and only
+	 * the poll unmasks them.
+	 */
+	napi_enable(&priv->napi);
 	ret = xcan_chip_start(ndev);
 	if (ret < 0) {
 		netdev_err(ndev, "xcan_chip_start failed!\n");
 		goto err_candev;
 	}
 
-	napi_enable(&priv->napi);
 	netif_start_queue(ndev);
 
 	return 0;
 
 err_candev:
+	napi_disable(&priv->napi);
 	close_candev(ndev);
 err_irq:
 	free_irq(ndev->irq, ndev);
-- 
2.50.1

Re: [PATCH v2 2/3] can: xilinx_can: enable NAPI before starting the controller

[T, Harini]

Thanks Patrick,

On 10/31/2026 5:03 AM, Patrick Zhang wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> xcan_open() calls xcan_chip_start(), which writes XCAN_IER_OFFSET to
> unmask the RX interrupts and then sets XCAN_SRR_CEN_MASK to enable the
> controller, and only then calls napi_enable(). A frame that arrives in
> that window raises the RX interrupt. xcan_interrupt() clears the RX
> bits in XCAN_IER_OFFSET and calls napi_schedule(). NAPI is still
> disabled, so napi_schedule_prep() finds NAPI_STATE_SCHED set and the
> schedule is dropped. Only xcan_rx_poll() unmasks RX again, and it never
> runs. The interface receives no data frames until the next close.
> Transmit still works. The RX overflow interrupt stays enabled, so once
> the RX FIFO fills, xcan_err_interrupt() counts rx_over_errors and
> reports overflow error frames, but nothing reads the FIFO. Every
> re-open (ip link down/up, a driver rebind) runs the same race again.
> 
> The window is only the tail of xcan_chip_start(), so the failure is
> rare. Adding an mdelay(50) at the end of xcan_chip_start() makes it
> reproducible with continuous traffic from another node and a single
> "ip link set down/up" on the test device.
> 
> Enable NAPI before the controller starts, so that an early RX interrupt
> can schedule the poll. NAPI can be enabled while the controller is in
> reset, because nothing schedules it until an interrupt arrives. If
> xcan_chip_start() fails, disable NAPI again. Otherwise the next open
> would call napi_enable() on an already enabled instance and trip the
> BUG_ON() in napi_enable_locked().
> 
> This is the same race as in "can: ctucanfd: enable NAPI before starting
> the controller".
> 
> Tested on a Versal board.
> 
> Fixes: b1201e44f50b ("can: xilinx CAN controller support")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Patrick Zhang <patrzhan@amazon.com>

LGTM.

Reviewed-by: Harini T <harini.t@amd.com>

> ---
> 
> Notes:
>      v2:
>      - Correct the commit message: the RX overflow interrupt stays enabled
>        during the stall, so rx_over_errors is counted once the FIFO fills
>      - Name the ctucanfd fix by subject instead of "the previous patch"
>      - v1: https://lore.kernel.org/r/20261021205924.2538511-3-patrzhan@amazon.com
> 
>   drivers/net/can/xilinx_can.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
> index 43d7f2282..5c2412bf9 100644
> --- a/drivers/net/can/xilinx_can.c
> +++ b/drivers/net/can/xilinx_can.c
> @@ -1570,18 +1570,22 @@ static int xcan_open(struct net_device *ndev)
>          if (ret)
>                  goto err_irq;
> 
> +       /* Keep before xcan_chip_start(): the ISR masks RX interrupts and only
> +        * the poll unmasks them.
> +        */
> +       napi_enable(&priv->napi);
>          ret = xcan_chip_start(ndev);
>          if (ret < 0) {
>                  netdev_err(ndev, "xcan_chip_start failed!\n");
>                  goto err_candev;
>          }
> 
> -       napi_enable(&priv->napi);
>          netif_start_queue(ndev);
> 
>          return 0;
> 
>   err_candev:
> +       napi_disable(&priv->napi);
>          close_candev(ndev);
>   err_irq:
>          free_irq(ndev->irq, ndev);
> --
> 2.50.1