</large snip>

> > 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.
> 
> As long as we are using (soft-)interrupts to process incoming packets
> they should.
> 
> If you don't raise the SPL level to softnet while CPU0 is executing some
> code in ioctl() path and it takes an interrupt at this point it might end
> up executing ip_input() *before* returning to the ioctl() (process)
> context.  This scenario might corrupt the states you're trying to protect.
> 

just to verify I got everything right here. I assume I've got MULTIPROCESSOR
kernel with my broken patch, which basically trades or splfotnet() locks for
KERNEL_LOCKS()...

We have ioctl() operation, which attempts to insert a rule into a global
ruleset, ioctl() grabs KERNEL_LOCK() only and gets to work. (yes my broken
patch does not grab splsoftnet()).

while ioctl() is working there is interrupt, which delivers packet from NIC.
since no one holds splsoftnet() on CPU packet is free to go and enters
firewall (pf_test()).  And that's very bad, interrupt tries to grab
KERNEL_LOCK(), which is still held by ioctl() packet has just interrupted. One
of the two bad things will happen:

        a) deadlock, we will be waiting for KERNEL_LOCK(), which is still held
           by interrupted ioctl() operation.

        b) since KERNEL_LOCK() is recursive, the pf_test() will be free to
           go possibly finding a list of rules in inconsistent state. In this
           case no deadlock happens, but PF plays game with pointers...

as Kettenis pointed out the KERNEL_LOCK() is recursive, I have an itchy feeling
we are taking b) option. Is that right?

> > Does that idea sound right? If not what piece I'm still missing in puzzle?
> 
> I think you're assuming that the "softnet" code path is running in a
> thread which is not (yet) the case.

yes, my context is not fully switched from Solaris to OpenBSD yet...

thanks and
regards
sasha

Reply via email to