Date: Mon, 04 Jun 2012 13:14:06 +1000 From: Lawrence Stewart <lstewart@freebsd.org> To: Colin Percival <cperciva@freebsd.org> Cc: freebsd-net@freebsd.org, Andre Oppermann <andre@FreeBSD.ORG>, Andrew Gallatin <gallatin@cs.duke.edu> Subject: Re: [please review] TSO mbuf chain length limiting patch Message-ID: <4FCC27FE.3030205@freebsd.org> In-Reply-To: <4FCB95F6.30204@freebsd.org> References: <4FC635CC.5030608@freebsd.org> <4FC63D27.70807@cs.duke.edu> <4FCB95F6.30204@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 06/04/12 02:51, Colin Percival wrote: > On 05/30/12 08:30, Andrew Gallatin wrote: >> On 05/30/12 10:59, Colin Percival wrote: >>> The Xen virtual network interface has an issue (ok, really the issue is with >>> the linux back-end, but that's what most people are using) where it can't >>> handle scatter-gather writes with lots of pieces, aka. long mbuf chains. >>> This currently bites us hard with TSO enabled, since it produces said long >>> mbuf chains. >> >> I think a better approach would be to have a limit on the size of the >> pre-segmented TCP payload size sent to the driver. I tend to think >> that this would be more generically useful, and it is a better match >> for the NDIS APIs, where a driver must specify the max TSO size. I >> think the changes to the TCP stack might be simpler (eg, they >> would seem to jive better with the existing "maxmtu" approach). >> >> I think this could work for you as well. You could set the Xen max >> tso size to be 32K (derived from 18 pages/skb, multiplied by a typical >> 2KB mbuf size, with some slack built in). If the chain was too large, >> you could m_defrag it down to size. > > I've attached a new patch which: > 1. adds a IFCAP_TSO_MSS "capability" and a if_tx_tso_mss field to struct ifnet, A minor thing, but I don't like the overloading of the term MSS. Perhaps s/MSS/CHUNKSIZE or another suitably similar but different word/phrase? > 2. sets these in netfront when the IFCAP_TSO4 flag is set, > 3. extends tcp_maxmtu to read this value, > 4. adds a tx_tso_mss field to struct tcpcb, > 5. makes tcp_mss_update set tx_tso_mss using tcp_maxmtu, and > 6. limits TSO lengths to tx_tso_mss in tcp_output. Is there a reason to mix signed and unsigned types for the various tx_tso_mss variables? If not, please pick a type and stick with it. I've added andre@ to CC in the hope he will eyeball this too given that he, if I recall correctly, was the original "Mr TSO". Cheers, Lawrence
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4FCC27FE.3030205>