Re: kern/41114: soft interrupt queue patch for BRIDGE_IPF to prevent lock issues

[Manuel Bouyer <bouyer%antioche.eu.org@localhost>]

The following reply was made to PR kern/41114; it has been noted by GNATS.

From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
To: Peter <necedemalis%gmail.com@localhost>
Cc: gnats-bugs%netbsd.org@localhost, kern-bug-people%netbsd.org@localhost, 
gnats-admin%netbsd.org@localhost,
        netbsd-bugs%netbsd.org@localhost
Subject: Re: kern/41114: soft interrupt queue patch for BRIDGE_IPF to prevent 
lock issues
Date: Thu, 2 Apr 2009 22:19:53 +0200

 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
 > unfiltered?
 
 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
 > filtered.
 
 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
 --