On Fri, May 26, 2023 at 05:08:06PM +0200, Alexander Bluhm wrote:
> On Fri, May 26, 2023 at 05:29:58PM +0300, Vitaliy Makkoveev wrote:
> > On Wed, May 17, 2023 at 01:02:58PM +0300, Vitaliy Makkoveev wrote:
> > > mrt_rtwalk_mfcsysctl() performs read-only access to protected data, so
> > > rtable_walk() could be called with shared netlock.
> > > 
> > 
> > Regardless on sysctl(2) unlocking backout, the netlock around
> > mrt_sysctl_mfc() could be relaxed to shared netlock.
> 
> IP multicast is far from MP ready.  As a usual workaround I use
> exclusive netlock or shared netlock plus kernel lock.
> 
> Whenever I have to call something with multicast from a section
> that has only shared netlock, I grab the kenrel lock.
> 
> So using only NET_LOCK_SHARED() for reading multicast data does not
> seem enough.
> 

mrt_rtwalk_mfcsysctl() does read-only access. Since the sysctl(2)
unlocking was reverted, this will be "shared netlock plus kernel lock"
case.

> Look at the way ip_input_if() calls ip_mforward().  Maybe we should
> start making ip_mroute.c MP safe.  Unfortunately I have no test
> environment for that.
> 

mpi@ said the kernel lock removal from uvm_swap_free() will be easy. So
I want to try to remove it and push sysctl(2) unlocking back.

> bluhm
> 
> > > Index: sys/netinet/ip_input.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/netinet/ip_input.c,v
> > > retrieving revision 1.384
> > > diff -u -p -r1.384 ip_input.c
> > > --- sys/netinet/ip_input.c        16 May 2023 19:36:00 -0000      1.384
> > > +++ sys/netinet/ip_input.c        17 May 2023 09:59:16 -0000
> > > @@ -1712,10 +1712,7 @@ ip_sysctl(int *name, u_int namelen, void
> > >   case IPCTL_MRTMFC:
> > >           if (newp)
> > >                   return (EPERM);
> > > -         NET_LOCK();
> > > -         error = mrt_sysctl_mfc(oldp, oldlenp);
> > > -         NET_UNLOCK();
> > > -         return (error);
> > > +         return (mrt_sysctl_mfc(oldp, oldlenp));
> > >   case IPCTL_MRTVIF:
> > >           if (newp)
> > >                   return (EPERM);
> > > Index: sys/netinet/ip_mroute.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/netinet/ip_mroute.c,v
> > > retrieving revision 1.138
> > > diff -u -p -r1.138 ip_mroute.c
> > > --- sys/netinet/ip_mroute.c       19 Apr 2023 20:03:51 -0000      1.138
> > > +++ sys/netinet/ip_mroute.c       17 May 2023 09:59:16 -0000
> > > @@ -479,10 +479,12 @@ mrt_sysctl_mfc(void *oldp, size_t *oldle
> > >   msa.msa_len = *oldlenp;
> > >   msa.msa_needed = 0;
> > >  
> > > + NET_LOCK_SHARED();
> > >   for (rtableid = 0; rtableid <= RT_TABLEID_MAX; rtableid++) {
> > >           rtable_walk(rtableid, AF_INET, NULL, mrt_rtwalk_mfcsysctl,
> > >               &msa);
> > >   }
> > > + NET_UNLOCK_SHARED();
> > >  
> > >   if (msa.msa_minfos != NULL && msa.msa_needed > 0 &&
> > >       (error = copyout(msa.msa_minfos, oldp, msa.msa_needed)) != 0) {
> > > 
> 

Reply via email to