On Wed, Aug 26, 2015 at 06:12:10PM +0200, Mark Kettenis wrote:
> > Date: Wed, 26 Aug 2015 17:30:14 +0200
> > From: Alexandr Nedvedicky <alexandr.nedvedi...@oracle.com>
> > 
> > Hello,
> > 
> > I'm not sure I got everything right in Calgary. So this patch should
> > roughly illustrates how I think we should start moving forward to
> > make PF MULTIPROCESSOR friendly. It's quite likely my proposal/way
> > is completely off, I'll be happy if you put me back to ground.
> > 
> > The brief summary of what patch is trying to achieve is as follows:
> > 
> >     patch trades all splsoftnet() for KERNEL_LOCK() when it gets compiled
> >     with MULTIPROCESSOR option on.
> > 
> >     if MULTIPROCESSOR option is off, the compiler produces PF, which uses
> >     splsoftnet.
> > 
> >     To achieve this the patch introduces macros PF_LOCK()/PF_UNLOCK(),
> >     which expand to KERNEL_LOCK()/KERNEL_UNLOCK(), when MULTIPROCESSOR is 
> > on.
> >     On the other hand if MULTIPROCESSOR is off the PF_*LOCK() macros become
> >     splsoftnet()/splx()
> 
> I don't think this will work.  Even on MULTIPROCESSOR kernels you'll
> need to raise the spl to prevent soft interrupts from running on the
> same CPU.  KERNEL_LOCK() will not prevent this from happening as it is
> a recursive lock.  This is why OpenBSD's mutexes (spinning locks)
> raise the spl.
> 
> So I think you'll have to define PF_LOCK()/PF_UNLOCK() to do the spl
> stuff even for MULTIPROCESSOR kernels.

Thanks for putting my feet to ground. I'm trying to make some sense from that.
So if I understand splsotfnet() right in MULTIPROCESSOR kernel, it prevents
CPU from handling another network interrupt until the first one is
handled. So that splsoftnet should be raised by network driver, before
packet reaches PF (pf_test() function).

the pf_test() must protect consistency of firewall data, which are shared
between CPUs (?can we say kernel threads?). So for the first phase, we opt for
KERNEL_LOCK() being grabbed by pf_test() as soon as it gets called. It makes PF
to still process single packet at time. All other CPUs will have to wait on
KERNEL_LOCK(), holding their splsoftnets(). Do I still follow the concept, or
am I wrong again?

Assuming the locking in MULTIPROCESSOR goes like:
         interrupt grabs splsoftnet -> ip_input -> PF grabs KERNEL_LOCK()
We need to take care of ioctl() call path and purge thread. Those need to
get synchronize with packets using KERNEL_LOCK(). They should not to mess with
splsoftnet() any more in MULTIPROCESSOR version.

Does that idea sound right? If not what piece I'm still missing in puzzle?

thanks and
regards
sasha

Reply via email to