Re: [PATCH net] af_packet: move notifier's packet_dev_mc out of rcu critical section

[Jakub Kicinski <kuba@kernel.org>]

On Tue, 20 May 2025 22:41:30 -0400 Willem de Bruijn wrote:
> > @@ -4277,6 +4280,13 @@ static int packet_notifier(struct notifier_block *this,
> >  		}
> >  	}
> >  	rcu_read_unlock();
> > +
> > +	/* packet_dev_mc might grab instance locks so can't run under rcu */
> > +	list_for_each_entry_safe(ml, tmp, &mclist, remove_list) {
> > +		packet_dev_mc(dev, ml, -1);
> > +		kfree(ml);
> > +	}
> > +  
> 
> Just verifying my understanding of the not entirely obvious locking:
> 
> po->mclist modifications (add, del, flush, unregister) are all
> protected by the RTNL, not the RCU. The RCU only protects the sklist
> and by extension the sks on it. So moving the mclist operations out of
> the RCU is fine.
> 
> The delayed operation on the mclist entry is still within the RTNL
> from unregister_netdevice_notifier. Which matter as it protects not
> only the list, but also the actual operations in packet_dev_mc, such
> as inc/dec on dev->promiscuity and associated dev_change_rx_flags.
> And new packet_mclist.remove_list too.

Matches my understanding FWIW, but this will be a great addition 
to the commit message. Let's add it in v2..

> >  	return NOTIFY_DONE;
> >  }
> >  
> > diff --git a/net/packet/internal.h b/net/packet/internal.h
> > index d5d70712007a..1e743d0316fd 100644
> > --- a/net/packet/internal.h
> > +++ b/net/packet/internal.h
> > @@ -11,6 +11,7 @@ struct packet_mclist {
> >  	unsigned short		type;
> >  	unsigned short		alen;
> >  	unsigned char		addr[MAX_ADDR_LEN];
> > +	struct list_head	remove_list;  
> 
> INIT_LIST_HEAD on alloc in packet_mc_add?

Just to be clear this is an "entry node" not a "head node",
is it common to init "entry nodes"? 
-- 
for the commit msg:
pw-bot: cr