Hi Marc, My understanding is that can_restart_now() may be called in interrupt context by some CAN driver, so netif_rx_any_context() would be safer, but I could be wrong and maybe can_restart_now() is really thought to be called in process context always (so better to use netif_rx_ni()). What do you think? should I update the patch? Thanks in advance, Alejandro On 6/11/20 18:33, Alejandro wrote: > On 6/11/20 11:25, Marc Kleine-Budde wrote: > >> On 11/5/20 10:51 PM, Alejandro wrote: >>> From: Alejandro Concepcion Rodriguez<alejandro@xxxxxxxx> >>> >>> netif_rx() is meant to be called from interrupt contexts. can_restart() >>> may be called by can_restart_work(), which is called from a worqueue, so >>> it may run in process context. Use netif_rx_any_context() which invokes >>> the correct code path depending on context. >>> >>> Co-developed-by: Loris Fauster<loris.fauster@xxxxxxxxxxxxx> >>> Signed-off-by: Loris Fauster<loris.fauster@xxxxxxxxxxxxx> >>> Signed-off-by: Alejandro Concepcion Rodriguez<alejandro@xxxxxxxx> >> I think we either call can_restart() from a netlink callback via >> can_restart_now() or via the can_restart_work(). So we should always use >> netif_rx_ni(skb), right? > Right, I think that currently it is as you say. However, it seems that > can_restart_now() has public visibility (/linux/can/dev.h), and even though > it doesn't seem to be used by other CAN drivers for now, I guess it could > potentially be used in future. netif_rx_any_context() should avoid issues > if can_restart_now() is called from an ISR later. > >>> --- >>> drivers/net/can/dev.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c >>> index b70ded3760f2..83114f8e8c24 100644 >>> --- a/drivers/net/can/dev.c >>> +++ b/drivers/net/can/dev.c >>> @@ -584,7 +584,7 @@ static void can_restart(struct net_device *dev) >>> >>> cf->can_id |= CAN_ERR_RESTARTED; >>> >>> - netif_rx(skb); >>> + netif_rx_any_context(skb); >>> >>> stats->rx_packets++; >>> stats->rx_bytes += cf->can_dlc; >>> >> Marc >> > BR, > Alejandro
