[ovs-dev] [PATCH] dpif-netdev: Avoid copying netdev_flow_key in emc_processing().

[Andy Zhou]

On Sun, Jan 24, 2016 at 8:32 AM, Ben Pfaff <blp at ovn.org> wrote:

> Before this commit, emc_processing() copied a netdev_flow_key if there was
> no exact-match cache (EMC) hit.  This commit eliminates the copy by
> constructing the netdev_flow_key in the place it would be copied.
>
> Found by inspection.
>
> Shahbaz (CCed) reports that this reduces the cost of an EMC miss by 72
> cycles in his test case in which the EMC is disabled.  Presumably this
> is similarly valuable in cases where the EMC merely has few hits.
>
> CC: Muhammad Shahbaz <mshahbaz at cs.princeton.edu>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
>

Logic looks good to me.

Acked-by: Andy Zhou <azhou at ovn.org>

I think Daniele is testing on performance impact on this patch. so it is
may be worthwhile
to wait for his input before pushing the change.

---
>  AUTHORS           |  1 +
>  lib/dpif-netdev.c | 17 ++++++++---------
>  2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/AUTHORS b/AUTHORS
> index 7ae64d3..420aa2d 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -352,6 +352,7 @@ Mike Kruze              mkruze at nicira.com
>  Min Chen                ustcer.tonychan at gmail.com
>  Mikael Doverhag         mdoverhag at nicira.com
>  Mrinmoy Das             mrdas at ixiacom.com
> +Muhammad Shahbaz        mshahbaz at cs.princeton.edu
>  Murali R                muralirdev at gmail.com
>  Nagi Reddy Jonnala      njonnala at Brocade.com
>  Niels van Adrichem      N.L.M.vanAdrichem at tudelft.nl
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index cd72e62..97c4cde 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
> + * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2016 Nicira, Inc.
>   *
>   * Licensed under the Apache License, Version 2.0 (the "License");
>   * you may not use this file except in compliance with the License.
> @@ -3210,7 +3210,6 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
> struct dp_packet **packets,
>                 struct packet_batch batches[], size_t *n_batches)
>  {
>      struct emc_cache *flow_cache = &pmd->flow_cache;
> -    struct netdev_flow_key key;
>      size_t i, notfound_cnt = 0;
>
>      for (i = 0; i < cnt; i++) {
> @@ -3226,20 +3225,20 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
> struct dp_packet **packets,
>              OVS_PREFETCH(dp_packet_data(packets[i+1]));
>          }
>
> -        miniflow_extract(packets[i], &key.mf);
> -        key.len = 0; /* Not computed yet. */
> -        key.hash = dpif_netdev_packet_get_rss_hash(packets[i], &key.mf);
> +        struct netdev_flow_key *key = &keys[notfound_cnt];
> +        miniflow_extract(packets[i], &key->mf);
> +        key->len = 0; /* Not computed yet. */
> +        key->hash = dpif_netdev_packet_get_rss_hash(packets[i], &key->mf);
>
> -        flow = emc_lookup(flow_cache, &key);
> +        flow = emc_lookup(flow_cache, key);
>          if (OVS_LIKELY(flow)) {
> -            dp_netdev_queue_batches(packets[i], flow, &key.mf, batches,
> +            dp_netdev_queue_batches(packets[i], flow, &key->mf, batches,
>                                      n_batches);
>          } else {
>              if (i != notfound_cnt) {
>                  dp_packet_swap(&packets[i], &packets[notfound_cnt]);
>              }
> -
> -            keys[notfound_cnt++] = key;
> +            notfound_cnt++;
>          }
>      }
>
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>