[PATCH 0/3] can: enable NAPI before starting the controller in three drivers

[PATCH 0/3] can: enable NAPI before starting the controller in three drivers

[Patrick Zhang]

Hi,

Three CAN drivers open the device in the order "start the controller
and unmask its RX interrupt, then napi_enable()". Each of them has an
ISR that masks the RX interrupt before calling napi_schedule(), and a
poll that is the only place the RX interrupt is unmasked again. A frame
that arrives between the controller start and napi_enable() is handled
by the ISR, which masks RX and calls napi_schedule() on a NAPI instance
that is still disabled. napi_schedule_prep() sees NAPI_STATE_SCHED set
and drops the schedule. napi_enable() clears SCHED without scheduling.
Nothing ever polls, so RX stays masked and the interface receives
nothing until the next close. Transmit keeps working and no error
counter moves, so the failure is silent.

The affected drivers are ctucanfd, xilinx_can and ifi_canfd. The fix is
the same in each: call napi_enable() before the controller start, so
that an early RX interrupt can schedule the poll, and call
napi_disable() on the error path if the start can fail.

Patch 1 (ctucanfd) was tested on QEMU with the ctucan_pci device.
Patch 2 (xilinx_can) was tested on a Versal board. Patch 3 (ifi_canfd)
is from code inspection only. I do not have the hardware, so it is
compile tested only. The sequence is the same as in the other two
drivers, and I would appreciate a test from someone who has an IFI CAN
FD core.

The bug has been present since each driver was added, so each patch
has a Fixes: tag pointing at the commit that added the driver.
Patches 1 and 2 are marked for stable backport. Patch 3 is not,
because it is untested. The patches are independent and can be
applied separately.

I looked at the other NAPI users under drivers/net/can. at91_can had
the same persistent-mask pattern in 6.6 but no longer masks RX in its
ISR since the conversion to rx-offload, so I left it alone; ti_hecc and
virtio_can start the device before napi_enable() but do not leave RX
masked, so a dropped first schedule only delays the first frame. All
the others enable NAPI before the device can interrupt.

Patrick Zhang (3):
  can: ctucanfd: enable NAPI before starting the controller
  can: xilinx_can: enable NAPI before starting the controller
  can: ifi_canfd: enable NAPI before starting the controller

 drivers/net/can/ctucanfd/ctucanfd_base.c | 6 +++++-
 drivers/net/can/ifi_canfd/ifi_canfd.c    | 5 ++++-
 drivers/net/can/xilinx_can.c             | 6 +++++-
 3 files changed, 14 insertions(+), 3 deletions(-)


base-commit: a7bfaba4823e3c165bb2004c74eff7c096672bc7
-- 
2.50.1

