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

[Steven Hartland]

On 05/01/2018 17:39, Matt Joras wrote:
> On Fri, Jan 5, 2018 at 9:32 AM, Eugene Grosbein<eugen at grosbein.net>  wrote:
>> 06.01.2018 0:28, Matt Joras wrote:
>>
>>> For what it's worth, this was the conclusion I came to, and at Isilon
>>> we've made the same change being discussed here. For the case of
>>> drivers that end up using a queue index for the flowid, you end up
>>> with pathological behavior on the lagg; the flowid ends up getting
>>> right shifted by (default) 16. So in the case of e.g. two bxe(4)
>>> interfaces with 4 queues, you always end up choosing the interface in
>>> the lagg at index 0.
>> Not all drivers have this bug. These are drivers that needs to be fixed to not shift by 16, not lagg.
>>
> I don't follow. It is if_lagg that does the shifting. For loadbalance
> it is done directly in lagg_snd_tag_alloc, and for LACP it is done in
> a separate fucntion, lacp_select_tx_port_by_hash. For both it shifts
> the flowid by the flowid_shift set on the lagg sc, which defaults to
> 16.
For reference lacp_select_tx_port is the normal method, 
lacp_select_tx_port_by_hash is only used if RATELIMIT is enabled. They 
both do the same shift though, so ...
> You could make the argument that we should fix every driver that sets
> a queue index to instead use an RSS hash, but that seems like more
> work than simply disabling the use of flowid in if_lagg by default.
> For cases where this has an appreciable impact on forwarding
> performance the sysctl can be flipped back. That seems more reasonable
> to me than making laggs effectively useless for anyone using any one
> of a random set of drivers that set the flowid to a queue index (grep
> for "flowid =" and you can see which drivers do this).
>
> Matt