From owner-svn-src-head@freebsd.org Mon Sep 25 17:05:06 2017 Return-Path: Delivered-To: svn-src-head@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 B1C86E1FA95; Mon, 25 Sep 2017 17:05:06 +0000 (UTC) (envelope-from shurd@sasktel.net) Received: from mail125c7.megamailservers.com (mail525c7.megamailservers.com [209.235.141.25]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 48E031BD5; Mon, 25 Sep 2017 17:05:05 +0000 (UTC) (envelope-from shurd@sasktel.net) X-Authenticated-User: hurds.sasktel.net X-VIP: 69.49.109.87 Received: from [192.168.0.33] (ip72-194-73-141.oc.oc.cox.net [72.194.73.141]) (authenticated bits=0) by mail125c7.megamailservers.com (8.14.9/8.13.1) with ESMTP id v8PH50pU021872; Mon, 25 Sep 2017 13:05:02 -0400 Subject: Re: svn commit: r323942 - head/sys/net To: "Bjoern A. Zeeb" Cc: Stephen Hurd , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org References: <201709230135.v8N1ZE6S063264@repo.freebsd.org> <283397c7-a01e-3776-7ed3-b64d68003d0b@sasktel.net> <6F5DC92C-2CF6-4A33-9663-BFECB7DB65F2@lists.zabbadoz.net> <89d68ff8-84ed-83a6-4e77-9a321babe2fe@sasktel.net> <4523452E-79B0-494A-B44F-44DE4B747D69@lists.zabbadoz.net> From: Stephen Hurd Message-ID: <61486844-e6f1-2607-2113-599ebcb02c58@sasktel.net> Date: Mon, 25 Sep 2017 10:04:59 -0700 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:51.0) Gecko/20100101 Firefox/51.0 SeaMonkey/2.48 MIME-Version: 1.0 In-Reply-To: <4523452E-79B0-494A-B44F-44DE4B747D69@lists.zabbadoz.net> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CTCH-RefID: str=0001.0A020204.59C93740.009A, ss=1, re=0.000, recu=0.000, reip=0.000, cl=1, cld=1, fgs=0 X-CTCH-VOD: Unknown X-CTCH-Spam: Unknown X-CTCH-Score: 0.000 X-CTCH-Rules: X-CTCH-Flags: 0 X-CTCH-ScoreCust: 0.000 X-CSC: 0 X-CHA: v=2.2 cv=fL1J5dSe c=1 sm=1 tr=0 a=l4Y+EJuLrT/8f1z5FvEQ1g==:117 a=l4Y+EJuLrT/8f1z5FvEQ1g==:17 a=IkcTkHD0fZMA:10 a=6I5d2MoRAAAA:8 a=JWSIKrv1NKH2LzKxutUA:9 a=QEXdDO2ut3YA:10 a=IjZwj45LgO3ly-622nXo:22 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Sep 2017 17:05:06 -0000 Bjoern A. Zeeb wrote: > On 23 Sep 2017, at 23:46, Stephen Hurd wrote: > >> Bjoern A. Zeeb wrote: >>> On 23 Sep 2017, at 6:32, Stephen Hurd wrote: >>> >>>> Bjoern A. Zeeb wrote: >>>>> On 23 Sep 2017, at 1:35, Stephen Hurd wrote: >>>>> >>>>>> Author: shurd >>>>>> Date: Sat Sep 23 01:35:14 2017 >>>>>> New Revision: 323942 >>>>>> URL: https://svnweb.freebsd.org/changeset/base/323942 >>>>>> >>>>>> Log: >>>>>> Chain mbufs before passing to if_input() >>>>>> >>>>>> Build a list of mbufs to pass to if_input() after LRO. Results in >>>>>> 12% small packet forwarding rate improvement. >>>>> forwarding seems a confusing word here.. >>>> >>>> The test was small (64 byte frames) received on one interface, then >>>> sent out on a different one using the net.inet.ip.forwarding sysctl >>>> (controlled via the gateway_enable setting in rc.conf). >>> >>> Then this makes no sense as we don’t do LRO if forwarding is enabled >>> on the machine; >>> https://svnweb.freebsd.org/base/head/sys/netinet/tcp_lro.c?annotate=317390#l645 >>> >> >> Basically, it changed from this: > .. >> To this: > … >> So while before it called if_input() for each separate mbuf that was >> not LROed, it now builds a chain of mbufs that were not LROed, and >> makes a single call to if_input() with the whole chain. For cases >> like packet forwarding where no packets are LROed, performance is >> better. > > Got it, so the “after LRO” in the original commit message is as > confusing as forwarding. > > I not saying anything against the change, I am just saying the commit > message doesn’t describe what it does. Can you explain what was confusing about it or propose other wording? I'm not sure what was confusing, and I'd like to avoid similarly confusing messages in the future. > Also I am pretty sure this works with ether_input but not so much with > fddi_input, iso88025_input, and ifdead_input is probably going to leak > as well. Thanks for the heads up. They all seem to use m_freem(), so they shouldn't leak. It doesn't look like they would actually work though (except ifdead_input of course).