Re: CVS commit: src/sys/net

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

On Sun, Apr 05, 2009 at 12:06:12AM +0900, Masao Uebayashi wrote:
> I was reviewing this change. :)
> 
> On Sat, Apr 04, 2009 at 10:00:23AM +0000, Manuel Bouyer wrote:
> > Module Name:        src
> > Committed By:       bouyer
> > Date:               Sat Apr  4 10:00:23 UTC 2009
> > 
> > Modified Files:
> >     src/sys/net: if_bridge.c if_bridgevar.h
> > 
> > Log Message:
> > Fix for if_start() and pfil_hook() being called from hardware interrupt
> > context (reported on various mailing-lists, and part of PR kern/41114,
> > causing panic in pf(4) and possibly ipf(4) when BRIDGE_IPF is used).
> > Defer bridge_forward() to a software interrupt; bridge_input() enqueues
> > mbufs to ifp->if_snd which is handled in bridge_forward().
> > 
> > 
> > To generate a diff of this commit:
> > cvs rdiff -u -r1.64 -r1.65 src/sys/net/if_bridge.c
> > cvs rdiff -u -r1.13 -r1.14 src/sys/net/if_bridgevar.h
> > 
> > Please note that diffs are not public domain; they are subject to the
> > copyright notices on the relevant files.
> > 
> 
> One more thing:
> 
> > @@ -1305,124 +1318,139 @@
> >   * The forwarding function of the bridge.
> >   */
> >  static void
> > -bridge_forward(struct bridge_softc *sc, struct mbuf *m)
> > +bridge_forward(void *v)
> >  {
> > +   struct bridge_softc *sc = v;
> > +   struct mbuf *m;
> >     struct bridge_iflist *bif;
> >     struct ifnet *src_if, *dst_if;
> >     struct ether_header *eh;
> > +   int s;
> (snip)
> > -   bridge_enqueue(sc, dst_if, m, 1);
> > +           bridge_enqueue(sc, dst_if, m, 1);
> > +   }
> > +   splx(s);
> >  }
> >  
> >  /*
> 
> You can put a wrapping function (for example bridge_forward_intr()) and call
> the original bridge_forward() from within it.  And the diff will be much
> smaller & easier to review.

I don't like such useless intermediate functions. It doesn't make code
easier to read. Also this makes sure that bridge_forward isn't called
directly any more since the prototype changed (yes, I could also have
changed its name)

-- 
Manuel Bouyer <bouyer%antioche.eu.org@localhost>
     NetBSD: 26 ans d'experience feront toujours la difference
--