cvs commit: src/sys/conf options src/sys/netinet ip_output.c

[Maxime Henrion]

Mike Silbersack wrote:
> 
> On Tue, 25 Mar 2003, Doug Ambrisko wrote:
> 
> > | I think that we'll end up being even better off by just making a
> > | m_defragment function, thereby reducing code duplication even more.  I'll
> > | make sure to test any such function more than I tested the MBUF_FRAG_TEST
> > | code, however. :)
> >
> > Yes, I was thinking that as well.
> 
> Ok, here's my attempt at a m_defragment function.  It seems to work well
> here, although I have only tested it with the if_xl driver.  Could you
> guys give it a quick lookover to see if it has any bugs?  I'll add the
> ability to copy mbuf chains longer than MCLBYTES once I know all the
> packet header handling / etc is correct.
> 
> Also note that the patch contains my changes to if_xl, which are mostly
> debugging code.  You can ignore most of that.  One change I _will_ make to
> if_xl, however, is to walk the chain and find out its length _before_ we
> do any busdma calls; this moves the defragmentation occur earlier, thereby
> avoiding the need for a complex error recovery case.  I suspect we'll want
> to do the same in the other network drivers.

Nice work!  This will really be very useful to have for the network
interface drivers.  I have a few comments :

- You removed the m_getcl() optimization that has been added in if_xl.c
recently.  You should use m_getcl() if len is > MHLEN, because m_getcl()
grabs both a header and a cluster.  If len is <= MHLEN, you can just use
m_gethdr().  I guess you did this because you have been hitting the
KASSERT() in m_dup_pkthdr(), and I think this KASSERT() is a bit bogus.
It seems to me there should be a way to do it without having to delay
the cluster allocation.  Maybe Sam or Robert could comment on this?

- I believe an mbuf chain passed to a network driver should always have
a packet header.

- Minor style(9) nits: you should test pointers against NULL and not using them
as booleans.  Also, there's a trailing newline in the m_defragment()
function.

Thanks for doing this work!

Cheers,
Maxime