[ovs-dev] [PATCH v13 1/1] Avoid dp_hash recirculation for balance-tcp bond selection mode

[Ilya Maximets]

On 6/18/20 2:58 PM, Vishal Deep Ajmera wrote:
>>
>> Hi.  Thanks for the new version it works fine.  In order to speed things
>> up I prepared an incremental patch with one bug fix and some changes
>> that I think are necessary to keep the code clean.  Don't be afraid of
>> the patch size, there are very few functional changes, most of other
>> changes are style fixes or readability improvements.
>>
>> This patch + my incremental was reviewed by Eelco and tested by Adrian.
>> I'll add proper tags while applying to master.
>>
>> So, what I'm proposing for you is to take a look at the incremental patch
>> and if it's OK for you, I could squash it into your v13 and apply to master.
>>
>> What do you think?
>>
>> Best regards, Ilya Maximets.
>>
>> Incremental patch itself:
>> ------------------------------
>> Summary:
>>   * Dropped 'dp->bond_mutex' while removing stale ports.  We're not
>>     touching bonds there at all.
>>   * Added new argument 'update' to dp_netdev_add_bond_tx_to_pmd() to
>>     avoid unnecessary re-allocation of the same slaves during
>>     reconfiguration.  We might also want to avoid adding tx bonds to
>>     PMDs that doesn't poll anything inside dpif_netdev_bond_add(), but
>>     this might be done later.
>>   * Added actual error codes to dpif_netdev_bond_*() functions.
>>     They are not used right now, but it's better to have them anyway.
>>   * Refined API around dpif_bond_stats_get().  Now it always clears
>>     n_bytes array regardless of success.  Memory clearing removed from
>>     the caller.
>>   * dpif_bond_*() functions should not check dpif and dpif_class
>>     pointers.  Rewritten to be more like other code.  Error codes
>>     must be positive and we're usually using EOPNOTSUPP instead of
>>     ENOTSUP for unsupported operations.
>>   * bond/show output changed to reflect actual values, not the
>>     configured ones.  Dropped printing of recirc_id if recirc_id is not
>>     supported (actually if lb_output is not supported).
>>   * Fixed wrong array size in bond_add_lb_output_buckets().
>>     MASK mistakenly used as an array size.
>>   * Reorganized compose_output_action__() to make it more readable
>>     and not fetch hash algo if not needed. (incremental diff is big,
>>     but it mostly restores previous code :) )
>>   * Removed is_lb_output_action_supported from the ofproto-provider
>>     API.  This was not a pretty solution.  Replaced with new
>>     ovs_lb_output_action_supported() function called from the
>>     ofproto/bond.c.  For this purpose validation of lb-output-action
>>     knob moved down to bond_reconfigure() out of bridge.c.
>>   * 'lb-output-action' replaced with 'lb_output action' in docs and
>>     comments and in the output of bond/show to make things more
>>     readable and easier to understand.
>>   * Tabs in output of bond/show command replaced with spaces.
>>     OVS doesn't use tabs in the output anywhere.
>>   * Random style fixes, improvements.
>>
> Hi Ilya,
> 
> The changes look ok to me.
> 
> Warm Regards,
> Vishal Ajmera
> 

That was a long journey, but I finally applied this patch to master.

Thanks, everyone!

Best regards, Ilya Maximets.