dpif-netdev: Fix double parsing of packets when EMC disabled.

This partially reverts commit bde94613e6276d48a6e0be7a592ebcf9836b4aaf.

Commit bde94613e627 was aimed to slightly ( < 1%) increase performance
in the case where EMC disabled, but it avoids RSS hash calculation and
OVS has to calculate it while executing OVS_ACTION_ATTR_HASH in order
to handle balanced-tcp bonding. At the time of executing that action
there is no parsed flow, and OVS parses the packet for the second time
to calculate the hash. This happens for all packets received from the
virtual interfaces because they have no HW RSS.

Here is the example of 'perf' output for VM --> (bonded PHY) traffic:

  Samples: 401K of event 'cycles', Event count (approx.): 50964771478
  Overhead  Shared Object       Symbol
    27.50%  ovs-vswitchd        [.] dpcls_lookup.370382
    16.30%  ovs-vswitchd        [.] rte_vhost_dequeue_burst.9267
    14.95%  ovs-vswitchd        [.] miniflow_extract
     7.22%  ovs-vswitchd        [.] flow_extract
     7.10%  ovs-vswitchd        [.] dp_netdev_input__.371002.4826
     4.01%  ovs-vswitchd        [.] fast_path_processing.370987.4893

We can see that packet parsed twice. First time by 'miniflow_extract'
right after receiving and the second time by 'flow_extract' while
executing actions.

In this particular case calculating RSS on receive saves > 7% of the
total CPU processing time. It varies from ~7 to ~10 % depending on
scenario/traffic types.

It's better to calculate hash each time because performance
improvements of avoiding are negligible in compare with performance
drop in case of sending packets to bonded interface.

Another solution could be to pass the parsed flow explicitly through
the datapath, but this will require big code changes and will have
additional overhead for metadata updating on packet changes.

Also, this change should have small impact since SMC works well in most
cases and will be enabled/recommended by default in the future.

CC: Antonio Fischetti <antonio.fischetti@intel.com>
Fixes: bde94613e627 ("dpif-netdev: Avoid reading RSS hash when EMC is disabled.")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Signed-off-by: Ian Stokes <ian.stokes@intel.com>

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 4d6d0c3722368317b00a280835a4f6e090c40381..35cd0071294fe03027320c65f06a75456ea8851d 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -6438,20 +6438,13 @@ dfc_processing(struct dp_netdev_pmd_thread *pmd,
 
         miniflow_extract(packet, &key->mf);
         key->len = 0; /* Not computed yet. */
-        /* If EMC and SMC disabled skip hash computation */
-        if (smc_enable_db == true || cur_min != 0) {
-            if (!md_is_valid) {
-                key->hash = dpif_netdev_packet_get_rss_hash_orig_pkt(packet,
-                        &key->mf);
-            } else {
-                key->hash = dpif_netdev_packet_get_rss_hash(packet, &key->mf);
-            }
-        }
-        if (cur_min) {
-            flow = emc_lookup(&cache->emc_cache, key);
-        } else {
-            flow = NULL;
-        }
+        key->hash =
+                (md_is_valid == false)
+                ? dpif_netdev_packet_get_rss_hash_orig_pkt(packet, &key->mf)
+                : dpif_netdev_packet_get_rss_hash(packet, &key->mf);
+
+        /* If EMC is disabled skip emc_lookup */
+        flow = (cur_min != 0) ? emc_lookup(&cache->emc_cache, key) : NULL;
         if (OVS_LIKELY(flow)) {
             tcp_flags = miniflow_get_tcp_flags(&key->mf);
             n_emc_hit++;