Re: [ovs-dev] [PATCH 1/5] dpif-netdev: Skip EMC lookup when EMC is disabled.

[Ben Pfaff]

Thanks Bhanuprakash, Kevin, Darrell.  I applied this patch to master.

On Wed, May 17, 2017 at 02:49:53AM +, Darrell Ball wrote:
> I found 4-5% throughput improvement when emc is disabled and it is a simple 
> change
> Thanks for adding this.
> 
> Acked-by: Darrell Ball dlu...@gmail.com
> 
> 
> 
> On 4/13/17, 11:30 AM, "ovs-dev-boun...@openvswitch.org on behalf of 
> Bodireddy, Bhanuprakash"  bhanuprakash.bodire...@intel.com> wrote:
> 
> >On 04/13/2017 07:11 PM, Kevin Traynor wrote:
> >> On 03/12/2017 05:33 PM, Bhanuprakash Bodireddy wrote:
> >>> Conditional EMC insert patch gives the flexibility to configure the
> >>> probability of flow insertion in to EMC. This also allows an option
> >>> to entirely disable EMC by setting 'emc-insert-inv-prob=0' which can
> >>> be useful at large number of parallel flows.
> >>>
> >>> This patch skips EMC lookup when EMC is disabled. This is useful to
> >>> avoid wasting CPU cycles and also improve performance considerably.
> >>>
> >>
> >> LGTM. How much does this improve performance?
> 
> I found  significant performance improvement when testing with few 
> hundred streams.  I remember the improvement was  ~800kpps  with smaller 
> packets. This is for the reason that emc_lookup() invokes expensive memcmp() 
> to compare the netdev_flow_key in EMC and it takes up significant cycles. 
> Longer the 'key', worse the performance.  
> 
> >Ack for the series,
> >Acked-by: Kevin Traynor 
> 
> Thanks kevin for the review and Acks.
> 
> Regards,
> Bhanuprakash. 
> ___
> dev mailing list
> d...@openvswitch.org
> 
> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=0nx_fadlj6m2KxOZGCsMJYGW9jUMnpXfb-sE5Aesw54=IeltQjLxliaHPElGHcd0mWdY7hRUJqbAEspM8CMkyco=
>  
> 
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/5] dpif-netdev: Skip EMC lookup when EMC is disabled.

[Darrell Ball]

I found 4-5% throughput improvement when emc is disabled and it is a simple 
change
Thanks for adding this.

Acked-by: Darrell Ball dlu...@gmail.com



On 4/13/17, 11:30 AM, "ovs-dev-boun...@openvswitch.org on behalf of Bodireddy, 
Bhanuprakash"  wrote:

>On 04/13/2017 07:11 PM, Kevin Traynor wrote:
>> On 03/12/2017 05:33 PM, Bhanuprakash Bodireddy wrote:
>>> Conditional EMC insert patch gives the flexibility to configure the
>>> probability of flow insertion in to EMC. This also allows an option
>>> to entirely disable EMC by setting 'emc-insert-inv-prob=0' which can
>>> be useful at large number of parallel flows.
>>>
>>> This patch skips EMC lookup when EMC is disabled. This is useful to
>>> avoid wasting CPU cycles and also improve performance considerably.
>>>
>>
>> LGTM. How much does this improve performance?

I found  significant performance improvement when testing with few hundred 
streams.  I remember the improvement was  ~800kpps  with smaller packets. This 
is for the reason that emc_lookup() invokes expensive memcmp() to compare the 
netdev_flow_key in EMC and it takes up significant cycles. Longer the 'key', 
worse the performance.  

>Ack for the series,
>Acked-by: Kevin Traynor 

Thanks kevin for the review and Acks.

Regards,
Bhanuprakash. 
___
dev mailing list
d...@openvswitch.org

https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev=DwICAg=uilaK90D4TOVoH58JNXRgQ=BVhFA09CGX7JQ5Ih-uZnsw=0nx_fadlj6m2KxOZGCsMJYGW9jUMnpXfb-sE5Aesw54=IeltQjLxliaHPElGHcd0mWdY7hRUJqbAEspM8CMkyco=
 


___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/5] dpif-netdev: Skip EMC lookup when EMC is disabled.

