Re: [PATCH] can: ctucanfd: ctucan_err_interrupt(): do not emit info messages on Arbitration Lost or CAN Bus Error IRQs

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

Pre-existing issues:
- [High] Unconditional evaluation of the global `net_ratelimit()` in a high-frequency (10kHz) interrupt handler exhausts the global network ratelimit token bucket and causes lock contention, effectively silencing rate-limited messages across the entire networking subsystem. Furthermore, the generic `netdev_info` print at the top of the function still executes for these noisy interrupts, meaning the patch fails to completely fix the log flooding it aimed to resolve.
--

--- Patch [1]: [PATCH] can: ctucanfd: ctucan_err_interrupt(): do not emit info messages on Arbitration Lost or CAN Bus Error IRQs ---
commit 5e775ed40c20c9c562c6441b9b627d0ada3f07fd
Author: Marc Kleine-Budde <[email protected]>

can: ctucanfd: ctucan_err_interrupt(): do not emit info messages on Arbitration Lost or CAN Bus Error IRQs

With CAN Bus Error reporting enabled, Arbitration Lost and Bus Error
interrupts can occur at a very high rate (~10kHz). To avoid flooding the
system, this commit stops emitting specific netdev_info messages for these
interrupts.

> diff --git a/drivers/net/can/ctucanfd/ctucanfd_base.c b/drivers/net/can/ctucanfd/ctucanfd_base.c
> index 07d4aa43c700a..9aec18681fc54 100644
> --- a/drivers/net/can/ctucanfd/ctucanfd_base.c
> +++ b/drivers/net/can/ctucanfd/ctucanfd_base.c
> @@ -884,8 +884,6 @@ static void ctucan_err_interrupt(struct net_device *ndev, u32 isr)

[Severity: High]
This isn't a bug introduced by this patch, but does calling net_ratelimit()
unconditionally at the top of this function exhaust the global rate limit
token bucket?

Looking at ctucan_err_interrupt() earlier in this file:

	int dologerr = net_ratelimit();

	ctucan_get_rec_tec(priv, &bec);
	state = ctucan_read_fault_state(priv);
	err_capt_alc = ctucan_read32(priv, CTUCANFD_ERR_CAPT);

	if (dologerr)
		netdev_info(ndev, "%s: ISR = 0x%08x...

Since ALI and BEI interrupts can still occur at ~10kHz and trigger this
handler, won't this still drain the shared ratelimit budget and cause lock
contention on net_ratelimit_state.lock for the rest of the networking
subsystem?

Also, since the generic netdev_info log statement above is still executed
for these noisy interrupts, does this patch completely fix the log flooding
it aims to resolve?

>  
>  	/* Check for Arbitration Lost interrupt */
>  	if (FIELD_GET(REG_INT_STAT_ALI, isr)) {
> -		if (dologerr)
> -			netdev_info(ndev, "arbitration lost\n");
>  		priv->can.can_stats.arbitration_lost++;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-ctucanfd-silence-can-bus-error-messages-v1-1-70eff67debfd@pengutronix.de?part=1