On Wed, Mar 15, 2006 at 06:34:41PM +0900, YAMAMOTO Takashi wrote:
> > I think setting m_len to 0 will cause the m_copyback() to add a second mbuf to
> > the chain to store the data, leaving the first one with a NULL size.
> 
> yes, it's why i said "the minimal fix". :-)
> 
> it's also trivial to make m_copyback to use trailing space of
> the given mbuf if any.  but let's fix the problem first,
> and then optimize it.

But some things won't like a zero-lenght mbuf at the head of the chain
(beside being hihgly inefficient for small packets).

> 
> > Now, it seems that m_copyback() adjust m->m_pkthdr.len, but not m->m_len.
> > Should it ? It so, adding
> > 			m->m_len = m->m_pkthdr.len;
> > after the call to m_copyback() should be enough.
> 
> i'd say the mbuf chain is "broken" unless m->m_pkthdr.len is the sum of
> m->m_len in the chain.

Sure.

> 
> besides, i don't think it's a good idea to assume that
> the result of m_copyback is a single mbuf.

Of course :) So we should set m_len to min(MHLEN, rx->status) before calling
m_copyback(). Also, it seems m_copyback() will never allocate a mbuf cluster,
so it won't be efficient for large packets. I wonder if using m_copyback()
here was a good idea at all ...

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--