diff mbox

[ovs-dev,2/2] dpif-netdev: Avoid copying netdev_flow_key in emc_processing()

Message ID 1454123675-13245-2-git-send-email-azhou@ovn.org
State Superseded
Headers show

Commit Message

Andy Zhou Jan. 30, 2016, 3:14 a.m. UTC
This is essentially the same patch as the original patched posted at:
http://openvswitch.org/pipermail/dev/2016-January/064971.html

The original commit message:
    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.

However, the original patch introduced a slight performance drop for the
fast path, where every packets hits the emc cache.  This patch removes
the performance drop by only reloading the key variable when a miss
happens.

CC: Muhammad Shahbaz <mshahbaz@cs.princeton.edu>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Sgned-off-by: Andy Zhou <azhou@ovn.org>
---
 lib/dpif-netdev.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

Comments

Ben Pfaff Feb. 1, 2016, 7:15 p.m. UTC | #1
On Fri, Jan 29, 2016 at 07:14:35PM -0800, Andy Zhou wrote:
> This is essentially the same patch as the original patched posted at:
> http://openvswitch.org/pipermail/dev/2016-January/064971.html
> 
> The original commit message:
>     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.
> 
> However, the original patch introduced a slight performance drop for the
> fast path, where every packets hits the emc cache.  This patch removes
> the performance drop by only reloading the key variable when a miss
> happens.
> 
> CC: Muhammad Shahbaz <mshahbaz@cs.princeton.edu>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> Sgned-off-by: Andy Zhou <azhou@ovn.org>

Oops, I applied my version of this after getting your and Daniele's
acks.  You'll probably have to respin this series, sorry.
Andy Zhou Feb. 1, 2016, 8:56 p.m. UTC | #2
On Mon, Feb 1, 2016 at 11:15 AM, Ben Pfaff <blp@ovn.org> wrote:
> On Fri, Jan 29, 2016 at 07:14:35PM -0800, Andy Zhou wrote:
>> This is essentially the same patch as the original patched posted at:
>> http://openvswitch.org/pipermail/dev/2016-January/064971.html
>>
>> The original commit message:
>>     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.
>>
>> However, the original patch introduced a slight performance drop for the
>> fast path, where every packets hits the emc cache.  This patch removes
>> the performance drop by only reloading the key variable when a miss
>> happens.
>>
>> CC: Muhammad Shahbaz <mshahbaz@cs.princeton.edu>
>> Signed-off-by: Ben Pfaff <blp@ovn.org>
>> Sgned-off-by: Andy Zhou <azhou@ovn.org>
>
> Oops, I applied my version of this after getting your and Daniele's
> acks.  You'll probably have to respin this series, sorry.

Sure. I rebased and posted a V2.
diff mbox

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 205176a..94fa5ad 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -3297,7 +3297,7 @@  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;
+    struct netdev_flow_key *key = &keys[0];
     size_t i, n_missed = 0, n_dropped = 0;
 
     for (i = 0; i < cnt; i++) {
@@ -3315,19 +3315,19 @@  emc_processing(struct dp_netdev_pmd_thread *pmd, struct dp_packet **packets,
             OVS_PREFETCH(dp_packet_data(packets[i+1]));
         }
 
-        miniflow_extract(packet, &key.mf);
-        key.len = 0; /* Not computed yet. */
-        key.hash = dpif_netdev_packet_get_rss_hash(packet, &key.mf);
+        miniflow_extract(packet, &key->mf);
+        key->len = 0; /* Not computed yet. */
+        key->hash = dpif_netdev_packet_get_rss_hash(packet, &key->mf);
 
-        flow = emc_lookup(flow_cache, &key);
+        flow = emc_lookup(flow_cache, key);
         if (OVS_LIKELY(flow)) {
-            dp_netdev_queue_batches(packet, flow, &key.mf, batches,
+            dp_netdev_queue_batches(packet, flow, &key->mf, batches,
                                     n_batches);
         } else {
             /* Exact match cache missed. Group missed packets together at
              * the beginning of the 'packets' array.  */
-            packets[n_missed] = packet;
-            keys[n_missed++] = key;
+            packets[n_missed++] = packet;
+            key = &keys[n_missed];
         }
     }