Date: Fri, 28 Nov 2003 23:41:47 -0800 (PST) From: Sam Leffler <sam@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 43150 for review Message-ID: <200311290741.hAT7flSU089709@repoman.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://perforce.freebsd.org/chv.cgi?CH=43150 Change 43150 by sam@sam_ebb on 2003/11/28 23:40:48 eliminate widespread on-stack mbuf use for bpf by introducing a new bpf_mtap2 routine that does the right thing for an mbuf and a variable-length chunk of data that should be prepended while we're at it cleanup some places where int is assumed to be 4 bytes (when prepending the address family) return M_ASSERTVALID to BPF_MTAP* now that all stack-allocate mbufs have been eliminated Affected files ... .. //depot/projects/netperf/sys/dev/ath/if_ath.c#42 edit .. //depot/projects/netperf/sys/dev/iicbus/if_ic.c#4 edit .. //depot/projects/netperf/sys/dev/ppbus/if_plip.c#4 edit .. //depot/projects/netperf/sys/dev/wi/if_wi.c#16 edit .. //depot/projects/netperf/sys/net/bpf.c#10 edit .. //depot/projects/netperf/sys/net/bpf.h#4 edit .. //depot/projects/netperf/sys/net/if_disc.c#6 edit .. //depot/projects/netperf/sys/net/if_ef.c#6 edit .. //depot/projects/netperf/sys/net/if_faith.c#9 edit .. //depot/projects/netperf/sys/net/if_gif.c#3 edit .. //depot/projects/netperf/sys/net/if_gre.c#4 edit .. //depot/projects/netperf/sys/net/if_loop.c#15 edit .. //depot/projects/netperf/sys/net/if_stf.c#6 edit .. //depot/projects/netperf/sys/net/if_tun.c#6 edit .. //depot/projects/netperf/sys/netgraph/ng_iface.c#3 edit .. //depot/projects/netperf/sys/netinet/ip_gre.c#2 edit Differences ... ==== //depot/projects/netperf/sys/dev/ath/if_ath.c#42 (text+ko) ==== @@ -777,20 +777,10 @@ bpf_mtap(ic->ic_rawbpf, m); if (sc->sc_drvbpf) { - struct mbuf *mb; - - MGETHDR(mb, M_DONTWAIT, m->m_type); - if (mb != NULL) { - sc->sc_tx_th.wt_rate = - ni->ni_rates.rs_rates[ni->ni_txrate]; - - mb->m_next = m; - mb->m_data = (caddr_t)&sc->sc_tx_th; - mb->m_len = sizeof(sc->sc_tx_th); - mb->m_pkthdr.len += mb->m_len; - bpf_mtap(sc->sc_drvbpf, mb); - m_free(mb); - } + sc->sc_tx_th.wt_rate = + ni->ni_rates.rs_rates[ni->ni_txrate]; + bpf_mtap2(sc->sc_drvbpf, + &sc->sc_tx_th, sizeof(sc->sc_tx_th), m); } if (ath_tx_start(sc, ni, bf, m)) { @@ -1711,27 +1701,14 @@ m->m_pkthdr.len = m->m_len = len; if (sc->sc_drvbpf) { - struct mbuf *mb; + sc->sc_rx_th.wr_rate = + sc->sc_hwmap[ds->ds_rxstat.rs_rate]; + sc->sc_rx_th.wr_antsignal = ds->ds_rxstat.rs_rssi; + sc->sc_rx_th.wr_antenna = ds->ds_rxstat.rs_antenna; + /* XXX TSF */ - /* XXX pre-allocate space when setting up recv's */ - MGETHDR(mb, M_DONTWAIT, m->m_type); - if (mb != NULL) { - sc->sc_rx_th.wr_rate = - sc->sc_hwmap[ds->ds_rxstat.rs_rate]; - sc->sc_rx_th.wr_antsignal = - ds->ds_rxstat.rs_rssi; - sc->sc_rx_th.wr_antenna = - ds->ds_rxstat.rs_antenna; - /* XXX TSF */ - - (void) m_dup_pkthdr(mb, m, M_DONTWAIT); - mb->m_next = m; - mb->m_data = (caddr_t)&sc->sc_rx_th; - mb->m_len = sizeof(sc->sc_rx_th); - mb->m_pkthdr.len += mb->m_len; - bpf_mtap(sc->sc_drvbpf, mb); - m_free(mb); - } + bpf_mtap2(sc->sc_drvbpf, + &sc->sc_rx_th, sizeof(sc->sc_rx_th), m); } m_adj(m, -IEEE80211_CRC_LEN); ==== //depot/projects/netperf/sys/dev/iicbus/if_ic.c#4 (text+ko) ==== @@ -66,7 +66,7 @@ #define PCF_MASTER_ADDRESS 0xaa -#define ICHDRLEN sizeof(u_int) +#define ICHDRLEN sizeof(u_int32_t) #define ICMTU 1500 /* default mtu */ struct ic_softc { @@ -369,7 +369,7 @@ int s, len, sent; struct mbuf *mm; u_char *cp; - u_int hdr = dst->sa_family; + u_int32_t hdr = dst->sa_family; ifp->if_flags |= IFF_RUNNING; @@ -400,23 +400,7 @@ } while ((mm = mm->m_next)); - if (ifp->if_bpf) { - struct mbuf m0, *n = m; - - /* - * We need to prepend the address family as - * a four byte field. Cons up a dummy header - * to pacify bpf. This is safe because bpf - * will only read from the mbuf (i.e., it won't - * try to free it or keep a pointer a to it). - */ - m0.m_next = m; - m0.m_len = sizeof(u_int); - m0.m_data = (char *)&hdr; - n = &m0; - - BPF_MTAP(ifp, n); - } + BPF_MTAP2(ifp, &hdr, sizeof(hdr), m); sc->ic_sending = 1; ==== //depot/projects/netperf/sys/dev/ppbus/if_plip.c#4 (text+ko) ==== @@ -445,19 +445,8 @@ static void lptap(struct ifnet *ifp, struct mbuf *m) { - /* - * Send a packet through bpf. We need to prepend the address family - * as a four byte field. Cons up a dummy header to pacify bpf. This - * is safe because bpf will only read from the mbuf (i.e., it won't - * try to free it or keep a pointer to it). - */ u_int32_t af = AF_INET; - struct mbuf m0; - - m0.m_next = m; - m0.m_len = sizeof(u_int32_t); - m0.m_data = (char *)⁡ - BPF_MTAP(ifp, &m0); + BPF_MTAP2(ifp, &af, sizeof(af), m); } static void ==== //depot/projects/netperf/sys/dev/wi/if_wi.c#16 (text+ko) ==== @@ -979,17 +979,8 @@ } #if NBPFILTER > 0 if (sc->sc_drvbpf) { - struct mbuf *mb; - - MGETHDR(mb, M_DONTWAIT, m0->m_type); - if (mb != NULL) { - mb->m_next = m0; - mb->m_data = (caddr_t)&sc->sc_tx_th; - mb->m_len = sizeof(sc->sc_tx_th); - mb->m_pkthdr.len += mb->m_len; - bpf_mtap(sc->sc_drvbpf, mb); - m_free(mb); - } + bpf_mtap2(sc->sc_drvbpf, + &sc->sc_tx_th, sizeof(sc->sc_tx_th), m0); } #endif m_copydata(m0, 0, sizeof(struct ieee80211_frame), @@ -1531,29 +1522,17 @@ #if NBPFILTER > 0 if (sc->sc_drvbpf) { - struct mbuf *mb; - - /* XXX pre-allocate space when setting up recv's */ - MGETHDR(mb, M_DONTWAIT, m->m_type); - if (mb != NULL) { - /* XXX replace divide by table */ - sc->sc_rx_th.wr_rate = frmhdr.wi_rx_rate / 5; - sc->sc_rx_th.wr_antsignal = - WI_RSSI_TO_DBM(sc, frmhdr.wi_rx_signal); - sc->sc_rx_th.wr_antnoise = - WI_RSSI_TO_DBM(sc, frmhdr.wi_rx_silence); - sc->sc_rx_th.wr_time = - htole32((frmhdr.wi_rx_tstamp1 << 16) | - frmhdr.wi_rx_tstamp0); - - (void) m_dup_pkthdr(mb, m, M_DONTWAIT); - mb->m_next = m; - mb->m_data = (caddr_t)&sc->sc_rx_th; - mb->m_len = sizeof(sc->sc_rx_th); - mb->m_pkthdr.len += mb->m_len; - bpf_mtap(sc->sc_drvbpf, mb); - m_free(mb); - } + /* XXX replace divide by table */ + sc->sc_rx_th.wr_rate = frmhdr.wi_rx_rate / 5; + sc->sc_rx_th.wr_antsignal = + WI_RSSI_TO_DBM(sc, frmhdr.wi_rx_signal); + sc->sc_rx_th.wr_antnoise = + WI_RSSI_TO_DBM(sc, frmhdr.wi_rx_silence); + sc->sc_rx_th.wr_time = + htole32((frmhdr.wi_rx_tstamp1 << 16) | + frmhdr.wi_rx_tstamp0); + bpf_mtap2(sc->sc_drvbpf, + &sc->sc_rx_th, sizeof(sc->sc_rx_th), m); } #endif wh = mtod(m, struct ieee80211_frame *); ==== //depot/projects/netperf/sys/net/bpf.c#10 (text+ko) ==== @@ -1227,6 +1227,53 @@ } /* + * Incoming linkage from device drivers, when packet is in + * an mbuf chain and to be prepended by a contiguous header. + */ +void +bpf_mtap2(bp, data, dlen, m) + struct bpf_if *bp; + void *data; + u_int dlen; + struct mbuf *m; +{ + struct mbuf mb; + struct bpf_d *d; + u_int pktlen, slen; + + pktlen = m_length(m, NULL); + if (pktlen == m->m_len) { + bpf_tap(bp, mtod(m, u_char *), pktlen); + return; + } + /* + * Craft on-stack mbuf suitable for passing to bpf_filter. + * Note that we cut corners here; we only setup what's + * absolutely needed--this mbuf should never go anywhere else. + */ + mb.m_next = m; + mb.m_data = data; + mb.m_len = dlen; + + BPFIF_LOCK(bp); + for (d = bp->bif_dlist; d != 0; d = d->bd_next) { + if (!d->bd_seesent && (m->m_pkthdr.rcvif == NULL)) + continue; + BPFD_LOCK(d); + ++d->bd_rcount; + slen = bpf_filter(d->bd_filter, (u_char *)&mb, pktlen, 0); + if (slen != 0) +#ifdef MAC + if (mac_check_bpfdesc_receive(d, bp->bif_ifp) == 0) +#endif + catchpacket(d, (u_char *)&mb, pktlen, slen, + bpf_mcopy); + BPFD_UNLOCK(d); + } + BPFIF_UNLOCK(bp); +} + +/* * Move the packet data from interface memory (pkt) into the * store buffer. Return 1 if it's time to wakeup a listener (buffer full), * otherwise 0. "copy" is the routine called to do the actual data @@ -1580,6 +1627,15 @@ } void +bpf_mtap2(bp, d, l, m) + struct bpf_if *bp; + const void *d; + u_int l; + struct mbuf *m; +{ +} + +void bpfattach(ifp, dlt, hdrlen) struct ifnet *ifp; u_int dlt, hdrlen; ==== //depot/projects/netperf/sys/net/bpf.h#4 (text+ko) ==== @@ -354,6 +354,7 @@ int bpf_validate(const struct bpf_insn *, int); void bpf_tap(struct bpf_if *, u_char *, u_int); void bpf_mtap(struct bpf_if *, struct mbuf *); +void bpf_mtap2(struct bpf_if *, void *, u_int, struct mbuf *); void bpfattach(struct ifnet *, u_int, u_int); void bpfattach2(struct ifnet *, u_int, u_int, struct bpf_if **); void bpfdetach(struct ifnet *); @@ -366,8 +367,16 @@ bpf_tap((_ifp)->if_bpf, (_pkt), (_pktlen)); \ } while (0) #define BPF_MTAP(_ifp,_m) do { \ - if ((_ifp)->if_bpf) \ + if ((_ifp)->if_bpf) { \ + M_ASSERTVALID(_m); \ bpf_mtap((_ifp)->if_bpf, (_m)); \ + } \ +} while (0) +#define BPF_MTAP2(_ifp,_data,_dlen,_m) do { \ + if ((_ifp)->if_bpf) { \ + M_ASSERTVALID(_m); \ + bpf_mtap2((_ifp)->if_bpf,(_data),(_dlen),(_m)); \ + } \ } while (0) #endif ==== //depot/projects/netperf/sys/net/if_disc.c#6 (text+ko) ==== @@ -164,21 +164,8 @@ } if (ifp->if_bpf) { - /* - * We need to prepend the address family as - * a four byte field. Cons up a dummy header - * to pacify bpf. This is safe because bpf - * will only read from the mbuf (i.e., it won't - * try to free it or keep a pointer a to it). - */ - struct mbuf m0; u_int af = dst->sa_family; - - m0.m_next = m; - m0.m_len = 4; - m0.m_data = (char *)⁡ - - BPF_MTAP(ifp, &m0); + bpf_mtap2(ifp->if_bpf, &af, sizeof(af), m); } m->m_pkthdr.rcvif = ifp; ==== //depot/projects/netperf/sys/net/if_ef.c#6 (text+ko) ==== @@ -372,13 +372,7 @@ eifp->if_ibytes += m->m_pkthdr.len + sizeof (*eh); m->m_pkthdr.rcvif = eifp; - if (eifp->if_bpf) { - struct mbuf m0; - m0.m_next = m; - m0.m_len = ETHER_HDR_LEN; - m0.m_data = (char *)eh; - BPF_MTAP(eifp, &m0); - } + BPF_MTAP2(eifp, eh, ETHER_HDR_LEN, m); /* * Now we ready to adjust mbufs and pass them to protocol intr's */ ==== //depot/projects/netperf/sys/net/if_faith.c#9 (text+ko) ==== @@ -213,21 +213,8 @@ } if (ifp->if_bpf) { - /* - * We need to prepend the address family as - * a four byte field. Cons up a faith header - * to pacify bpf. This is safe because bpf - * will only read from the mbuf (i.e., it won't - * try to free it or keep a pointer a to it). - */ - struct mbuf m0; u_int32_t af = dst->sa_family; - - m0.m_next = m; - m0.m_len = 4; - m0.m_data = (char *)⁡ - - BPF_MTAP(ifp, &m0); + bpf_mtap2(ifp->if_bpf, &af, sizeof(af), m); } if (rt && rt->rt_flags & (RTF_REJECT|RTF_BLACKHOLE)) { ==== //depot/projects/netperf/sys/net/if_gif.c#3 (text+ko) ==== @@ -349,21 +349,8 @@ } if (ifp->if_bpf) { - /* - * We need to prepend the address family as - * a four byte field. Cons up a dummy header - * to pacify bpf. This is safe because bpf - * will only read from the mbuf (i.e., it won't - * try to free it or keep a pointer a to it). - */ - struct mbuf m0; u_int32_t af = dst->sa_family; - - m0.m_next = m; - m0.m_len = 4; - m0.m_data = (char *)⁡ - - BPF_MTAP(ifp, &m0); + bpf_mtap2(ifp->if_bpf, &af, sizeof(af), m); } ifp->if_opackets++; ifp->if_obytes += m->m_pkthdr.len; @@ -418,21 +405,8 @@ #endif if (ifp->if_bpf) { - /* - * We need to prepend the address family as - * a four byte field. Cons up a dummy header - * to pacify bpf. This is safe because bpf - * will only read from the mbuf (i.e., it won't - * try to free it or keep a pointer a to it). - */ - struct mbuf m0; u_int32_t af1 = af; - - m0.m_next = m; - m0.m_len = 4; - m0.m_data = (char *)&af1; - - BPF_MTAP(ifp, &m0); + bpf_mtap2(ifp->if_bpf, &af1, sizeof(af1), m); } if (ng_gif_input_p != NULL) { ==== //depot/projects/netperf/sys/net/if_gre.c#4 (text+ko) ==== @@ -240,15 +240,8 @@ osrc = 0; if (ifp->if_bpf) { - /* see comment of other if_foo.c files */ - struct mbuf m0; u_int32_t af = dst->sa_family; - - m0.m_next = m; - m0.m_len = 4; - m0.m_data = (char *)⁡ - - BPF_MTAP(ifp, &m0); + bpf_mtap2(ifp->if_bpf, &af, sizeof(af), m); } m->m_flags &= ~(M_BCAST|M_MCAST); ==== //depot/projects/netperf/sys/net/if_loop.c#15 (text+ko) ==== @@ -252,22 +252,14 @@ /* Let BPF see incoming packet */ if (ifp->if_bpf) { - struct mbuf m0, *n = m; - if (ifp->if_bpf->bif_dlt == DLT_NULL) { + u_int32_t af1 = af; /* XXX beware sizeof(af) != 4 */ /* - * We need to prepend the address family as - * a four byte field. Cons up a dummy header - * to pacify bpf. This is safe because bpf - * will only read from the mbuf (i.e., it won't - * try to free it or keep a pointer a to it). + * We need to prepend the address family. */ - m0.m_next = m; - m0.m_len = 4; - m0.m_data = (char *)⁡ - n = &m0; - } - BPF_MTAP(ifp, n); + bpf_mtap2(ifp->if_bpf, &af1, sizeof(af1), m); + } else + bpf_mtap(ifp->if_bpf, m); } /* Strip away media header */ ==== //depot/projects/netperf/sys/net/if_stf.c#6 (text+ko) ==== @@ -430,17 +430,17 @@ * will only read from the mbuf (i.e., it won't * try to free it or keep a pointer a to it). */ + u_int32_t af = AF_INET6; +#ifdef HAVE_OLD_BPF struct mbuf m0; - u_int32_t af = AF_INET6; m0.m_next = m; m0.m_len = 4; m0.m_data = (char *)⁡ -#ifdef HAVE_OLD_BPF BPF_MTAP(ifp, &m0); #else - bpf_mtap(ifp->if_bpf, &m0); + bpf_mtap2(ifp->if_bpf, &af, sizeof(af), m); #endif } #endif /*NBPFILTER > 0*/ @@ -685,17 +685,17 @@ * will only read from the mbuf (i.e., it won't * try to free it or keep a pointer a to it). */ + u_int32_t af = AF_INET6; +#ifdef HAVE_OLD_BPF struct mbuf m0; - u_int32_t af = AF_INET6; m0.m_next = m; m0.m_len = 4; m0.m_data = (char *)⁡ -#ifdef HAVE_OLD_BPF BPF_MTAP(ifp, &m0); #else - bpf_mtap(ifp->if_bpf, &m0); + bpf_mtap2(ifp->if_bpf, &af, sizeof(ah), m); #endif } ==== //depot/projects/netperf/sys/net/if_tun.c#6 (text+ko) ==== @@ -456,21 +456,8 @@ } if (ifp->if_bpf) { - /* - * We need to prepend the address family as - * a four byte field. Cons up a dummy header - * to pacify bpf. This is safe because bpf - * will only read from the mbuf (i.e., it won't - * try to free it or keep a pointer to it). - */ - struct mbuf m; uint32_t af = dst->sa_family; - - m.m_next = m0; - m.m_len = 4; - m.m_data = (char *)⁡ - - BPF_MTAP(ifp, &m); + bpf_mtap2(ifp->if_bpf, &af, sizeof(af), m0); } /* prepend sockaddr? this may abort if the mbuf allocation fails */ @@ -743,39 +730,6 @@ mac_create_mbuf_from_ifnet(ifp, top); #endif - if (ifp->if_bpf) { - if (tp->tun_flags & TUN_IFHEAD) { - /* - * Conveniently, we already have a 4-byte address - * family prepended to our packet ! - * Inconveniently, it's in the wrong byte order ! - */ - if ((top = m_pullup(top, sizeof(family))) == NULL) - return (ENOBUFS); - *mtod(top, u_int32_t *) = - ntohl(*mtod(top, u_int32_t *)); - BPF_MTAP(ifp, top); - *mtod(top, u_int32_t *) = - htonl(*mtod(top, u_int32_t *)); - } else { - /* - * We need to prepend the address family as - * a four byte field. Cons up a dummy header - * to pacify bpf. This is safe because bpf - * will only read from the mbuf (i.e., it won't - * try to free it or keep a pointer to it). - */ - struct mbuf m; - uint32_t af = AF_INET; - - m.m_next = top; - m.m_len = 4; - m.m_data = (char *)⁡ - - BPF_MTAP(ifp, &m); - } - } - if (tp->tun_flags & TUN_IFHEAD) { if (top->m_len < sizeof(family) && (top = m_pullup(top, sizeof(family))) == NULL) @@ -785,6 +739,8 @@ } else family = AF_INET; + BPF_MTAP2(ifp, &family, sizeof(family), top); + switch (family) { #ifdef INET case AF_INET: ==== //depot/projects/netperf/sys/netgraph/ng_iface.c#3 (text+ko) ==== @@ -485,16 +485,10 @@ static void ng_iface_bpftap(struct ifnet *ifp, struct mbuf *m, sa_family_t family) { - int32_t family4 = (int32_t)family; - struct mbuf m0; - KASSERT(family != AF_UNSPEC, ("%s: family=AF_UNSPEC", __func__)); if (ifp->if_bpf != NULL) { - bzero(&m0, sizeof(m0)); - m0.m_next = m; - m0.m_len = sizeof(family4); - m0.m_data = (char *)&family4; - BPF_MTAP(ifp, &m0); + int32_t family4 = (int32_t)family; + bpf_mtap2(ifp->if_bpf, &family4, sizeof(family4), m); } } ==== //depot/projects/netperf/sys/netinet/ip_gre.c#2 (text+ko) ==== @@ -196,14 +196,8 @@ m->m_pkthdr.len -= hlen; if (sc->sc_if.if_bpf) { - struct mbuf m0; u_int32_t af = AF_INET; - - m0.m_next = m; - m0.m_len = 4; - m0.m_data = (char *)⁡ - - BPF_MTAP(&(sc->sc_if), &m0); + bpf_mtap2(sc->sc_if.if_bpf, &af, sizeof(af), m); } m->m_pkthdr.rcvif = &sc->sc_if; @@ -283,14 +277,8 @@ ip->ip_sum = in_cksum(m, (ip->ip_hl << 2)); if (sc->sc_if.if_bpf) { - struct mbuf m0; - u_int af = AF_INET; - - m0.m_next = m; - m0.m_len = 4; - m0.m_data = (char *)⁡ - - BPF_MTAP(&(sc->sc_if), &m0); + u_int32_t af = AF_INET; + bpf_mtap2(sc->sc_if.if_bpf, &af, sizeof(af), m); } m->m_pkthdr.rcvif = &sc->sc_if;
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200311290741.hAT7flSU089709>