FYI I commited this patch to HEAD and will request a pullup to netbsd-5.
Post by Manuel BouyerHi,
following a discussion on current-users about bridge(4) forwarding packets
in (possibly) hardware interrupt context, I've come up with the attached
patch. What it does is just defer bridge_forward() processing to a
software interrupt, with bridge_input() queuing mbufs for processing
to the ifp->if_snd queue.
AFAIK bridge_input() is the only entry point in bridge(4) which can be
called from a hardware interrupt, all others being called from
software interrupt or thread context. I've added a KASSERT() in
bridge_ipf() to check for this.
I've lightly tested this in a Xen dom0 context, with a wm(4) and xvif
part of the bridge; stp enabled on wm(4) (and also enabled on the cisco
switch at the other end), BRIDGE_IPF and ipf enabled on the bridge with
a few simple rules (including one 'block return-rst').
I'd like to get this commited RSN to try to get it in netbsd-5.
It should fix a panic reported with BRIDGE_IPF and pf on current-users
(I'm not sure this panic is reproductible with ipf).
comments ?
PS: the patch is diff -ub for ease of read, but bridge_forward() is of course
properly indented in my local tree.
--
NetBSD: 26 ans d'experience feront toujours la difference
--
Index: if_bridge.c
===================================================================
RCS file: /cvsroot/src/sys/net/if_bridge.c,v
retrieving revision 1.62
diff -u -p -u -b -r1.62 if_bridge.c
--- if_bridge.c 15 Jun 2008 16:37:21 -0000 1.62
+++ if_bridge.c 2 Apr 2009 09:12:37 -0000
@@ -97,6 +97,7 @@ __KERNEL_RCSID(0, "$NetBSD: if_bridge.c,
#include <sys/proc.h>
#include <sys/pool.h>
#include <sys/kauth.h>
+#include <sys/cpu.h>
#if NBPFILTER > 0
#include <net/bpf.h>
@@ -185,7 +186,7 @@ static int bridge_init(struct ifnet *);
static void bridge_stop(struct ifnet *, int);
static void bridge_start(struct ifnet *);
-static void bridge_forward(struct bridge_softc *, struct mbuf *m);
+static void bridge_forward(void *);
static void bridge_timer(void *);
@@ -350,6 +351,13 @@ bridge_clone_create(struct if_clone *ifc
sc->sc_hold_time = BSTP_DEFAULT_HOLD_TIME;
sc->sc_filter_flags = 0;
+ /* software interrupt to do the work */
+ sc->sc_softintr = softint_establish(SOFTINT_NET, bridge_forward, sc);
+ if (sc->sc_softintr == NULL) {
+ free(sc, M_DEVBUF);
+ return ENOMEM;
+ }
+
/* Initialize our routing table. */
bridge_rtable_init(sc);
@@ -370,6 +378,7 @@ bridge_clone_create(struct if_clone *ifc
ifp->if_addrlen = 0;
ifp->if_dlt = DLT_EN10MB;
ifp->if_hdrlen = ETHER_HDR_LEN;
+ IFQ_SET_READY(&ifp->if_snd);
if_attach(ifp);
@@ -410,6 +419,10 @@ bridge_clone_destroy(struct ifnet *ifp)
/* Tear down the routing table. */
bridge_rtable_fini(sc);
+
+
+ softint_disestablish(sc->sc_softintr);
+
free(sc, M_DEVBUF);
return (0);
@@ -1298,11 +1311,24 @@ bridge_start(struct ifnet *ifp)
* 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;
+
+ if ((sc->sc_if.if_flags & IFF_RUNNING) == 0)
+ return;
+
+ s = splbio();
+ while (1) {
+ IFQ_POLL(&sc->sc_if.if_snd, m);
+ if (m == NULL)
+ break;
+ IFQ_DEQUEUE(&sc->sc_if.if_snd, m);
src_if = m->m_pkthdr.rcvif;
@@ -1316,7 +1342,7 @@ bridge_forward(struct bridge_softc *sc,
if (bif == NULL) {
/* Interface is not a bridge member (anymore?) */
m_freem(m);
- return;
+ continue;
}
if (bif->bif_flags & IFBIF_STP) {
@@ -1325,7 +1351,7 @@ bridge_forward(struct bridge_softc *sc,
m_freem(m);
- return;
+ continue;
}
}
@@ -1351,7 +1377,7 @@ bridge_forward(struct bridge_softc *sc,
if ((bif->bif_flags & IFBIF_STP) != 0 &&
bif->bif_state == BSTP_IFSTATE_LEARNING) {
m_freem(m);
- return;
+ continue;
}
/*
@@ -1367,7 +1393,7 @@ bridge_forward(struct bridge_softc *sc,
dst_if = bridge_rtlookup(sc, eh->ether_dhost);
if (src_if == dst_if) {
m_freem(m);
- return;
+ continue;
}
} else {
/* ...forward it to all interfaces. */
@@ -1380,15 +1406,15 @@ bridge_forward(struct bridge_softc *sc,
m->m_pkthdr.rcvif, PFIL_IN) != 0) {
if (m != NULL)
m_freem(m);
- return;
+ continue;
}
if (m == NULL)
- return;
+ continue;
#endif /* PFIL_HOOKS */
if (dst_if == NULL) {
bridge_broadcast(sc, src_if, m);
- return;
+ continue;
}
/*
@@ -1397,13 +1423,13 @@ bridge_forward(struct bridge_softc *sc,
*/
if ((dst_if->if_flags & IFF_RUNNING) == 0) {
m_freem(m);
- return;
+ continue;
}
bif = bridge_lookup_member_if(sc, dst_if);
if (bif == NULL) {
/* Not a member of the bridge (anymore?) */
m_freem(m);
- return;
+ continue;
}
if (bif->bif_flags & IFBIF_STP) {
@@ -1411,11 +1437,13 @@ bridge_forward(struct bridge_softc *sc,
m_freem(m);
- return;
+ continue;
}
}
bridge_enqueue(sc, dst_if, m, 1);
+ }
+ splx(s);
}
/*
@@ -1423,6 +1451,7 @@ bridge_forward(struct bridge_softc *sc,
*
* Receive input from a member interface. Queue the packet for
* bridging if it is not for us.
+ * should be called at splbio()
*/
struct mbuf *
bridge_input(struct ifnet *ifp, struct mbuf *m)
@@ -1464,12 +1493,17 @@ bridge_input(struct ifnet *ifp, struct m
* for bridge processing; return the original packet for
* local processing.
*/
+ if (IF_QFULL(&sc->sc_if.if_snd)) {
+ IF_DROP(&sc->sc_if.if_snd);
+ return (m);
+ }
mc = m_dup(m, 0, M_COPYALL, M_NOWAIT);
if (mc == NULL)
return (m);
/* Perform the bridge forwarding function with the copy. */
- bridge_forward(sc, mc);
+ IF_ENQUEUE(&sc->sc_if.if_snd, mc);
+ softint_schedule(sc->sc_softintr);
/* Return the original packet for local processing. */
return (m);
@@ -1517,7 +1551,13 @@ bridge_input(struct ifnet *ifp, struct m
}
/* Perform the bridge forwarding function. */
- bridge_forward(sc, m);
+ if (IF_QFULL(&sc->sc_if.if_snd)) {
+ IF_DROP(&sc->sc_if.if_snd);
+ m_freem(m);
+ return (NULL);
+ }
+ IF_ENQUEUE(&sc->sc_if.if_snd, m);
+ softint_schedule(sc->sc_softintr);
return (NULL);
}
@@ -2003,6 +2043,7 @@ bridge_ipf(void *arg, struct mbuf **mp,
/*
* Check basic packet sanity and run IPF through pfil.
*/
+ KASSERT(!cpu_intr_p());
switch (ether_type)
{
Index: if_bridgevar.h
===================================================================
RCS file: /cvsroot/src/sys/net/if_bridgevar.h,v
retrieving revision 1.11
diff -u -p -u -b -r1.11 if_bridgevar.h
--- if_bridgevar.h 9 Jul 2007 21:11:00 -0000 1.11
+++ if_bridgevar.h 2 Apr 2009 09:12:37 -0000
@@ -300,6 +300,7 @@ struct bridge_softc {
LIST_HEAD(, bridge_rtnode) sc_rtlist; /* list version of above */
uint32_t sc_rthash_key; /* key for hash */
uint32_t sc_filter_flags; /* ipf and flags */
+ void *sc_softintr;
};
extern const uint8_t bstp_etheraddr[];