On 6/11/07, Matthew Dillon <[EMAIL PROTECTED]> wrote:
:Hi all,
:
:Following patch fixes bridge serializer coverage problem and possible
:serializer ordering problem:
:http://leaf.dragonflybsd.org/~sephe/bridge.diff
:
:cpuN's msgport is used.
:
:Please review/test it.
:
:Best Regards,
:sephe
I'm not sure tags can be ignored that way. Tags are usually not
allocated (I could be wrong, but that is what I remember)... instead
they are often declared as stack variables in the calling procedure
so they have to be stripped by the callee before returning, because
their memory may go poof.
Ah, :) Patch regenerated.
Best Regards,
sephe
--
Live Free or Die
Index: netisr.h
===================================================================
RCS file: /opt/df_cvs/src/sys/net/netisr.h,v
retrieving revision 1.26
diff -u -p -r1.26 netisr.h
--- netisr.h 23 May 2007 08:57:10 -0000 1.26
+++ netisr.h 11 Jun 2007 11:17:02 -0000
@@ -217,6 +217,7 @@ struct netisr {
#ifdef _KERNEL
extern lwkt_port netisr_afree_rport;
+extern lwkt_port netisr_apanic_rport;
lwkt_port_t cpu0_portfn(struct mbuf **mptr);
lwkt_port_t cpu_portfn(int cpu);
Index: bridge/if_bridge.c
===================================================================
RCS file: /opt/df_cvs/src/sys/net/bridge/if_bridge.c,v
retrieving revision 1.24
diff -u -p -r1.24 if_bridge.c
--- bridge/if_bridge.c 6 Jun 2007 13:10:39 -0000 1.24
+++ bridge/if_bridge.c 11 Jun 2007 11:17:02 -0000
@@ -124,6 +124,7 @@
#include <netinet/if_ether.h> /* for struct arpcom */
#include <net/bridge/if_bridgevar.h>
#include <net/if_llc.h>
+#include <net/netmsg2.h>
#include <net/route.h>
#include <sys/in_cksum.h>
@@ -265,6 +266,13 @@ static int bridge_ip6_checkbasic(struct
#endif /* INET6 */
static int bridge_fragment(struct ifnet *, struct mbuf *,
struct ether_header *, int, struct llc *);
+static void bridge_enqueue_internal(struct ifnet *, struct mbuf *m,
+ netisr_fn_t);
+static void bridge_enqueue_handler(struct netmsg *);
+static void bridge_pfil_enqueue_handler(struct netmsg *);
+static void bridge_pfil_enqueue(struct ifnet *, struct mbuf *, int);
+static void bridge_handoff_notags(struct ifnet *, struct mbuf *);
+static void bridge_handoff(struct ifnet *, struct mbuf *);
SYSCTL_DECL(_net_link);
SYSCTL_NODE(_net_link, IFT_BRIDGE, bridge, CTLFLAG_RW, 0, "Bridge");
@@ -367,7 +375,6 @@ struct if_clone bridge_cloner = IF_CLONE
static int
bridge_modevent(module_t mod, int type, void *data)
{
-
switch (type) {
case MOD_LOAD:
LIST_INIT(&bridge_list);
@@ -1348,37 +1355,56 @@ bridge_stop(struct ifnet *ifp)
ifp->if_flags &= ~IFF_RUNNING;
}
-/*
- * bridge_enqueue:
- *
- * Enqueue a packet on a bridge member interface.
- *
- */
-void
-bridge_enqueue(struct ifnet *dst_ifp, struct mbuf *m)
+static void
+bridge_enqueue_internal(struct ifnet *dst_ifp, struct mbuf *m,
+ netisr_fn_t handler)
{
- struct altq_pktattr pktattr;
- struct mbuf *m0;
+ struct netmsg_packet *nmp;
+ lwkt_port_t port;
+ int cpu = mycpu->gd_cpuid;
while (m->m_type == MT_TAG) {
/* XXX see ether_output_frame for full rules check */
m = m->m_next;
}
- lwkt_serialize_enter(dst_ifp->if_serializer);
+ nmp = &m->m_hdr.mh_netmsg;
+ netmsg_init(&nmp->nm_netmsg, &netisr_apanic_rport, 0, handler);
+ nmp->nm_packet = m;
+ nmp->nm_netmsg.nm_lmsg.u.ms_resultp = dst_ifp;
- /* We may be sending a fragment so traverse the mbuf */
- for (; m; m = m0) {
- m0 = m->m_nextpkt;
- m->m_nextpkt = NULL;
+ port = cpu_portfn(cpu);
+ lwkt_sendmsg(port, &nmp->nm_netmsg.nm_lmsg);
+}
- if (ifq_is_enabled(&dst_ifp->if_snd))
- altq_etherclassify(&dst_ifp->if_snd, m, &pktattr);
+static void
+bridge_pfil_enqueue(struct ifnet *dst_ifp, struct mbuf *m,
+ int runfilt)
+{
+ netisr_fn_t handler;
- ifq_handoff(dst_ifp, m, &pktattr);
+ if (runfilt && (inet_pfil_hook.ph_hashooks > 0
+#ifdef INET6
+ || inet6_pfil_hook.ph_hashooks > 0
+#endif
+ )) {
+ handler = bridge_pfil_enqueue_handler;
+ } else {
+ handler = bridge_enqueue_handler;
}
+ bridge_enqueue_internal(dst_ifp, m, handler);
+}
- lwkt_serialize_exit(dst_ifp->if_serializer);
+/*
+ * bridge_enqueue:
+ *
+ * Enqueue a packet on a bridge member interface.
+ *
+ */
+void
+bridge_enqueue(struct ifnet *dst_ifp, struct mbuf *m)
+{
+ bridge_enqueue_internal(dst_ifp, m, bridge_enqueue_handler);
}
/*
@@ -1474,9 +1500,7 @@ bridge_output_serialized(struct ifnet *i
continue;
}
}
- lwkt_serialize_exit(sc->sc_ifp->if_serializer);
bridge_enqueue(dst_if, mc);
- lwkt_serialize_enter(sc->sc_ifp->if_serializer);
}
if (used == 0)
m_freem(m);
@@ -1564,8 +1588,8 @@ bridge_forward(struct bridge_softc *sc,
ASSERT_SERIALIZED(ifp->if_serializer);
- sc->sc_ifp->if_ipackets++;
- sc->sc_ifp->if_ibytes += m->m_pkthdr.len;
+ ifp->if_ipackets++;
+ ifp->if_ibytes += m->m_pkthdr.len;
/*
* Look up the bridge_iflist.
@@ -1590,12 +1614,6 @@ bridge_forward(struct bridge_softc *sc,
eh = mtod(m, struct ether_header *);
/*
- * Various ifp's are used below, release the serializer for
- * the bridge ifp so other ifp serializers can be acquired.
- */
- lwkt_serialize_exit(ifp->if_serializer);
-
- /*
* If the interface is learning, and the source
* address is valid and not multicast, record
* the address.
@@ -1614,7 +1632,7 @@ bridge_forward(struct bridge_softc *sc,
if ((bif->bif_flags & IFBIF_STP) != 0 &&
bif->bif_state == BSTP_IFSTATE_LEARNING) {
m_freem(m);
- goto done;
+ return;
}
/*
@@ -1630,7 +1648,7 @@ bridge_forward(struct bridge_softc *sc,
dst_if = bridge_rtlookup(sc, eh->ether_dhost);
if (src_if == dst_if) {
m_freem(m);
- goto done;
+ return;
}
} else {
/* ...forward it to all interfaces. */
@@ -1638,21 +1656,9 @@ bridge_forward(struct bridge_softc *sc,
dst_if = NULL;
}
- /* run the packet filter */
- if (inet_pfil_hook.ph_hashooks > 0
-#ifdef INET6
- || inet6_pfil_hook.ph_hashooks > 0
-#endif
- ) {
- if (bridge_pfil(&m, ifp, src_if, PFIL_IN) != 0)
- goto done;
- if (m == NULL)
- goto done;
- }
-
if (dst_if == NULL) {
bridge_broadcast(sc, src_if, m, 1);
- goto done;
+ return;
}
/*
@@ -1661,13 +1667,13 @@ bridge_forward(struct bridge_softc *sc,
*/
if ((dst_if->if_flags & IFF_RUNNING) == 0) {
m_freem(m);
- goto done;
+ return;
}
bif = bridge_lookup_member_if(sc, dst_if);
if (bif == NULL) {
/* Not a member of the bridge (anymore?) */
m_freem(m);
- goto done;
+ return;
}
if (bif->bif_flags & IFBIF_STP) {
@@ -1675,21 +1681,29 @@ bridge_forward(struct bridge_softc *sc,
case BSTP_IFSTATE_DISABLED:
case BSTP_IFSTATE_BLOCKING:
m_freem(m);
- goto done;
+ return;
}
}
+ lwkt_serialize_exit(ifp->if_serializer);
+
+ /* run the packet filter */
if (inet_pfil_hook.ph_hashooks > 0
#ifdef INET6
|| inet6_pfil_hook.ph_hashooks > 0
#endif
) {
- if (bridge_pfil(&m, sc->sc_ifp, dst_if, PFIL_OUT) != 0)
+ if (bridge_pfil(&m, ifp, src_if, PFIL_IN) != 0)
+ goto done;
+ if (m == NULL)
+ goto done;
+
+ if (bridge_pfil(&m, ifp, dst_if, PFIL_OUT) != 0)
goto done;
if (m == NULL)
goto done;
}
- bridge_enqueue(dst_if, m);
+ bridge_handoff(dst_if, m);
/*
* ifp's serializer was held on entry and is expected to be held
@@ -1852,8 +1866,11 @@ bridge_input(struct ifnet *ifp, struct m
eh->ether_shost, ifp, 0, IFBAF_DYNAMIC);
}
- m->m_flags |= M_PROTO1; /* XXX loop prevention */
- new_ifp = bif->bif_ifp;
+ if (bif->bif_ifp != ifp) {
+ /* XXX loop prevention */
+ m->m_flags |= M_PROTO1;
+ new_ifp = bif->bif_ifp;
+ }
goto out;
}
@@ -1898,17 +1915,32 @@ bridge_broadcast(struct bridge_softc *sc
{
struct bridge_iflist *bif;
struct mbuf *mc;
- struct ifnet *dst_if;
+ struct ifnet *dst_if, *bifp;
int used = 0;
- /* Filter on the bridge interface before broadcasting */
+ bifp = sc->sc_ifp;
+
+ ASSERT_SERIALIZED(bifp->if_serializer);
+
+ /* run the packet filter */
if (runfilt && (inet_pfil_hook.ph_hashooks > 0
#ifdef INET6
|| inet6_pfil_hook.ph_hashooks > 0
#endif
)) {
- if (bridge_pfil(&m, sc->sc_ifp, NULL, PFIL_OUT) != 0)
- return;
+ lwkt_serialize_exit(bifp->if_serializer);
+
+ /* Filter on the bridge interface before broadcasting */
+
+ if (bridge_pfil(&m, bifp, src_if, PFIL_IN) != 0)
+ goto filt;
+ if (m == NULL)
+ goto filt;
+
+ if (bridge_pfil(&m, bifp, NULL, PFIL_OUT) != 0)
+ m = NULL;
+filt:
+ lwkt_serialize_enter(bifp->if_serializer);
if (m == NULL)
return;
}
@@ -1943,24 +1975,7 @@ bridge_broadcast(struct bridge_softc *sc
continue;
}
}
-
- /*
- * Filter on the output interface. Pass a NULL bridge interface
- * pointer so we do not redundantly filter on the bridge for
- * each interface we broadcast on.
- */
- if (runfilt && (inet_pfil_hook.ph_hashooks > 0
-#ifdef INET6
- || inet6_pfil_hook.ph_hashooks > 0
-#endif
- )) {
- if (bridge_pfil(&mc, NULL, dst_if, PFIL_OUT) != 0)
- continue;
- if (mc == NULL)
- continue;
- }
-
- bridge_enqueue(dst_if, mc);
+ bridge_pfil_enqueue(dst_if, mc, runfilt);
}
if (used == 0)
m_freem(m);
@@ -2813,3 +2828,81 @@ out:
m_freem(m);
return (error);
}
+
+static void
+bridge_enqueue_handler(struct netmsg *nmsg)
+{
+ struct netmsg_packet *nmp;
+ struct ifnet *dst_ifp;
+ struct mbuf *m;
+
+ nmp = (struct netmsg_packet *)nmsg;
+ m = nmp->nm_packet;
+ dst_ifp = nmp->nm_netmsg.nm_lmsg.u.ms_resultp;
+
+ bridge_handoff_notags(dst_ifp, m);
+}
+
+static void
+bridge_pfil_enqueue_handler(struct netmsg *nmsg)
+{
+ struct netmsg_packet *nmp;
+ struct ifnet *dst_ifp;
+ struct mbuf *m;
+
+ nmp = (struct netmsg_packet *)nmsg;
+ m = nmp->nm_packet;
+ dst_ifp = nmp->nm_netmsg.nm_lmsg.u.ms_resultp;
+
+ /*
+ * Filter on the output interface. Pass a NULL bridge interface
+ * pointer so we do not redundantly filter on the bridge for
+ * each interface we broadcast on.
+ */
+ if (inet_pfil_hook.ph_hashooks > 0
+#ifdef INET6
+ || inet6_pfil_hook.ph_hashooks > 0
+#endif
+ ) {
+ if (bridge_pfil(&m, NULL, dst_ifp, PFIL_OUT) != 0)
+ return;
+ if (m == NULL)
+ return;
+ }
+ bridge_handoff_notags(dst_ifp, m);
+}
+
+static void
+bridge_handoff(struct ifnet *dst_ifp, struct mbuf *m)
+{
+ while (m->m_type == MT_TAG) {
+ /* XXX see ether_output_frame for full rules check */
+ m = m->m_next;
+ }
+ bridge_handoff_notags(dst_ifp, m);
+}
+
+static void
+bridge_handoff_notags(struct ifnet *dst_ifp, struct mbuf *m)
+{
+ struct mbuf *m0;
+
+ KKASSERT(m->m_type != MT_TAG);
+
+ lwkt_serialize_enter(dst_ifp->if_serializer);
+
+ /* We may be sending a fragment so traverse the mbuf */
+ for (; m; m = m0) {
+ struct altq_pktattr pktattr;
+
+ m0 = m->m_nextpkt;
+ m->m_nextpkt = NULL;
+
+ if (ifq_is_enabled(&dst_ifp->if_snd))
+ altq_etherclassify(&dst_ifp->if_snd, m, &pktattr);
+
+ ifq_handoff(dst_ifp, m, &pktattr);
+ }
+
+ lwkt_serialize_exit(dst_ifp->if_serializer);
+}
