On Wed, Feb 05, 2025 at 11:07:45AM -0800, Breno Leitao wrote:
> > + else if (is_valid_ether_addr(np->dev_mac))
> > + ndev = dev_getbyhwaddr_rcu(net, ARPHRD_ETHER, np->dev_mac);
>
> You do not have the RCU read lock here. You have the rtnl(), which is
> sufficient, but, CONFIG_PROVE_RCU_LIST will show something as:
>
> WARNING: suspicious RCU usage
> 6.13.0-09701-g6610c7be45bb-dirty #18 Not tainted
> -----------------------------
> net/core/dev.c:1143 RCU-list traversed in non-reader section!!
> other info that might help us debug this:
> rcu_scheduler_active = 2, debug_locks = 1
> 1 lock held by swapper/0/1:
> #0: ffffffff832795b8 (rtnl_mutex){+.+.}-{4:4}, at: netpoll_setup+0x48/0x540
> stack backtrace:
> CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.13.0-virtme-09701-g6610c7be45bb-dirty #18
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> Call Trace:
> <TASK>
> dump_stack_lvl+0x9f/0xf0
> lockdep_rcu_suspicious+0x11a/0x150
> dev_getbyhwaddr_rcu+0xb6/0xc0
> netpoll_setup+0x8a/0x540
> ? netpoll_parse_options+0x2bd/0x310
>
> This is not a problem per-se, since you have RTNL. We probably need to
> tell for_each_netdev_rcu() to not comply about "RCU-list traversed in
> non-reader section" if RTNL is held. Not sure why we didn't hit in the
> test infrastructure, tho:
>
> https://patchwork.kernel.org/project/netdevbpf/patch/20250204-netconsole-v2-2-5ef5eb5f6056@xxxxxxxxxxxxxxx/
I don't think there is an automated test that will hit this path yet. I
guess you got this trace from your manual testing?
>
> Anyway, no action item for you here. I am talking to Jakub on a way to
> solve it, and I should send a fix soon.
/**
* list_for_each_entry_rcu - iterate over rcu list of given type
* @pos: the type * to use as a loop cursor.
* @head: the head for your list.
* @member: the name of the list_head within the struct.
* @cond: optional lockdep expression if called from non-RCU protection.
*
* This list-traversal primitive may safely run concurrently with
* the _rcu list-mutation primitives such as list_add_rcu()
* as long as the traversal is guarded by rcu_read_lock().
*/
#define list_for_each_entry_rcu(pos, head, member, cond...) \
for (__list_check_rcu(dummy, ## cond, 0), \
pos = list_entry_rcu((head)->next, typeof(*pos), member); \
&pos->member != (head); \
pos = list_entry_rcu(pos->member.next, typeof(*pos), member))
If we do something like
list_for_each_entry_rcu(..., lockdep_rtnl_is_held())
...
I think that code will be okay with being called with either rcu or rtnl
held. Of course, we need to plumb it through the net-specific helpers.
