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(&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.
___
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


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

2017-02-16 Thread Ciara Loftus
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
v9:
- Revert back to original 1/N formula for configuring the probability
  (don't use percentiles).
- Rename option to reflect inverse probability & update documentation to
  make the wording clearer.
v8:
- Floating point precision percentiles
- Moved NEWS entry to post-2.7 section and is no longer in the DPDK
  specific section.
v7:
- Remove further code duplication
v6:
- Refactor the code to remove duplication around calls to
  emc_probabilistic_insert()
v5:
- Use percentiles for emc-insert-prob (0-100%)
- Update docs to reflect the option not exclusive to the DPDK datapath.
v4:
- Added Georg to Authors file
- Set emc-insert-prob=1 for 'PMD - stats' unit test
- Use read_relaxed on atomic var
- Correctly allow for 0 and 100% probababilites
- Cache align new element in dp_netdev struct
- Revert to default probability if invalid emc-insert-prob set
- Allow configurability for non-DPDK case
v3:
- Use new dpif other_config infrastructure to tidy up how the
  emc-insert-prob value is passed to dpif-netdev.
v2:
- Enable probability configurability via other_config:emc-insert-prob
  option.

 AUTHORS.rst  |  1 +
 Documentation/howto/dpdk.rst | 20 
 NEWS |  2 ++
 lib/dpif-netdev.c| 57 
 tests/pmd.at |  1 +
 vswitchd/vswitch.xml | 17 +
 6 files changed, 94 insertions(+), 4 deletions(-)

diff --git a/AUTHORS.rst b/AUTHORS.rst
index f247df5..9a37423 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -385,6 +385,7 @@ Eric Lopez  elo...@nicira.com
 Frido Roose fr.ro...@gmail.com
 Gaetano Catalli gaetano.cata...@gmail.com
 Gavin Remaley   gavin_rema...@selinc.com
+Georg Schmuecking   georg.schmueck...@ericsson.com
 George Shuklin  ama...@desunote.ru
 Gerald Rogers   gerald.rog...@intel.com
 Ghanem Bahribahri.gha...@gmail.com
diff --git a/Documentation/howto/dpdk.rst b/Documentation/howto/dpdk.rst
index d1e6e89..52cb3fc 100644
--- a/Documentation/howto/dpdk.rst
+++ b/Documentation/howto/dpdk.rst
@@ -354,6 +354,26 @@ the `DPDK documentation
 
 Note: Not all DPDK virtual PMD drivers have been tested and verified to work.
 
+EMC Insertion Probability
+-
+By default 1 in every 100 flows are inserted into the Exact Match Cache (EMC).
+It is possible to change this insertion probability by setting the
+``emc-insert-inv-prob`` option::
+
+$ ovs-vsctl --no-wait set Open_vSwitch . other_config:emc-insert-inv-prob=N
+
+where:
+
+``N``
+  is a positive integer representing the inverse probability of insertion ie.
+  on average 1 in every N packets with a unique flow will generate an EMC
+  insertion.
+
+If ``N`` is set to 1, an insertion will be performed for every flow. If set to
+0, no insertions will be performed and the EMC will effectively be disabled.
+
+For more information on the EMC refer to :doc:`/intro/install/dpdk` .
+
 .. _dpdk-ovs-in-guest:
 
 OVS with DPDK Inside VMs
diff --git a/NEWS b/NEWS
index aebd99c..b14c76d 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,8 @@ Post-v2.7.0
- Tunnels:
  * Added support to set packet mark for tunnel endpoint using
`egress_pkt_mark` OVSDB option.
+   - EMC insertion probability is reduced to 1% and is configurable via
+ the new 'other_config:emc-insert-inv-prob' option.
 
 v2.7.0 - xx xxx 
 -
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 0be5db5..35d3eda 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -144,6 +144,11 @@ struct netdev_flow_key {
 #define EM_FLOW_HASH_MASK (EM_FLOW_HASH_ENTRIES - 1)
 #define EM_FLOW_HASH_SEGS 2
 
+/* Default EMC insert probability is 1 / DEFAULT_EM_FLOW_INSERT_INV_PROB */
+#define DEFAU