svn commit: r327559 - in head: . sys/net

[Steven Hartland]

On 04/01/2018 20:50, Eugene Grosbein wrote:
> 05.01.2018 3:05, Steven Hartland wrote:
>
>> Author: smh
>> Date: Thu Jan  4 20:05:47 2018
>> New Revision: 327559
>> URL: https://svnweb.freebsd.org/changeset/base/327559
>>
>> Log:
>>    Disabled the use of flowid for lagg by default
>>    
>>    Disabled the use of RSS hash from the network card aka flowid for
>>    lagg(4) interfaces by default as it's currently incompatible with
>>    the lacp and loadbalance protocols.
>>    
>>    The incompatibility is due to the fact that the flowid isn't know
>>    for the first packet of a new outbound stream which can result in
>>    the hash calculation method changing and hence a stream being
>>    incorrectly split across multiple interfaces during normal
>>    operation.
>>    
>>    This can be re-enabled by setting the following in loader.conf:
>>    net.link.lagg.default_use_flowid="1"
>>    
>>    Discussed with: kmacy
>>    Sponsored by:	Multiplay
> RSS by definition has meaning to received stream. What is "outbound" stream
> in this context, why can the hash calculatiom method change and what exactly
> does it mean "a stream being incorrectly split"?
Yes RSS is indeed a received stream but that is used by lagg for lacp 
and loadbalance protocols to decide which port of the lagg to "send" the 
packet out of. As the flowid is not known when a new "output" stream is 
instigated the current code falls back to manual hash calculation to 
determine which port to send the initial packet from. Once a response is 
received a tx then uses the flowid. This change of hash calculation 
method can result in the initial packet being sent from a different port 
than the rest of the stream; this is what I meant by "incorrectly split".

See the following:
https://github.com/freebsd/freebsd/blob/master/sys/net/if_lagg.c#L2066
https://github.com/freebsd/freebsd/blob/master/sys/net/ieee8023ad_lacp.c#L846

> Defaults should not be changed so easily just because they are not optimal
> for some specific case. Each lagg has its own setting for flowid usage
> and why one cannot just use "ifconfig lagg0 -use_flowid" for such cases?
>
Yes we're already using -use_flowid to mitigate the problem, but the 
defaults should never result in broken behavior hence the change, at 
least for now.

For reference I did look at keeping the default of 1 but only using that 
for protocols which weren't effected by the issue, and introducing a 2 
to force those that are, but as its defined as acting on creation and we 
always create lagg interfaces as failover and then amend them that 
wasn't possible without making more invasive changes.

     Regards
     Steve