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

[Matt Joras]

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.

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