[ovs-dev] [PATCH] odp-execute: Reuse rss hash in OVS_ACTION_ATTR_HASH.

[Andy Zhou]

On Tue, Jul 11, 2017 at 7:30 AM, Ilya Maximets <i.maximets at samsung.com> wrote:
> If RSS hash exists in a packet it can be reused instead of
> 5 tuple hash re-calculation in OVS_ACTION_ATTR_HASH. This
> leads to increasing the performance of sending packets to
> the OVS bonding in userspace datapath up to 10-15%.
>
> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
> ---
>  lib/odp-execute.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/lib/odp-execute.c b/lib/odp-execute.c
> index d656334..471a364 100644
> --- a/lib/odp-execute.c
> +++ b/lib/odp-execute.c
> @@ -646,8 +646,17 @@ odp_execute_actions(void *dp, struct dp_packet_batch *batch, bool steal,
>                  uint32_t hash;
>
>                  DP_PACKET_BATCH_FOR_EACH (packet, batch) {
> -                    flow_extract(packet, &flow);
> -                    hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
> +                    /* RSS hash can be used here instead of 5tuple for
> +                     * performance reasons. */
> +                    if (dp_packet_rss_valid(packet)) {
> +                        hash = dp_packet_get_rss_hash(packet);

> +                        if (hash_act->hash_basis) {
> +                            hash = hash_finish(hash, hash_act->hash_basis);
> +                        }

This is not a full review. I have some comments on the 3 lines of code above.

Would it make more sense to always include 'hash_basis'?  Also it
seems hash_int() would be more appropriate than hash_finish().

I suppose the performance gain may not be as significant. On the other
hand, I am not sure if we should count on the performance
gain by assuming hash_basis is always zero.

> +                    } else {
> +                        flow_extract(packet, &flow);
> +                        hash = flow_hash_5tuple(&flow, hash_act->hash_basis);
> +                    }
>                      packet->md.dp_hash = hash;
>                  }
>              } else {
> --
> 2.7.4
>