Date: Sun, 5 Sep 2004 14:20:36 -0700 From: Luigi Rizzo <luigi@freebsd.org> To: Gleb Smirnoff <glebius@freebsd.org> Cc: net@freebsd.org Subject: Re: bridge callbacks in if_ed.c? Message-ID: <20040905142036.A23213@xorpc.icir.org> In-Reply-To: <20040905205249.GA81337@cell.sick.ru>; from glebius@freebsd.org on Mon, Sep 06, 2004 at 12:52:49AM %2B0400 References: <20040905205249.GA81337@cell.sick.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Sep 06, 2004 at 12:52:49AM +0400, Gleb Smirnoff wrote:
> Luigi,
>
> I see that bridge callbacks are still living in if_ed.c
> from FreeBSD 2.x times. See if_ed.c:2816. I think this is
> not correct.
>
> Bridge code is called from ether_input(), which is
> indirectly called from if_ed.c:2836.
>
> Any objections about attached patch?
there are performance reasons to do this way -- grabbing
the entire packet is expensive because it is done via programmed
I/O, so the current code only grabs the header, does the
filtering, and grabs the rest of the packet only if
needed.
Probably the current code runs bridge_in_ptr() twice, but I
believe this is still cheaper than grabbing all packets
entirely.
I'd rather not apply the patch unless you can show that
the current code leads to incorrect behaviour.
cheers
luigi
> [ccing hackers@ and net@ to get more eyes reviewing]
>
> --
> Totus tuus, Glebius.
> GLEBIUS-RIPN GLEB-RIPE
> Index: if_ed.c
> ===================================================================
> RCS file: /home/ncvs/src/sys/dev/ed/if_ed.c,v
> retrieving revision 1.233
> diff -u -r1.233 if_ed.c
> --- if_ed.c 13 Aug 2004 23:04:23 -0000 1.233
> +++ if_ed.c 5 Sep 2004 20:48:19 -0000
> @@ -2810,26 +2810,9 @@
> eh = mtod(m, struct ether_header *);
>
> /*
> - * Don't read in the entire packet if we know we're going to drop it
> - * and no bpf is active.
> + * Get packet, including link layer address, from interface.
> */
> - if (!ifp->if_bpf && BDG_ACTIVE( (ifp) ) ) {
> - struct ifnet *bif;
> -
> - ed_ring_copy(sc, buf, (char *)eh, ETHER_HDR_LEN);
> - bif = bridge_in_ptr(ifp, eh) ;
> - if (bif == BDG_DROP) {
> - m_freem(m);
> - return;
> - }
> - if (len > ETHER_HDR_LEN)
> - ed_ring_copy(sc, buf + ETHER_HDR_LEN,
> - (char *)(eh + 1), len - ETHER_HDR_LEN);
> - } else
> - /*
> - * Get packet, including link layer address, from interface.
> - */
> - ed_ring_copy(sc, buf, (char *)eh, len);
> + ed_ring_copy(sc, buf, (char *)eh, len);
>
> m->m_pkthdr.len = m->m_len = len;
>
> _______________________________________________
> freebsd-hackers@freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe@freebsd.org"
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20040905142036.A23213>