| Message ID | 1489340008-77897-1-git-send-email-bhanuprakash.bodireddy@intel.com |
|---|---|
| State | Accepted |
| Delegated to: | Darrell Ball |
| Headers | show |
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 <bhanuprakash.bodireddy@intel.com> > CC: Ciara Loftus <ciara.loftus@intel.com> > CC: Georg Schmuecking <georg.schmuecking@ericsson.com> > --- > 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(&pmd->dp->emc_insert_min, &cur_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, &key->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, &key->mf, batches, > n_batches); >
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 <ktraynor@redhat.com> >> Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> >> CC: Ciara Loftus <ciara.loftus@intel.com> >> CC: Georg Schmuecking <georg.schmuecking@ericsson.com> >> --- >> 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(&pmd->dp->emc_insert_min, &cur_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, &key->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, &key->mf, batches, >> n_batches); >> > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
>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 <ktraynor@redhat.com> Thanks kevin for the review and Acks. Regards, Bhanuprakash.
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 <ktraynor@redhat.com> > > Thanks kevin for the review and Acks. > > Regards, > Bhanuprakash. >
I found 4-5% throughput improvement when emc is disabled and it is a simple change Thanks for adding this. Acked-by: Darrell Ball dlu998@gmail.com On 4/13/17, 11:30 AM, "ovs-dev-bounces@openvswitch.org on behalf of Bodireddy, Bhanuprakash" <ovs-dev-bounces@openvswitch.org on behalf of bhanuprakash.bodireddy@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 <ktraynor@redhat.com> Thanks kevin for the review and Acks. Regards, Bhanuprakash. _______________________________________________ dev mailing list dev@openvswitch.org https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=0nx_fadlj6m2KxOZGCsMJYGW9jUMnpXfb-sE5Aesw54&s=IeltQjLxliaHPElGHcd0mWdY7hRUJqbAEspM8CMkyco&e=
Thanks Bhanuprakash, Kevin, Darrell. I applied this patch to master. On Wed, May 17, 2017 at 02:49:53AM +0000, 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 dlu998@gmail.com > > > > On 4/13/17, 11:30 AM, "ovs-dev-bounces@openvswitch.org on behalf of Bodireddy, Bhanuprakash" <ovs-dev-bounces@openvswitch.org on behalf of bhanuprakash.bodireddy@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 <ktraynor@redhat.com> > > Thanks kevin for the review and Acks. > > Regards, > Bhanuprakash. > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=0nx_fadlj6m2KxOZGCsMJYGW9jUMnpXfb-sE5Aesw54&s=IeltQjLxliaHPElGHcd0mWdY7hRUJqbAEspM8CMkyco&e= > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
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(&pmd->dp->emc_insert_min, &cur_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, &key->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, &key->mf, batches, n_batches);
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 <bhanuprakash.bodireddy@intel.com> CC: Ciara Loftus <ciara.loftus@intel.com> CC: Georg Schmuecking <georg.schmuecking@ericsson.com> --- lib/dpif-netdev.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)