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

[Peter <necedemalis%gmail.com@localhost>]

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

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

 On Thu, Apr 2, 2009 at 6:10 AM, Manuel Bouyer 
<bouyer%antioche.eu.org@localhost> wrot=
 e:
 > 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-interf=
 ace
 >  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 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 funct=
 ion
 >  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 inter=
 faces that have been added to the bridge.  Previously the bridge code was c=
 alled even if the bridge was down and then the bridge would either return o=
 r 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 effe=
 ctively
 >  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 previously passed runfilt=3D0 to bridge_enqueue meani=
 ng 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 loc=
 al 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.
 
 >  --
 >  Manuel Bouyer <bouyer%antioche.eu.org@localhost>
 >      NetBSD: 26 ans d'experience feront toujours la difference
 >  --
 >
 >
 
 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?
 
    Peter