netdev - Re: [PATCHv2 net] net: restore lro after device detached from bridge

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 02 Feb 2015 02:35:02 -0800
From:	Alexander Duyck <alexander.h.duyck@...hat.com>
To:	Fan Du <fengyuleidian0615@...il.com>
CC:	Fan Du <fan.du@...el.com>, bhutchings@...arflare.com,
	davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCHv2 net] net: restore lro after device detached from bridge

On 02/01/2015 06:20 PM, Fan Du wrote:
> 于 2015年01月31日 04:48, Alexander Duyck 写道:
>> On 01/30/2015 04:33 AM, Fan Du wrote:
>>> Either detaching a device from bridge or switching a device
>>> out of FORWARDING state, the original lro feature should
>>> possibly be enabled for good reason, e.g. hw feature like
>>> receive side coalescing could come into play.
>>>
>>> BEFORE:
>>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k 
>>> ens806f0 | grep large
>>> large-receive-offload: off
>>>
>>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k 
>>> ens806f0 | grep large
>>> large-receive-offload: off
>>>
>>> AFTER:
>>> echo 1 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k 
>>> ens806f0 | grep large
>>> large-receive-offload: off
>>>
>>> echo 0 > /proc/sys/net/ipv4/conf/ens806f0/forwarding && ethtool -k 
>>> ens806f0 | grep large
>>> large-receive-offload: on
>>>
>>> Signed-off-by: Fan Du <fan.du@...el.com>
>>> Fixes: 0187bdfb0567 ("net: Disable LRO on devices that are forwarding")
>>
>
>> First off this isn't a "fix".  This is going to likely break more than
>> it fixes.  The main reason why LRO is disabled is because it can cause
>> more harm then it helps.  Since GRO is available we should err on the
>> side of caution since enabling LRO/RSC can have undesirable side effects
>> in a number of cases.
>
> I think you are talking about bad scenarios when net device is 
> attached to a bridge.
> Then what's the good reason user has to pay extra cpu power for using 
> GRO, instead
> of using hw capable LRO/RSC when this net device is detached from 
> bridge acting as
> a standalone NIC?
>
> Note, SRC is defaulted to *ON* in practice for ALL ixgbe NICs, as same 
> other RSC capable
> NICs. Attaching net device to a bridge _once_ should not changed its 
> default configuration,
> moreover it's a subtle change without any message that user won't 
> noticed at all.

No, RSC only has benefits for IPv4/TCP large packets.  However 
historically there have been issues seen w/ small packet performance 
with RSC enabled.  Some have been addressed, however there are still 
other effects such as increasing latency for receive unless the push 
flag is set in the frame.

I still say this patch is not valid, even with your changes.  Your 
performance gain doesn't trump the regressions you would be causing on 
other peoples platforms.

I would suggest figuring out why you are seeing issues with routing or 
bridging being enabled and disabled and possibly cleaning up the issue 
via a script rather than trying to modify the kernel to make it take 
care of it for you.

- Alex
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html