Date: Thu, 27 Mar 2003 14:48:02 -0800 From: "Sam Leffler" <sam@errno.com> To: "Mike Silbersack" <silby@silby.com> Cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/sys/conf options src/sys/netinet ip_output.c Message-ID: <03d001c2f4b2$ecf461b0$52557f42@errno.com> References: <200303260452.h2Q4quap015364@www.ambrisko.com> <20030326114030.U2075@odysseus.silby.com> <20030326183351.GJ57674@elvis.mu.org> <20030326130903.G2075@odysseus.silby.com> <20030327013224.P7674@odysseus.silby.com> <031301c2f49b$09d2bfb0$52557f42@errno.com> <20030327161237.T748@odysseus.silby.com>
next in thread | previous in thread | raw e-mail | index | archive | help
> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?03d001c2f4b2$ecf461b0$52557f42>