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

[Sam Leffler]

> On Thu, 27 Mar 2003, Sam Leffler wrote:
>
> > 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
>
> The purpose of it is to defragment mbuf chains.  As you point out, we
> already have m_copypacket, m_clone, and a bunch of handwritten functions
> in network drivers which try to do various parts of this operation, with
> optimizations.  And, due to these optimizations, they all have
> shortcomings.
>

"shortcomings" is perhaps a misnomer.  Certain functions were written for
specific applications and are not generally applicable.  It sounds like the
m_defrag routine started off for a different need but you're trying to apply
it in several different ways.

> m_defrag shouldn't be called all that often by network drivers, so I'm not
> overly concerned about speed issues; I'm more concerned that it achieve
> its goal in the most correct fashion.
>

It is perfectly acceptable to drop a packet at the network interface level.
It's not a great idea to do it a lot but if protocols are sending large
numbers of highly-fragmented packets then you need to address the problem at
the source, or at least higher up.

> > 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.
>
> Discarding a packet because we're too lazy to defrag it isn't a very good
> solution.
>

Too lazy is the wrong way to think about this.  Handling an aribtrarily
fragmented packet has a cost.  As I said before there is nothing wrong with
dropping a packet in the if layer if it's deemed too expensive to process.

> Also note that this function will be useful in other places where we are
> keeping mbuf chains around for long periods of time (IP reassembly) and
> wish to save memory at the cost of a bit of processor time.  If I optimize
> it so that it doesn't merge mbuf clusters, it'll be a useless function.
>

Your requirements for IP reassembly sound different than where you first
applied m_defrag.

> I'm sure that enhancing the function so that it stops once it reaches
> "goal" would be advantageous, but that's an optimization I'll let someone
> else do in the future.

As I've said already, in the drivers you want to use the minimum-cost
technique to get the packet on the wire.  I think your original
single-cluster version is close to what I would do, but so long as this
stuff only happens as an exception to the normal processing path I really
don't care.  Just keep stats so we can see how much it's happening.

    Sam