[PATCH 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>
---
 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: a7bfaba4823e3c165bb2004c74eff7c096672bc7
-- 
2.50.1

[PATCH 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 nothing until the next close. Transmit
still works and no rx_errors subcounter moves, so the stall is silent.
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 ctucanfd, fixed by the previous patch.

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>
---
 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

[PATCH 3/3] can: ifi_canfd: enable NAPI before starting the controller

[Patrick Zhang]

ifi_canfd_open() calls ifi_canfd_start(), which unmasks the interrupts
in IFI_CANFD_IRQMASK and then enables the controller through
IFI_CANFD_STCMD, and only then calls napi_enable(). A frame that
arrives in that window raises RXFIFO_NEMPTY. ifi_canfd_isr() masks all
interrupts with ifi_canfd_irq_enable(ndev, 0) and calls
napi_schedule(). NAPI is still disabled, so napi_schedule_prep() finds
NAPI_STATE_SCHED set and the schedule is dropped. Only ifi_canfd_poll()
unmasks the interrupts again, and it never runs. From then on the
interface receives nothing. ifi_canfd_start_xmit() stops the queue
after every frame and relies on the TX FIFO interrupt to wake it, and
that interrupt is masked too, so transmit stops after the next frame.
The stall lasts until the next close. Every re-open runs the same race
again.

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.
ifi_canfd_start() cannot fail, so no error path changes.

This is the same race as in ctucanfd and xilinx_can, fixed by the
previous patches. I do not have IFI CAN FD hardware, so this change is
compile tested only and the failure sequence is taken from the code.

Fixes: 0c4d9c94a130 ("can: ifi: Add IFI CANFD IP support")
Assisted-by: LLM
Signed-off-by: Patrick Zhang <patrzhan@amazon.com>
---
 drivers/net/can/ifi_canfd/ifi_canfd.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c
index 0f83335e4..feb738eb0 100644
--- a/drivers/net/can/ifi_canfd/ifi_canfd.c
+++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
@@ -846,9 +846,12 @@ static int ifi_canfd_open(struct net_device *ndev)
 		goto err_irq;
 	}
 
+	/* Keep before ifi_canfd_start(): the ISR masks the interrupts and only
+	 * the poll unmasks them.
+	 */
+	napi_enable(&priv->napi);
 	ifi_canfd_start(ndev);
 
-	napi_enable(&priv->napi);
 	netif_start_queue(ndev);
 
 	return 0;
-- 
2.50.1

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

[Pavel Pisa]

Hello Patrick Zhang,

thanks for the problem finding and patch.

On Wednesday 21 of October 2026 22:59:22 Patrick Zhang wrote:
> 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>

I have added patch into our CAN latency testing system

https://canbus.pages.fel.cvut.cz/#can-bus-channels-mutual-latency-testing
https://canbus.pages.fel.cvut.cz/can-latester/

See in the log for both master 7.3.0-gcac8667a979c
and RT tested branch 7.1.1-rt2-g1661cb7f80a0

-> Running pre-build script /var/lib/latester/patches-dut/apply-script.sh
Applying: can: ctucanfd: enable NAPI before starting the controller
-> Setting up build dir

I have spotted problem appearing in out-of-the-tree device tree overlay
driver build which is consequence of CAN unrelated this night change
in mainline which required next fix

--- a/dtbocfg.c
+++ b/dtbocfg.c
@@ -263,7 +263,11 @@ static struct configfs_attribute *dtbocfg_overlay_attrs[] = {
     NULL,
 };

-static struct configfs_bin_attribute *dtbocfg_overlay_bin_attrs[] = {
+static
+#if (LINUX_VERSION_CODE >= KERNEL_VERSION(7, 3, 0))
+const
+#endif
+struct configfs_bin_attribute *dtbocfg_overlay_bin_attrs[] = {
     &dtbocfg_overlay_item_attr_dtbo,
     NULL,
 };

As for fully-preemptive kernel testing, we have been on the git

https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git/refs/heads

at the branch for-kbuild-bot/current-stable but that seems to be no longer
active to track latest RT development above stable branch so it is
time to switch to some else branch or master. Ideal would be something
above next with lattest RT fixes applied before they reach mainline.
But we probably switch to mainline with RT config if nothing better
to catch possible RT deviations (changes in latency or some bugs)
is suggested.

For the curiosity, we have run CTU CAN FD IP core on 64-core
RISC-V with 7.3 kernel and drivers to control motor servosystem
based on ESP32C5 over CAN FD on the SUSE booth at Open Source
Summit for most of the days and it performed well

  https://social.kernel.org/notice/BBBNKOIUZ3NnMKcsXQ

Best wishes,

                Pavel

                Pavel Pisa
    phone:      +420 603531357
    e-mail:     pisa@cmp.felk.cvut.cz
    Department of Control Engineering FEE CVUT
    Karlovo namesti 13, 121 35, Prague 2
    university: http://control.fel.cvut.cz/
    personal:   http://cmp.felk.cvut.cz/~pisa
    social:     https://social.kernel.org/ppisa
    projects:   https://www.openhub.net/accounts/ppisa
    CAN related:http://canbus.pages.fel.cvut.cz/
    RISC-V education: https://comparch.edu.cvut.cz/
    Open Technologies Research Education and Exchange Services
    https://gitlab.fel.cvut.cz/otrees/org/-/wikis/home

Re: [PATCH 3/3] can: ifi_canfd: enable NAPI before starting the controller

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Because napi_enable() now runs before ifi_canfd_start(),
  ifi_canfd_poll() can run against an IFI CAN FD IP that has not been…

--

can: ifi_canfd: enable NAPI before starting the controller

This patch moves napi_enable() ahead of ifi_canfd_start() in
ifi_canfd_open(). The goal is that an RX interrupt raised right after the
controller is enabled can schedule the poll, instead of being dropped and
leaving the interrupts masked.

> diff --git a/drivers/net/can/ifi_canfd/ifi_canfd.c b/drivers/net/can/ifi_canfd/ifi_canfd.c
> index 0f83335e4d07..feb738eb0c9a 100644
> --- a/drivers/net/can/ifi_canfd/ifi_canfd.c
> +++ b/drivers/net/can/ifi_canfd/ifi_canfd.c
> @@ -846,9 +846,12 @@ static int ifi_canfd_open(struct net_device *ndev)
>  		goto err_irq;
>  	}
>  
> +	/* Keep before ifi_canfd_start(): the ISR masks the interrupts and only
> +	 * the poll unmasks them.
> +	 */
> +	napi_enable(&priv->napi);
>  	ifi_canfd_start(ndev);

[Severity: Medium]
With NAPI enabled before ifi_canfd_start(), can ifi_canfd_poll() now run
against an IP that has not been reset yet? Can it also run alongside the
reset and programming sequence in ifi_canfd_start()?

The handler is installed with IRQF_SHARED just above. So ifi_canfd_isr()
can run because of another device on the line, or because of a line that
is still asserted. The ISR acts on the raw status and does not AND it with
IRQMASK:

ifi_canfd_isr() {
    ...
	isr = readl(priv->base + IFI_CANFD_INTERRUPT);
    ...
	if (isr & rx_irq_mask) {
		ifi_canfd_irq_enable(ndev, 0);
		napi_schedule(&priv->napi);
	}
    ...
}

Before this patch, NAPI was still disabled, so this schedule was dropped.
The ifi_canfd_irq_enable(ndev, 1) inside ifi_canfd_start() then recovered.
Now the schedule succeeds.

ifi_canfd_plat_probe() only reads IP_ID, VER and CANCLOCK, and the driver
has no .shutdown hook. After a kexec or warm boot, could the IP still hold
RX data or error status when the device is first opened?

If so, the poll can run on another CPU, or at irq_exit in the middle of
ifi_canfd_start(). It would then do the following with no lock:

  - read STCMD and RXSTCMD
  - drain the RX FIFO through ifi_canfd_read_fifo()
  - possibly write ERROR_CTR from ifi_canfd_handle_lec_err()
  - call ifi_canfd_irq_enable(ndev, 1)

Each of these could interleave with the HARDRESET, the RX/TX FIFO resets
and the ERROR_CTR unlock/reset/enable writes in ifi_canfd_start().

Possible results:

  - a stale or half-reset FIFO entry delivered as a received frame
  - a spurious ERROR_ACTIVE state-change frame with error_warning++,
    because priv->can.state is still CAN_STATE_STOPPED
  - a stale BUSOFF bit in STCMD, which makes
    ifi_canfd_handle_state_change() do this:

	priv->can.state = CAN_STATE_BUS_OFF;
	ifi_canfd_irq_enable(ndev, 0);
	priv->can.can_stats.bus_off++;
	can_bus_off(ndev);

In the bus-off case, ifi_canfd_start() then overwrites the state:

	priv->can.state = CAN_STATE_ERROR_ACTIVE;

With restart-ms set to 0, can_restart_now() would return -EBUSY, because
the state is no longer CAN_STATE_BUS_OFF. Would the interface then stay
carrier-off until the next down/up?

The commit message says:

  "NAPI can be enabled while the controller is in reset, because nothing
  schedules it until an interrupt arrives."

That seems to hold only after a previous close, where ifi_canfd_stop()
writes HARDRESET, masks IRQMASK and clears INTERRUPT. It does not seem to
hold for the first open after boot or kexec, or for ISR calls coming from
the shared line.

Would it be safer to quiesce the IP first? That would mean HARDRESET,
masking IRQMASK and clearing INTERRUPT, either in probe or before
napi_enable(). Another option is to move napi_enable() into
ifi_canfd_start(), after the reset and interrupt clear and before
ifi_canfd_irq_enable(ndev, 1).

The bus-off restart path, ifi_canfd_set_mode() -> ifi_canfd_start(),
already runs with NAPI enabled, so a similar overlap exists there. This
change extends that existing weakness to the open path.

>  
> -	napi_enable(&priv->napi);
>  	netif_start_queue(ndev);
>  
>  	return 0;

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261021205924.2538511-1-patrzhan%40amazon.com