From owner-freebsd-stable@freebsd.org Thu Aug 20 02:30:37 2015 Return-Path: Delivered-To: freebsd-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 8FE929BD6AD; Thu, 20 Aug 2015 02:30:37 +0000 (UTC) (envelope-from pyunyh@gmail.com) Received: from mail-pd0-x22f.google.com (mail-pd0-x22f.google.com [IPv6:2607:f8b0:400e:c02::22f]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 5650E1D29; Thu, 20 Aug 2015 02:30:37 +0000 (UTC) (envelope-from pyunyh@gmail.com) Received: by pdbmi9 with SMTP id mi9so8247316pdb.3; Wed, 19 Aug 2015 19:30:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:date:to:cc:subject:message-id:reply-to:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=eQTimaEialIMMPKihgRhyaqLUInJ7JYcDtq5NJmch/I=; b=JRe0lBHMhWSn+1p95Bnf9eLM6MkepZq9Bqaiz42vniZTCxj4UXD3CsRnS6HxO5P4eA js3PXcW32KC0MX59qkX4T5nmuMnFXOLHrP1wpaqtJUqIKgZUJDArcg5u5Vj7bl3lroJY AkTNO6Ledl6HE8OkwLdbz6HIZ1tCBPsQM+rZKhtPhY0broHi3Ckrcwi2DZfzU3EZHR8I c8c7K8o7DLxr9mZwfjmHlao5mOVJIIEXArzZ+SKWR/GTPZMzV8mVsHf9Lj+xrPamcwjx Ize2seGECskFd5LcGGo7gqJchVN8Vfpg+qDVUA3yXy0WR3R1kUq+hQJ4wpt/ETh9svbh Tiqg== X-Received: by 10.70.103.74 with SMTP id fu10mr1578826pdb.11.1440037836738; Wed, 19 Aug 2015 19:30:36 -0700 (PDT) Received: from pyunyh@gmail.com ([106.247.248.2]) by smtp.gmail.com with ESMTPSA id em1sm2330892pbd.42.2015.08.19.19.30.31 (version=TLSv1 cipher=RC4-SHA bits=128/128); Wed, 19 Aug 2015 19:30:35 -0700 (PDT) From: Yonghyeon PYUN X-Google-Original-From: "Yonghyeon PYUN" Received: by pyunyh@gmail.com (sSMTP sendmail emulation); Thu, 20 Aug 2015 11:30:24 +0900 Date: Thu, 20 Aug 2015 11:30:24 +0900 To: Rick Macklem Cc: Hans Petter Selasky , FreeBSD stable , FreeBSD Net , Slawa Olhovchenkov , Christopher Forgeron , Daniel Braniss , Gleb Smirnoff Subject: Re: ix(intel) vs mlxen(mellanox) 10Gb performance Message-ID: <20150820023024.GB996@michelle.fasterthan.com> Reply-To: pyunyh@gmail.com References: <1D52028A-B39F-4F9B-BD38-CB1D73BF5D56@cs.huji.ac.il> <9D8B0503-E8FA-43CA-88F0-01F184F84D9B@cs.huji.ac.il> <1721122651.24481798.1439902381663.JavaMail.zimbra@uoguelph.ca> <55D333D6.5040102@selasky.org> <1325951625.25292515.1439934848268.JavaMail.zimbra@uoguelph.ca> <55D429A4.3010407@selasky.org> <20150819074212.GB964@michelle.fasterthan.com> <55D43615.1030401@selasky.org> <2013503980.25726607.1439989235806.JavaMail.zimbra@uoguelph.ca> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2013503980.25726607.1439989235806.JavaMail.zimbra@uoguelph.ca> User-Agent: Mutt/1.4.2.3i X-BeenThere: freebsd-stable@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: Production branch of FreeBSD source code List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Aug 2015 02:30:37 -0000 On Wed, Aug 19, 2015 at 09:00:35AM -0400, Rick Macklem wrote: > Hans Petter Selasky wrote: > > On 08/19/15 09:42, Yonghyeon PYUN wrote: > > > On Wed, Aug 19, 2015 at 09:00:52AM +0200, Hans Petter Selasky wrote: > > >> On 08/18/15 23:54, Rick Macklem wrote: > > >>> Ouch! Yes, I now see that the code that counts the # of mbufs is before > > >>> the > > >>> code that adds the tcp/ip header mbuf. > > >>> > > >>> In my opinion, this should be fixed by setting if_hw_tsomaxsegcount to > > >>> whatever > > >>> the driver provides - 1. It is not the driver's responsibility to know if > > >>> a tcp/ip > > >>> header mbuf will be added and is a lot less confusing that expecting the > > >>> driver > > >>> author to know to subtract one. (I had mistakenly thought that > > >>> tcp_output() had > > >>> added the tc/ip header mbuf before the loop that counts mbufs in the > > >>> list. > > >>> Btw, > > >>> this tcp/ip header mbuf also has leading space for the MAC layer header.) > > >>> > > >> > > >> Hi Rick, > > >> > > >> Your question is good. With the Mellanox hardware we have separate > > >> so-called inline data space for the TCP/IP headers, so if the TCP stack > > >> subtracts something, then we would need to add something to the limit, > > >> because then the scatter gather list is only used for the data part. > > >> > > > > > > I think all drivers in tree don't subtract 1 for > > > if_hw_tsomaxsegcount. Probably touching Mellanox driver would be > > > simpler than fixing all other drivers in tree. > > > > > >> Maybe it can be controlled by some kind of flag, if all the three TSO > > >> limits should include the TCP/IP/ethernet headers too. I'm pretty sure > > >> we want both versions. > > >> > > > > > > Hmm, I'm afraid it's already complex. Drivers have to tell almost > > > the same information to both bus_dma(9) and network stack. > > > > Don't forget that not all drivers in the tree set the TSO limits before > > if_attach(), so possibly the subtraction of one TSO fragment needs to go > > into ip_output() .... > > > Ok, I realized that some drivers may not know the answers before ether_ifattach(), > due to the way they are configured/written (I saw the use of if_hw_tsomax_update() > in the patch). I was not able to find an interface that configures TSO parameters after if_t conversion. I'm under the impression if_hw_tsomax_update() is not designed to use this way. Probably we need a better one?(CCed to Gleb). > > If it is subtracted as a part of the assignment to if_hw_tsomaxsegcount in tcp_output() > at line#791 in tcp_output() like the following, I don't think it should matter if the > values are set before ether_ifattach()? > /* > * Subtract 1 for the tcp/ip header mbuf that > * will be prepended to the mbuf chain in this > * function in the code below this block. > */ > if_hw_tsomaxsegcount = tp->t_tsomaxsegcount - 1; > > I don't have a good solution for the case where a driver doesn't plan on using the > tcp/ip header provided by tcp_output() except to say the driver can add one to the > setting to compensate for that (and if they fail to do so, it still works, although > somewhat suboptimally). When I now read the comment in sys/net/if_var.h it is clear > what it means, but for some reason I didn't read it that way before? (I think it was > the part that said the driver didn't have to subtract for the headers that confused me?) > In any case, we need to try and come up with a clear definition of what they need to > be set to. > > I can now think of two ways to deal with this: > 1 - Leave tcp_output() as is, but provide a macro for the device driver authors to use > that sets if_hw_tsomaxsegcount with a flag for "driver uses tcp/ip header mbuf", > documenting that this flag should normally be true. > OR > 2 - Change tcp_output() as above, noting that this is a workaround for confusion w.r.t. > whether or not if_hw_tsomaxsegcount should include the tcp/ip header mbuf and > update the comment in if_var.h to reflect this. Then drivers that don't use the > tcp/ip header mbuf can increase their value for if_hw_tsomaxsegcount by 1. > (The comment should also mention that a value of 35 or greater is much preferred to > 32 if the hardware will support that.) > Both works for me. My preference is 2 just because it's very common for most drivers that use tcp/ip header mbuf.