On Fri, Jan 02, 2026 at 06:32:31AM +0000, Vitaliy Makkoveev wrote:
> On Mon, Dec 29, 2025 at 01:20:05PM +0100, Alexander Bluhm wrote:
> > On Tue, Dec 02, 2025 at 11:01:36PM +0100, Alexander Bluhm wrote:
> > > Hi,
> > > 
> > > After moving around code for a while, I think a rwlock to protect
> > > if_maddrlist is the next step.  The malloc(M_WAITOK) in rti_fill()
> > > prevents us from using a mutex here.
> > > 
> > > So protect the TAILQ if_maddrlist with rwlock if_maddrlock.  Also
> > > struct in_multi and in6_multi use this lock for their state and
> > > timer.  Sleeps in malloc and IP output are possible.
> > > 
> > > This allows to run IGMP and MLD6 fast timeout with shared net lock.
> > 
> > I would like to proceed with MP multicast.
> > 
> > OK? objections?
> >
> 
> Just for curiosity, why have you replaced mutex with rwlock?

There is this call path, I found the sleeping point during testing.

in_addmulti()
rw_enter_write(&ifp->if_maddrlock)
igmp_joingroup()
rti_fill()
malloc(M_WAITOK)

Maybe I can pull the M_WAITOK out of the lock later.  Sleeping for
memory with a rwlock held is not optimal.  But moving the malloc
and replacing the lock with a mutex requires more refacotring.  Not
sure if it is worth it.

IPv6 is different, it uses M_NOWAIT.  But that means a system call
can fail due to low memory situations.  Also not good, sleeping
would be better.

> in{,6}_var.h have missing locks description. Can you add something
> like of 'if_maddrlock rwlock of parent interface' for [m]? And
> description for [I] too.

done

> The rest looks good to me.

thanks