On Fri, Sep 27, 2002 at 10:06:05AM +1000, Darren Reed wrote:

 > To each of 'these', I'm planning on adding this:
 > 
 >                   /* XXX mbufs are not usually on the stack ... */
 >                   m.m_type = -1;
 >                   m.m_flags = 0;
 > 
 > Before I *do* this, does anyone want to pipe up and say "bad boy, use
 > MGET() to get a _real_ mbuf for this" ?

Hm..  In some sense, you do want to MGET(), because that would reduce
stack usage.  Since these code paths are used only when doing BPF taps,
the performance penalty of the MGET() will only be incurred there, and
if the MGET() fails, well, "so I guess we don't tap that one." :-)

The only other problem I see then is the assignment of m->m_data without
proper external storage tracking ... but for the purposes of the tap, it
probably doesn't really matter that much to do proper tracking.

...I suppose another option would be to have an "mbuf initializer" macro
that made sure all the fields were set properly... but that doesn't address
the stack usage problem.  I suppose the stack usage hasn't been a problem so
far, so maybe this is the best choice.

 > The issue here is that if bpf_mtap() and further into the BPF code
 > ever expect the pointer it gets passed to be a _real_ mbuf and tries
 > to do things with it, it could run into trouble.  The tradeoff is
 > speed.  I'm quite happy to put a large comment in bpf.c saying
 > something along the lines of "not all mbufs are equal and don't
 > ever try to alter the mbuf chain in here".

Right, BPF for all time has simply copied the new mbufs into its own
ring buffer, and not kept references to them.  I don't see why this
would change ever :-)

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>