[PATCH v3 0/2] can: enable NAPI before starting the controller in two drivers

[Patrick Zhang]

Hi,

ctucanfd and xilinx_can 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. Transmit keeps working. ctucanfd counts no errors, so the
failure is silent there. xilinx_can counts RX FIFO overflows once the
FIFO fills.

The fix is the same in both: 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 fails.

Patch 1 (ctucanfd) was tested on QEMU with the ctucan_pci device.
Patch 2 (xilinx_can) was tested on a Versal board.

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, and both
are marked for stable backport. 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.
ifi_canfd has the same race; see the changes in v3. All the others
enable NAPI before the device can interrupt.

Changes in v3:
- Drop the ifi_canfd patch. The netdev AI review of v2 showed that a
  safe fix has to reorder the reset, error counter and interrupt mask
  writes in ifi_canfd_start(), on the open and the bus-off restart
  paths. That change is no longer obviously correct by inspection, and
  I cannot test it without IFI CAN FD hardware. The race in
  ifi_canfd_open() remains.
- Patch 1: no changes
- Patch 2: add Reviewed-by from Harini
- v2: https://lore.kernel.org/r/20261030233330.3483988-1-patrzhan@amazon.com

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 (2):
  can: ctucanfd: enable NAPI before starting the controller
  can: xilinx_can: enable NAPI before starting the controller

 drivers/net/can/ctucanfd/ctucanfd_base.c | 6 +++++-
 drivers/net/can/xilinx_can.c             | 6 +++++-
 2 files changed, 10 insertions(+), 2 deletions(-)


base-commit: abeacca61eea4f6803df4a595b656ea45c2d3249
-- 
2.50.1