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

[Patrick Zhang <patrzhan@amazon.com>]

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 no
data frames until the next close. In ctucanfd and xilinx_can, transmit
keeps working. ctucanfd counts no errors, so the failure is silent
there. xilinx_can counts RX FIFO overflows once the FIFO fills. In
ifi_canfd the TX wake-up interrupt is masked too, so transmit also
stops.

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. ifi_canfd has
no reset in open() outside its start function, so that patch also
splits the reset out of ifi_canfd_start() and enables NAPI between the
reset and the start.

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.

Changes in v2:
- Patch 1: add Acked-by and Tested-by from Pavel
- Patch 2: correct the commit message, which said no error counter
  moves during the stall; the RX overflow interrupt stays enabled and
  counts rx_over_errors
- Patch 3: split the reset out of ifi_canfd_start() and enable NAPI
  after it, with synchronize_irq() in between, so the poll cannot race
  with the reset (reported by the netdev AI review). This replaces the
  ifi_canfd_stop() call proposed in my reply to that review; see the
  patch notes
- Patches 2 and 3: name the related fixes by subject
- Rebased on current linux-can main
- v1: https://lore.kernel.org/r/20261021205924.2538511-1-patrzhan@amazon.com

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    | 17 ++++++++++++++---
 drivers/net/can/xilinx_can.c             |  6 +++++-
 3 files changed, 24 insertions(+), 5 deletions(-)


base-commit: abeacca61eea4f6803df4a595b656ea45c2d3249
-- 
2.50.1