bridge callbacks in if_ed.c?

[Luigi Rizzo]

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 at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
> To unsubscribe, send any mail to "freebsd-hackers-unsubscribe at freebsd.org"