Re: BPF extensions

[Joerg Sonnenberger <joerg@xxxxxxxxxxxxxxxxx>]

On Tue, Jan 25, 2005 at 11:58:59AM -0800, Jeffrey Hsu wrote:
> Despite what the preceding comment in the code says, it seems to me what
> you really have to prepend a natural word size, not necessarily 32-bits.
> So 'int' seems like the correct data type.
> 
> Plus, there's the following bcopy() of ICHDRLEN, which is defined
> as 'sizeof(u_int)', so at the very least, you'd have to change that
> to to 'sizeof(uint32_t)' to match.

I'll change the ICHDRLEN. The interface contract e.g. with libpcap
definitely says 4 byte, not natural word size.

> After sleeping on it, I think 'bpf_ptap()' is best.

OK, I'll adjust it. http://leaf.dragonflybsd.org/~joerg/bpf.diff

> Can you construct the dummy mbuf header bpf_ptap() and then call bpf_mtap()?
> The comment for bpf_ptap() would then describe its true function,
> which is to construct a dummy mbuf header for a packet before
> calling bpf_mtap().

Done.

> static __inline boolean_t
> bpf_enabled(struct ifnet *ifp)
> {
> 	return (ifp->if_bpf != NULL);
> }
> 
> then the code changes from
> 
> BPF_MTAP2(ifp, eh, ETHER_HDR_LEN, m);
> 
> to
> 
> if (bpf_enabled(ifp))
> 	bpf_ptap(ifp->if_bpf, m, eh, ETHER_HDR_LEN);

I don't want to pull net/if_var.h in net/bpf.h. Well, let's keep the classic
approach.

> 
> (Note, I've reordered the arguments to move the important parameter, the
> packet being tapped to right after the interface.)

It can be argued that the data flow is first eh and afterwards m, but
the other order is fine with me.

> 
> I think not having to go look up what BPF_MTAP2, BPF_MTAP3, etc. means is
> more important than being able to omit the the 'if_bpf' field in the call to
> bpf_ptap().  The code is clearer to the reader if he can see that a call
> is going to be made if bpf is enabled, rather than an opaque macro in 
> capital
> letters.
> 
> If not specifying the 'if_bpf' field is really important to you, than you
> can change bpf_ptap() to take a ifnet pointer as the first argument and
> just do the dereference inside bpf_ptap():

I have to specify the if_bpf field. Both bpf_mtap and bpf_ptab can be used
by interfaces with multiple DLTs, e.g. the 802.11 stack. Many userland
programs are not interested / don't support the additional headers.

Joerg