[Kevin Traynor]

On 04/13/2017 07:30 PM, Bodireddy, Bhanuprakash wrote:
>> On 04/13/2017 07:11 PM, Kevin Traynor wrote:
>>> On 03/12/2017 05:33 PM, Bhanuprakash Bodireddy wrote:
 Conditional EMC insert patch gives the flexibility to configure the
 probability of flow insertion in to EMC. This also allows an option
 to entirely disable EMC by setting 'emc-insert-inv-prob=0' which can
 be useful at large number of parallel flows.

 This patch skips EMC lookup when EMC is disabled. This is useful to
 avoid wasting CPU cycles and also improve performance considerably.

>>>
>>> LGTM. How much does this improve performance?
> 
> I found  significant performance improvement when testing with few hundred 
> streams.  I remember the improvement was  ~800kpps  with smaller packets. 
> This is for the reason that emc_lookup() invokes expensive memcmp() to 
> compare the netdev_flow_key in EMC and it takes up significant cycles. Longer 
> the 'key', worse the performance.  
> 

I just tested and saw about the same improvement. The memcmp won't
happen if emc-insert-inv-prob=0 but you would still check the EMC for
the key, so it's nice to avoid that.

thanks,
Kevin.


>> Ack for the series,
>> Acked-by: Kevin Traynor 
> 
> Thanks kevin for the review and Acks.
> 
> Regards,
> Bhanuprakash. 
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/5] dpif-netdev: Skip EMC lookup when EMC is disabled.

[Bodireddy, Bhanuprakash]

>On 04/13/2017 07:11 PM, Kevin Traynor wrote:
>> On 03/12/2017 05:33 PM, Bhanuprakash Bodireddy wrote:
>>> Conditional EMC insert patch gives the flexibility to configure the
>>> probability of flow insertion in to EMC. This also allows an option
>>> to entirely disable EMC by setting 'emc-insert-inv-prob=0' which can
>>> be useful at large number of parallel flows.
>>>
>>> This patch skips EMC lookup when EMC is disabled. This is useful to
>>> avoid wasting CPU cycles and also improve performance considerably.
>>>
>>
>> LGTM. How much does this improve performance?

I found  significant performance improvement when testing with few hundred 
streams.  I remember the improvement was  ~800kpps  with smaller packets. This 
is for the reason that emc_lookup() invokes expensive memcmp() to compare the 
netdev_flow_key in EMC and it takes up significant cycles. Longer the 'key', 
worse the performance.  

>Ack for the series,
>Acked-by: Kevin Traynor 

Thanks kevin for the review and Acks.

Regards,
Bhanuprakash. 
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/5] dpif-netdev: Skip EMC lookup when EMC is disabled.

[Kevin Traynor]

On 04/13/2017 07:11 PM, Kevin Traynor wrote:
> On 03/12/2017 05:33 PM, Bhanuprakash Bodireddy wrote:
>> Conditional EMC insert patch gives the flexibility to configure the
>> probability of flow insertion in to EMC. This also allows an option to
>> entirely disable EMC by setting 'emc-insert-inv-prob=0' which can be
>> useful at large number of parallel flows.
>>
>> This patch skips EMC lookup when EMC is disabled. This is useful to
>> avoid wasting CPU cycles and also improve performance considerably.
>>
> 
> LGTM. How much does this improve performance?
> 

Ack for the series,
Acked-by: Kevin Traynor 

>> Signed-off-by: Bhanuprakash Bodireddy 
>> CC: Ciara Loftus 
>> CC: Georg Schmuecking 
>> ---
>>  lib/dpif-netdev.c | 6 +-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
>> index 7d53a8d..faadedb 100644
>> --- a/lib/dpif-netdev.c
>> +++ b/lib/dpif-netdev.c
>> @@ -4517,8 +4517,11 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
>>  size_t n_missed = 0, n_dropped = 0;
>>  struct dp_packet *packet;
>>  const size_t size = dp_packet_batch_size(packets_);
>> +uint32_t cur_min;
>>  int i;
>>  
>> +atomic_read_relaxed(>dp->emc_insert_min, _min);
>> +
>>  DP_PACKET_BATCH_REFILL_FOR_EACH (i, size, packet, packets_) {
>>  struct dp_netdev_flow *flow;
>>  
>> @@ -4542,7 +4545,8 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
>>  key->len = 0; /* Not computed yet. */
>>  key->hash = dpif_netdev_packet_get_rss_hash(packet, >mf);
>>  
>> -flow = emc_lookup(flow_cache, key);
>> +/* If EMC is disabled skip emc_lookup */
>> +flow = (cur_min == 0) ? NULL: emc_lookup(flow_cache, key);
>>  if (OVS_LIKELY(flow)) {
>>  dp_netdev_queue_batches(packet, flow, >mf, batches,
>>  n_batches);
>>
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH 1/5] dpif-netdev: Skip EMC lookup when EMC is disabled.

[Kevin Traynor]

On 03/12/2017 05:33 PM, Bhanuprakash Bodireddy wrote:
> Conditional EMC insert patch gives the flexibility to configure the
> probability of flow insertion in to EMC. This also allows an option to
> entirely disable EMC by setting 'emc-insert-inv-prob=0' which can be
> useful at large number of parallel flows.
> 
> This patch skips EMC lookup when EMC is disabled. This is useful to
> avoid wasting CPU cycles and also improve performance considerably.
> 

LGTM. How much does this improve performance?

> Signed-off-by: Bhanuprakash Bodireddy 
> CC: Ciara Loftus 
> CC: Georg Schmuecking 
> ---
>  lib/dpif-netdev.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 7d53a8d..faadedb 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -4517,8 +4517,11 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
>  size_t n_missed = 0, n_dropped = 0;
>  struct dp_packet *packet;
>  const size_t size = dp_packet_batch_size(packets_);
> +uint32_t cur_min;
>  int i;
>  
> +atomic_read_relaxed(>dp->emc_insert_min, _min);
> +
>  DP_PACKET_BATCH_REFILL_FOR_EACH (i, size, packet, packets_) {
>  struct dp_netdev_flow *flow;
>  
> @@ -4542,7 +4545,8 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
>  key->len = 0; /* Not computed yet. */
>  key->hash = dpif_netdev_packet_get_rss_hash(packet, >mf);
>  
> -flow = emc_lookup(flow_cache, key);
> +/* If EMC is disabled skip emc_lookup */
> +flow = (cur_min == 0) ? NULL: emc_lookup(flow_cache, key);
>  if (OVS_LIKELY(flow)) {
>  dp_netdev_queue_batches(packet, flow, >mf, batches,
>  n_batches);
> 

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH 1/5] dpif-netdev: Skip EMC lookup when EMC is disabled.

[Bhanuprakash Bodireddy]

Conditional EMC insert patch gives the flexibility to configure the
probability of flow insertion in to EMC. This also allows an option to
entirely disable EMC by setting 'emc-insert-inv-prob=0' which can be
useful at large number of parallel flows.

This patch skips EMC lookup when EMC is disabled. This is useful to
avoid wasting CPU cycles and also improve performance considerably.

Signed-off-by: Bhanuprakash Bodireddy 
CC: Ciara Loftus 
CC: Georg Schmuecking 
---
 lib/dpif-netdev.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 7d53a8d..faadedb 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -4517,8 +4517,11 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
 size_t n_missed = 0, n_dropped = 0;
 struct dp_packet *packet;
 const size_t size = dp_packet_batch_size(packets_);
+uint32_t cur_min;
 int i;
 
+atomic_read_relaxed(>dp->emc_insert_min, _min);
+
 DP_PACKET_BATCH_REFILL_FOR_EACH (i, size, packet, packets_) {
 struct dp_netdev_flow *flow;
 
@@ -4542,7 +4545,8 @@ emc_processing(struct dp_netdev_pmd_thread *pmd,
 key->len = 0; /* Not computed yet. */
 key->hash = dpif_netdev_packet_get_rss_hash(packet, >mf);
 
-flow = emc_lookup(flow_cache, key);
+/* If EMC is disabled skip emc_lookup */
+flow = (cur_min == 0) ? NULL: emc_lookup(flow_cache, key);
 if (OVS_LIKELY(flow)) {
 dp_netdev_queue_batches(packet, flow, >mf, batches,
 n_batches);
-- 
2.4.11

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev