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 <bou...@antioche.eu.org>
NetBSD: 26 ans d'experience feront toujours la difference
--
