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

Sam Leffler sam at errno.com
Thu Mar 27 11:57:12 PST 2003


> Ok, I think I have m_defrag in a working state, please review.
>
> Changes from last time:
>
> - It supports infinitely long chains.
> - It has a "goal" argument which is supposed to be a hint to tell m_defrag
> how long the chain can be.  It is currently ignored; someone may want to
> honor this later for optimization purposes.
> - I fixed up the error case in if_xl, it only runs if needed now
>
> At the top of the if_loop and if_xl patches are debugging code I used to
> make it was working right, they're certainly not going in.
>
> m_defrag should be totally free of debug code.

I thought about this some more.  If the purpose of m_defrag* is to handle
problems in drivers where the s/g requirements are too large then you want
to do the minimum amount of work since the results going to be used once and
thrown away.  This says to me that you want to coalesce only until you know
you've got things in a form that the DMA engine can handle.  This actually
makes the requirements very close to those of the m_clone routine in fast
ipsec except m_clone must generate a writable mbuf chain while the drivers
are happy with having read-only elements.  My experience was that you don't
want to coalesce too agressively as you can end up copying lots of data for
little positive effect.  In particular I would not coalesce anything but
mbufs (i.e. leave mbufs w/ external storage alone).  Unfortunately this can
leave you with a result that doesn't meet the max s/g requirements of the
driver unless you make a second pass.  Given that this problem should happen
infrequently and that you're just trying to avoid discarding the packet I
think you're best off doing a best effort job where you coalesce only mbufs
and leave clusters/ext's alone.  Then if the subsequent bus_dma_load_mbuf
call fails you discard the packet.  Other than the read/write requirements
this is exactly m_clone.

FWIW when I've dealt with this issue in drivers I've always said:  if the
packet doesn't fit in a cluster then return an error; otherwise allocate a
cluster and copy everything in.  This insures you know you'll need at most 1
s/g descriptor so you to pre-allocate the resource in the driver and not end
up coalescing the data only to find you can't send it.

    Sam



More information about the cvs-src mailing list