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) { > > > >