[please review] TSO mbuf chain length limiting patch

[Lawrence Stewart]

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