[ovs-dev] [PATCH v10] dpif-netdev: Conditional EMC insert

[Daniele Di Proietto]

2017-02-16 3:01 GMT-08:00 Kevin Traynor <ktraynor at redhat.com>:
> On 02/16/2017 10:22 AM, Ciara Loftus wrote:
>> Unconditional insertion of EMC entries results in EMC thrashing at high
>> numbers of parallel flows. When this occurs, the performance of the EMC
>> often falls below that of the dpcls classifier, rendering the EMC
>> practically useless.
>>
>> Instead of unconditionally inserting entries into the EMC when a miss
>> occurs, use a 1% probability of insertion. This ensures that the most
>> frequent flows have the highest chance of creating an entry in the EMC,
>> and the probability of thrashing the EMC is also greatly reduced.
>>
>> The probability of insertion is configurable, via the
>> other_config:emc-insert-inv-prob option. This value sets the average
>> probability of insertion to 1/emc-insert-inv-prob.
>>
>> For example the following command changes the insertion probability to
>> (on average) 1 in every 20 packets ie. 1/20 ie. 5%.
>>
>> ovs-vsctl set Open_vSwitch . other_config:emc-insert-inv-prob=20
>>
>> Signed-off-by: Ciara Loftus <ciara.loftus at intel.com>
>> Signed-off-by: Georg Schmuecking <georg.schmuecking at ericsson.com>
>> Co-authored-by: Georg Schmuecking <georg.schmuecking at ericsson.com>
>> Acked-by: Kevin Traynor <ktraynor at redhat.com>
>> ---
>> v10:
>> - Fixed typo in commit message
>> - Only store insert_min when value has changed
>> - Add prints to reflect changes in the DB
>
> Thanks for the changes, LGTM.
> Kevin.

Thanks a lot.  I squashed the following incremental to support values
that don't fit
in a signed integer:

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 35d3eda5e..31aee51a2 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2762,7 +2762,9 @@ dpif_netdev_set_config(struct dpif *dpif, const
struct smap *other_config)
 {
     struct dp_netdev *dp = get_dp_netdev(dpif);
     const char *cmask = smap_get(other_config, "pmd-cpu-mask");
-    int insert_prob = smap_get_int(other_config, "emc-insert-inv-prob", -1);
+    unsigned long long insert_prob =
+        smap_get_ullong(other_config, "emc-insert-inv-prob",
+                        DEFAULT_EM_FLOW_INSERT_INV_PROB);
     uint32_t insert_min, cur_min;

     if (!nullable_string_is_equal(dp->pmd_cmask, cmask)) {
@@ -2772,7 +2774,7 @@ dpif_netdev_set_config(struct dpif *dpif, const
struct smap *other_config)
     }

     atomic_read_relaxed(&dp->emc_insert_min, &cur_min);
-    if (insert_prob >= 0 && insert_prob <= UINT32_MAX) {
+    if (insert_prob <= UINT32_MAX) {
         insert_min = insert_prob == 0 ? 0 : UINT32_MAX / insert_prob;
     } else {
         insert_min = DEFAULT_EM_FLOW_INSERT_MIN;
@@ -2784,7 +2786,7 @@ dpif_netdev_set_config(struct dpif *dpif, const
struct smap *other_config)
         if (insert_min == 0) {
             VLOG_INFO("EMC has been disabled");
         } else {
-            VLOG_INFO("EMC insertion probability changed to 1/%i (~%.2f%%)",
+            VLOG_INFO("EMC insertion probability changed to 1/%llu (~%.2f%%)",
                       insert_prob, (100 / (float)insert_prob));
         }
     }

and pushed this to master.