Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 03 Jun 2012 20:33:06 -0700
From:      Colin Percival <cperciva@freebsd.org>
To:        Lawrence Stewart <lstewart@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:  <4FCC2C72.7000806@freebsd.org>
In-Reply-To: <4FCC27FE.3030205@freebsd.org>
References:  <4FC635CC.5030608@freebsd.org> <4FC63D27.70807@cs.duke.edu> <4FCB95F6.30204@freebsd.org> <4FCC27FE.3030205@freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 06/03/12 20:14, Lawrence Stewart wrote:
> On 06/04/12 02:51, Colin Percival wrote:
>> 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?

I don't see this as being overloading; rather, it's using the same term to mean
exactly the same thing: Maximum Segment Size, in this case of a TCP segment
which is going to be re-segmented in hardware.

>> 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.

It's all unsigned now (see updated patch I sent in reply to gallatin@).  The
use of u_int vs. uint32_t is because I wanted to have this fit into spares in
struct ifnet and struct tcpcb and I wasn't 100% certain that there weren't any
platforms with sizeof(int) != sizeof(uint32_t).

-- 
Colin Percival
Security Officer Emeritus, FreeBSD | The power to serve
Founder, Tarsnap | www.tarsnap.com | Online backups for the truly paranoid



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4FCC2C72.7000806>