Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 25 Sep 2017 10:04:59 -0700
From:      Stephen Hurd <shurd@sasktel.net>
To:        "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net>
Cc:        Stephen Hurd <shurd@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   Re: svn commit: r323942 - head/sys/net
Message-ID:  <61486844-e6f1-2607-2113-599ebcb02c58@sasktel.net>
In-Reply-To: <4523452E-79B0-494A-B44F-44DE4B747D69@lists.zabbadoz.net>
References:  <201709230135.v8N1ZE6S063264@repo.freebsd.org> <B28D9879-4ECF-43E4-9A58-51F616CEC4BE@lists.zabbadoz.net> <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>

next in thread | previous in thread | raw e-mail | index | archive | help
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).



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?61486844-e6f1-2607-2113-599ebcb02c58>