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

Vishal Deep Ajmera vishal.deep.ajmera at ericsson.com
Thu Jun 18 12:58:21 UTC 2020


>
> 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


More information about the dev mailing list