Re: CVS commit: src/sys/net

[Masao Uebayashi <uebayasi%gmail.com@localhost>]

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.

        tatic void
        bridge_forward_intr(void *v)
        {
               struct bridge_softc *sc = v;
               struct mbuf *m;
               int s;
        
               if ((sc->sc_if.if_flags & IFF_RUNNING) == 0)
                       return;
        
               s = splnet();
               while (1) {
                       IFQ_POLL(&sc->sc_if.if_snd, m);
                       if (m == NULL)
                               break;
                       IFQ_DEQUEUE(&sc->sc_if.if_snd, m);
                       bridge_forward(sc, m);
               }
               splx(s);
        }

        static void
        bridge_forward(struct bridge_softc *sc, struct mbuf *m)
        {
                :

Masao