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

2017-02-16 Thread Daniele Di Proietto
2017-02-16 3:01 GMT-08:00 Kevin Traynor :
> 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 
>> Signed-off-by: Georg Schmuecking 
>> Co-authored-by: Georg Schmuecking 
>> Acked-by: Kevin Traynor 
>> ---
>> 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(>emc_insert_min, _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.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

2017-02-16 Thread Kevin Traynor
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 
> Signed-off-by: Georg Schmuecking 
> Co-authored-by: Georg Schmuecking 
> Acked-by: Kevin Traynor 
> ---
> 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.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev