[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: kern/41114: soft interrupt queue patch for BRIDGE_IPF to prevent lock issues
On Thu, Apr 02, 2009 at 04:08:05PM -0400, Peter wrote:
> On Thu, Apr 2, 2009 at 6:10 AM, Manuel Bouyer
> <bouyer%antioche.eu.org@localhost> wrote:
> > The following reply was made to PR kern/41114; it has been noted by GNATS.
> > I don't think it's necessery to have a soft interrupt queue for
> > bridge_enqueue(). It should already be called only from soft interrupt
> > of thread context.
> > bridge_input()/bridge_forward() can use the ifp->if_snd queue, there's
> > no need to store more bridge information here (it's already a per-interface
> > queue, which is currently unused for bridge).
> > See the patch I sent to tech-net which does this.
> I should have checked where enqueue was called from, that makes sense.
> But isn't it a problem to put packets on the if_snd queue that are
if_snd is used as input queue here, not output :)
> > > If bridge filtering is off, the soft interrupts are not used so there's
> > no performance penalty.
> > That may not be a good idea. The problem is not limited to BRIDGE_IPF;
> > as pointed out by tls@ calling the underlying interface's if_start function
> > from hard interrupt context may also be troublesome.
> I didn't know this. I just assumed that if the bridge worked without
> BRIDGE_IPF, then it would work if it was fixed and it would be better
> not to introduce slow downs to anything else.
> > >
> > > stopping and starting the bridge clears if_bridge pointers in
> > interfaces that have been added to the bridge. Previously the bridge code
> > was called even if the bridge was down and then the bridge would either
> > return or route it. This seemed strange. If the bridge is down, the
> > packet should be handled as normal by that interface.
> > This is a change in behavior that may have unwanted side effects.
> > This should be discussed in a separate PR I think.
> > Note that the current bridge code already checks IFF_RUNNING, so it
> > effectively
> > behaves as a non-bridged interface when the bridge is down, even if the
> > bridge code is called.
> As I said, it seemed strange. bridge_input checks IFF_RUNNING,
> bridge_output handles the packet. Why should the bridge code be
> called if the bridge is down?
bridge_output() also checks IFF_RUNNING
> > >
> > > bridge_output previously passed runfilt=0 to bridge_enqueue meaning
> > that outgoing local packets were not filtered. This seemed strange so I
> > put it to 1. If bridge filtering is on, then you would want it on for
> > local packets as well I would assume.
> > ether_output() has already been called in ether_output() for this packet.
> > I'm not sure this change is a good idea because the same packet would
> > then be seen 2 times on 2 different interfaces. Again this is a
> > behavior change that should be discussed separately.
> I don't understand, "ether_output() has already been called in
> either_output()"? Well, locally originated packets should be
I meant "pfil_hook() has already been called in ether_output()"
> There might however be a major issue with my patch in that "dup-to" in
> ipf seems broken for me in different ways. I am curious as to what
> might have caused that. Do you have any guesses?
Maybe you created a recursion by calling pfil_hook() from bridge_output ?
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
NetBSD: 26 ans d'experience feront toujours la difference
Main Index |
Thread Index |