diff mbox series

[ovs-dev] userspace: Switch default cache from EMC to SMC.

Message ID 20200919130743.550736-1-fbl@sysclose.org
State Changes Requested
Headers show
Series [ovs-dev] userspace: Switch default cache from EMC to SMC. | expand

Commit Message

Flavio Leitner Sept. 19, 2020, 1:07 p.m. UTC
The EMC is not large enough for current production cases
and they are scaling up, so this change switches over from
EMC to SMC by default, which provides better results.

The EMC is still available and could be used when only a
few number of flows is used.

Signed-off-by: Flavio Leitner <fbl@sysclose.org>
---
 Documentation/topics/dpdk/bridge.rst | 10 +++++-----
 NEWS                                 |  3 +++
 lib/dpif-netdev.c                    |  6 +++---
 tests/pmd.at                         |  4 ++--
 vswitchd/vswitch.xml                 |  4 ++--
 5 files changed, 15 insertions(+), 12 deletions(-)

Comments

Ilya Maximets Sept. 22, 2020, 11:22 a.m. UTC | #1
On 9/19/20 3:07 PM, Flavio Leitner wrote:
> The EMC is not large enough for current production cases
> and they are scaling up, so this change switches over from
> EMC to SMC by default, which provides better results.
> 
> The EMC is still available and could be used when only a
> few number of flows is used.
> 

Hi, Flavio.  Thanks for the patch!

I generally agree with this change.  The main thing that bothers me is that
with current version of the patch we will drop all testing on EMC at once.

Right now EMC is enabled by default, so we have it tested in many cases.
We also have probability configuration covered in one test.  And we have
at least one test with SMC enabled.  We doesn't check SMC hits, but at least
insertions should work. :)

After this patch we will have zero tests with EMC enabled.  And that is not
a good thing.  In general, I think that we need much more tests for both
SMC and EMC, but at least couple of tests needed for EMC right now since
you're disabling it by default.  And we definitely need at least one test
for emc-insert-inv-prob configuration.

One more thing is that our DPCLS implementation could actually be faster
than SMC in some cases [1], so, I think, maybe it worth performing a series
of performance tests to better understand what we actually want.  It may
turn out that we want to disable all the caches by default and just have
datapath classifier to handle all the traffic since it's more flexible and
even faster in common cases.

[1] https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360586.html

Few minor comments inline.

Best regards, Ilya Maximets.

> Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> ---
>  Documentation/topics/dpdk/bridge.rst | 10 +++++-----
>  NEWS                                 |  3 +++
>  lib/dpif-netdev.c                    |  6 +++---
>  tests/pmd.at                         |  4 ++--
>  vswitchd/vswitch.xml                 |  4 ++--
>  5 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst
> index 526d5c959..0d9aed4eb 100644
> --- a/Documentation/topics/dpdk/bridge.rst
> +++ b/Documentation/topics/dpdk/bridge.rst
> @@ -130,13 +130,13 @@ SMC cache
>  SMC cache or signature match cache is a new cache level after EMC cache.
>  The difference between SMC and EMC is SMC only stores a signature of a flow
>  thus it is much more memory efficient. With same memory space, EMC can store 8k
> -flows while SMC can store 1M flows. When traffic flow count is much larger than
> -EMC size, it is generally beneficial to turn off EMC and turn on SMC. It is
> -currently turned off by default.
> +flows while SMC can store 1M flows. When traffic flow count is small than
> +EMC size, it is generally beneficial to turn off SMC and turn on EMC. It is
> +currently turned on by default.
>  
> -To turn on SMC::
> +To turn off SMC::
>  
> -    $ ovs-vsctl --no-wait set Open_vSwitch . other_config:smc-enable=true
> +    $ ovs-vsctl --no-wait set Open_vSwitch . other_config:smc-enable=false
>  
>  Datapath Classifier Performance
>  -------------------------------
> diff --git a/NEWS b/NEWS
> index 2f67d5047..5b88937fb 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,6 +1,9 @@
>  Post-v2.14.0
>  ---------------------
>  

Usually, we do not have an empty line here.

> +   - DPDK:

This should say "Userspace datapath:" instead, as this is a generic change
for all port types.

> +     * The SMC cache is enabled by default.
> +     * The EMC cache is disabled by default.
>  
>  v2.14.0 - 17 Aug 2020
>  ---------------------
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 02df8f11e..0f2bb10fd 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2079,7 +2079,7 @@ port_create(const char *devname, const char *type,
>      port->netdev = netdev;
>      port->type = xstrdup(type);
>      port->sf = NULL;
> -    port->emc_enabled = true;
> +    port->emc_enabled = false;
>      port->need_reconfigure = true;
>      ovs_mutex_init(&port->txq_used_mutex);
>  
> @@ -4305,7 +4305,7 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
>          }
>      }
>  
> -    bool smc_enable = smap_get_bool(other_config, "smc-enable", false);
> +    bool smc_enable = smap_get_bool(other_config, "smc-enable", true);
>      bool cur_smc;
>      atomic_read_relaxed(&dp->smc_enable_db, &cur_smc);
>      if (smc_enable != cur_smc) {
> @@ -4440,7 +4440,7 @@ dpif_netdev_port_set_config(struct dpif *dpif, odp_port_t port_no,
>      struct dp_netdev_port *port;
>      int error = 0;
>      const char *affinity_list = smap_get(cfg, "pmd-rxq-affinity");
> -    bool emc_enabled = smap_get_bool(cfg, "emc-enable", true);
> +    bool emc_enabled = smap_get_bool(cfg, "emc-enable", false);
>  
>      ovs_mutex_lock(&dp->port_mutex);
>      error = get_port_by_number(dp, port_no, &port);
> diff --git a/tests/pmd.at b/tests/pmd.at
> index 5b612f88f..20c179b39 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -238,8 +238,8 @@ pmd thread numa_id <cleared> core_id <cleared>:
>    packets received: 20
>    packet recirculations: 0
>    avg. datapath passes per packet: 1.00
> -  emc hits: 19
> -  smc hits: 0
> +  emc hits: 0
> +  smc hits: 19
>    megaflow hits: 0
>    avg. subtable lookups per megaflow hit: 0.00
>    miss with success upcall: 1
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 81c84927f..009f0e550 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -513,7 +513,7 @@
>            when flow count is larger than EMC capacity.
>          </p>
>          <p>
> -          Defaults to false but can be changed at any time.
> +          Defaults to true but can be changed at any time.
>          </p>
>        </column>
>  
> @@ -3296,7 +3296,7 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
>            key="emc-insert-inv-prob"/> will have effect on this interface.
>          </p>
>          <p>
> -          Defaults to true.
> +          Defaults to false.
>          </p>
>        </column>
>      </group>
>
Flavio Leitner Sept. 22, 2020, 12:38 p.m. UTC | #2
Hi Ilya,

On Tue, Sep 22, 2020 at 01:22:58PM +0200, Ilya Maximets wrote:
> On 9/19/20 3:07 PM, Flavio Leitner wrote:
> > The EMC is not large enough for current production cases
> > and they are scaling up, so this change switches over from
> > EMC to SMC by default, which provides better results.
> > 
> > The EMC is still available and could be used when only a
> > few number of flows is used.
> > 
> 
> Hi, Flavio.  Thanks for the patch!
> 
> I generally agree with this change.

Great!


> The main thing that bothers me is that
> with current version of the patch we will drop all testing on EMC at once.
>
> Right now EMC is enabled by default, so we have it tested in many cases.
> We also have probability configuration covered in one test.  And we have
> at least one test with SMC enabled.  We doesn't check SMC hits, but at least
> insertions should work. :)
> 
> After this patch we will have zero tests with EMC enabled.  And that is not
> a good thing.  In general, I think that we need much more tests for both
> SMC and EMC, but at least couple of tests needed for EMC right now since
> you're disabling it by default.  And we definitely need at least one test
> for emc-insert-inv-prob configuration.

This patch should enable more testing, indeed.


> One more thing is that our DPCLS implementation could actually be faster
> than SMC in some cases [1], so, I think, maybe it worth performing a series
> of performance tests to better understand what we actually want.  It may
> turn out that we want to disable all the caches by default and just have
> datapath classifier to handle all the traffic since it's more flexible and
> even faster in common cases.
> 
> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2019-July/360586.html


Yes, but AFAICT the last proposed patch is from mid 2019 while SMC
is already merged since 2.10.  I mean, it's great that we have
further optimizations that might not even require caching, but not
sure if such work will be ready for 2.15.

> Few minor comments inline.

ACK to below changes.

I am curious to find out what others think about this change, so
going to wait a bit before following up with the next version if
that sounds OK.

Thanks for your review Ilya,
fbl

> 
> Best regards, Ilya Maximets.
> 
> > Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> > ---
> >  Documentation/topics/dpdk/bridge.rst | 10 +++++-----
> >  NEWS                                 |  3 +++
> >  lib/dpif-netdev.c                    |  6 +++---
> >  tests/pmd.at                         |  4 ++--
> >  vswitchd/vswitch.xml                 |  4 ++--
> >  5 files changed, 15 insertions(+), 12 deletions(-)
> > 
> > diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst
> > index 526d5c959..0d9aed4eb 100644
> > --- a/Documentation/topics/dpdk/bridge.rst
> > +++ b/Documentation/topics/dpdk/bridge.rst
> > @@ -130,13 +130,13 @@ SMC cache
> >  SMC cache or signature match cache is a new cache level after EMC cache.
> >  The difference between SMC and EMC is SMC only stores a signature of a flow
> >  thus it is much more memory efficient. With same memory space, EMC can store 8k
> > -flows while SMC can store 1M flows. When traffic flow count is much larger than
> > -EMC size, it is generally beneficial to turn off EMC and turn on SMC. It is
> > -currently turned off by default.
> > +flows while SMC can store 1M flows. When traffic flow count is small than
> > +EMC size, it is generally beneficial to turn off SMC and turn on EMC. It is
> > +currently turned on by default.
> >  
> > -To turn on SMC::
> > +To turn off SMC::
> >  
> > -    $ ovs-vsctl --no-wait set Open_vSwitch . other_config:smc-enable=true
> > +    $ ovs-vsctl --no-wait set Open_vSwitch . other_config:smc-enable=false
> >  
> >  Datapath Classifier Performance
> >  -------------------------------
> > diff --git a/NEWS b/NEWS
> > index 2f67d5047..5b88937fb 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,6 +1,9 @@
> >  Post-v2.14.0
> >  ---------------------
> >  
> 
> Usually, we do not have an empty line here.
> 
> > +   - DPDK:
> 
> This should say "Userspace datapath:" instead, as this is a generic change
> for all port types.
> 
> > +     * The SMC cache is enabled by default.
> > +     * The EMC cache is disabled by default.
> >  
> >  v2.14.0 - 17 Aug 2020
> >  ---------------------
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 02df8f11e..0f2bb10fd 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -2079,7 +2079,7 @@ port_create(const char *devname, const char *type,
> >      port->netdev = netdev;
> >      port->type = xstrdup(type);
> >      port->sf = NULL;
> > -    port->emc_enabled = true;
> > +    port->emc_enabled = false;
> >      port->need_reconfigure = true;
> >      ovs_mutex_init(&port->txq_used_mutex);
> >  
> > @@ -4305,7 +4305,7 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
> >          }
> >      }
> >  
> > -    bool smc_enable = smap_get_bool(other_config, "smc-enable", false);
> > +    bool smc_enable = smap_get_bool(other_config, "smc-enable", true);
> >      bool cur_smc;
> >      atomic_read_relaxed(&dp->smc_enable_db, &cur_smc);
> >      if (smc_enable != cur_smc) {
> > @@ -4440,7 +4440,7 @@ dpif_netdev_port_set_config(struct dpif *dpif, odp_port_t port_no,
> >      struct dp_netdev_port *port;
> >      int error = 0;
> >      const char *affinity_list = smap_get(cfg, "pmd-rxq-affinity");
> > -    bool emc_enabled = smap_get_bool(cfg, "emc-enable", true);
> > +    bool emc_enabled = smap_get_bool(cfg, "emc-enable", false);
> >  
> >      ovs_mutex_lock(&dp->port_mutex);
> >      error = get_port_by_number(dp, port_no, &port);
> > diff --git a/tests/pmd.at b/tests/pmd.at
> > index 5b612f88f..20c179b39 100644
> > --- a/tests/pmd.at
> > +++ b/tests/pmd.at
> > @@ -238,8 +238,8 @@ pmd thread numa_id <cleared> core_id <cleared>:
> >    packets received: 20
> >    packet recirculations: 0
> >    avg. datapath passes per packet: 1.00
> > -  emc hits: 19
> > -  smc hits: 0
> > +  emc hits: 0
> > +  smc hits: 19
> >    megaflow hits: 0
> >    avg. subtable lookups per megaflow hit: 0.00
> >    miss with success upcall: 1
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > index 81c84927f..009f0e550 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -513,7 +513,7 @@
> >            when flow count is larger than EMC capacity.
> >          </p>
> >          <p>
> > -          Defaults to false but can be changed at any time.
> > +          Defaults to true but can be changed at any time.
> >          </p>
> >        </column>
> >  
> > @@ -3296,7 +3296,7 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
> >            key="emc-insert-inv-prob"/> will have effect on this interface.
> >          </p>
> >          <p>
> > -          Defaults to true.
> > +          Defaults to false.
> >          </p>
> >        </column>
> >      </group>
> > 
>
Jan Scheurich Sept. 22, 2020, 12:57 p.m. UTC | #3
> -----Original Message-----
> From: Flavio Leitner <fbl@sysclose.org>
> On Tue, Sep 22, 2020 at 01:22:58PM +0200, Ilya Maximets wrote:
> > On 9/19/20 3:07 PM, Flavio Leitner wrote:
> > > The EMC is not large enough for current production cases and they
> > > are scaling up, so this change switches over from EMC to SMC by
> > > default, which provides better results.
> > >
> > > The EMC is still available and could be used when only a few number
> > > of flows is used.


> 
> I am curious to find out what others think about this change, so going to wait a
> bit before following up with the next version if that sounds OK.
> 

For production deployments of OVS-DPDK in NFVI we also recommend switching SMC on and EMC off.
No problem with making that configuration default in the future.
As to be expected SMC only provides acceleration over DPCLS if the avg. number of sub-table lookups is > 1.

BR, Jan
Kevin Traynor Sept. 22, 2020, 5:46 p.m. UTC | #4
On 19/09/2020 14:07, Flavio Leitner wrote:
> The EMC is not large enough for current production cases
> and they are scaling up, so this change switches over from
> EMC to SMC by default, which provides better results.
> 
> The EMC is still available and could be used when only a
> few number of flows is used.
> 

Andrew did perf testing of this a couple of years ago. It could be added
as a reference.
http://www.openvswitch.org/support/ovscon2018/5/1330-theurer.pdf

A lot of benchmarks run with a small number of flows that will drop a
lot by default with this. So people may get a surprise when they test
the first OVS version that includes it.

Checked the logs and we don't currently print out the default EMC/SMC
settings, only when they are changed. Perhaps it would be good to add a
log for the default state of EMC/SMC on startup. At least it may be a
clue for when someone hits this.

> Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> ---
>  Documentation/topics/dpdk/bridge.rst | 10 +++++-----
>  NEWS                                 |  3 +++
>  lib/dpif-netdev.c                    |  6 +++---
>  tests/pmd.at                         |  4 ++--
>  vswitchd/vswitch.xml                 |  4 ++--
>  5 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst
> index 526d5c959..0d9aed4eb 100644
> --- a/Documentation/topics/dpdk/bridge.rst
> +++ b/Documentation/topics/dpdk/bridge.rst
> @@ -130,13 +130,13 @@ SMC cache
>  SMC cache or signature match cache is a new cache level after EMC cache.
>  The difference between SMC and EMC is SMC only stores a signature of a flow
>  thus it is much more memory efficient. With same memory space, EMC can store 8k
> -flows while SMC can store 1M flows. When traffic flow count is much larger than
> -EMC size, it is generally beneficial to turn off EMC and turn on SMC. It is
> -currently turned off by default.
> +flows while SMC can store 1M flows. When traffic flow count is small than

Maybe Andrew's results can be used to put a number on 'small', even with
YMMV/caveats etc.

> +EMC size, it is generally beneficial to turn off SMC and turn on EMC. It is
> +currently turned on by default.

Not sure the advice should be to turn off the SMC here. I guess it's
almost zero overhead to keep the SMC enabled if the flows are cached in
the EMC, and if they spill out of the EMC a bit due to collisions or
increased flows it may then be beneficial to have the SMC enabled.

It might also be helpful initially if the SMC is still enabled at the
point where the EMC is first enabled to avoid going to the dpcls, though
I didn't check if it would populate the EMC from a hit in the SMC.

>  
> -To turn on SMC::
> +To turn off SMC::
>  
> -    $ ovs-vsctl --no-wait set Open_vSwitch . other_config:smc-enable=true
> +    $ ovs-vsctl --no-wait set Open_vSwitch . other_config:smc-enable=false
>  
>  Datapath Classifier Performance
>  -------------------------------
> diff --git a/NEWS b/NEWS
> index 2f67d5047..5b88937fb 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -1,6 +1,9 @@
>  Post-v2.14.0
>  ---------------------
>  
> +   - DPDK:
> +     * The SMC cache is enabled by default.
> +     * The EMC cache is disabled by default.
>  
>  v2.14.0 - 17 Aug 2020
>  ---------------------
> diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> index 02df8f11e..0f2bb10fd 100644
> --- a/lib/dpif-netdev.c
> +++ b/lib/dpif-netdev.c
> @@ -2079,7 +2079,7 @@ port_create(const char *devname, const char *type,
>      port->netdev = netdev;
>      port->type = xstrdup(type);
>      port->sf = NULL;
> -    port->emc_enabled = true;
> +    port->emc_enabled = false;
>      port->need_reconfigure = true;
>      ovs_mutex_init(&port->txq_used_mutex);
>  
> @@ -4305,7 +4305,7 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
>          }
>      }
>  
> -    bool smc_enable = smap_get_bool(other_config, "smc-enable", false);
> +    bool smc_enable = smap_get_bool(other_config, "smc-enable", true);
>      bool cur_smc;
>      atomic_read_relaxed(&dp->smc_enable_db, &cur_smc);
>      if (smc_enable != cur_smc) {
> @@ -4440,7 +4440,7 @@ dpif_netdev_port_set_config(struct dpif *dpif, odp_port_t port_no,
>      struct dp_netdev_port *port;
>      int error = 0;
>      const char *affinity_list = smap_get(cfg, "pmd-rxq-affinity");
> -    bool emc_enabled = smap_get_bool(cfg, "emc-enable", true);
> +    bool emc_enabled = smap_get_bool(cfg, "emc-enable", false);
>  
>      ovs_mutex_lock(&dp->port_mutex);
>      error = get_port_by_number(dp, port_no, &port);
> diff --git a/tests/pmd.at b/tests/pmd.at
> index 5b612f88f..20c179b39 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -238,8 +238,8 @@ pmd thread numa_id <cleared> core_id <cleared>:
>    packets received: 20
>    packet recirculations: 0
>    avg. datapath passes per packet: 1.00
> -  emc hits: 19
> -  smc hits: 0
> +  emc hits: 0
> +  smc hits: 19
>    megaflow hits: 0
>    avg. subtable lookups per megaflow hit: 0.00
>    miss with success upcall: 1
> diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> index 81c84927f..009f0e550 100644
> --- a/vswitchd/vswitch.xml
> +++ b/vswitchd/vswitch.xml
> @@ -513,7 +513,7 @@
>            when flow count is larger than EMC capacity.
>          </p>
>          <p>
> -          Defaults to false but can be changed at any time.
> +          Defaults to true but can be changed at any time.
>          </p>
>        </column>
>  
> @@ -3296,7 +3296,7 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
>            key="emc-insert-inv-prob"/> will have effect on this interface.
>          </p>
>          <p>
> -          Defaults to true.
> +          Defaults to false.
>          </p>
>        </column>
>      </group>
>
Flavio Leitner Sept. 24, 2020, 1:14 p.m. UTC | #5
On Tue, Sep 22, 2020 at 06:46:40PM +0100, Kevin Traynor wrote:
> On 19/09/2020 14:07, Flavio Leitner wrote:
> > The EMC is not large enough for current production cases
> > and they are scaling up, so this change switches over from
> > EMC to SMC by default, which provides better results.
> > 
> > The EMC is still available and could be used when only a
> > few number of flows is used.
> > 
> 
> Andrew did perf testing of this a couple of years ago. It could be added
> as a reference.
> http://www.openvswitch.org/support/ovscon2018/5/1330-theurer.pdf

Yeah, good idea.


> A lot of benchmarks run with a small number of flows that will drop a
> lot by default with this. So people may get a surprise when they test
> the first OVS version that includes it.
> 
> Checked the logs and we don't currently print out the default EMC/SMC
> settings, only when they are changed. Perhaps it would be good to add a
> log for the default state of EMC/SMC on startup. At least it may be a
> clue for when someone hits this.

I agree.


> > Signed-off-by: Flavio Leitner <fbl@sysclose.org>
> > ---
> >  Documentation/topics/dpdk/bridge.rst | 10 +++++-----
> >  NEWS                                 |  3 +++
> >  lib/dpif-netdev.c                    |  6 +++---
> >  tests/pmd.at                         |  4 ++--
> >  vswitchd/vswitch.xml                 |  4 ++--
> >  5 files changed, 15 insertions(+), 12 deletions(-)
> > 
> > diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst
> > index 526d5c959..0d9aed4eb 100644
> > --- a/Documentation/topics/dpdk/bridge.rst
> > +++ b/Documentation/topics/dpdk/bridge.rst
> > @@ -130,13 +130,13 @@ SMC cache
> >  SMC cache or signature match cache is a new cache level after EMC cache.
> >  The difference between SMC and EMC is SMC only stores a signature of a flow
> >  thus it is much more memory efficient. With same memory space, EMC can store 8k
> > -flows while SMC can store 1M flows. When traffic flow count is much larger than
> > -EMC size, it is generally beneficial to turn off EMC and turn on SMC. It is
> > -currently turned off by default.
> > +flows while SMC can store 1M flows. When traffic flow count is small than
> 
> Maybe Andrew's results can be used to put a number on 'small', even with
> YMMV/caveats etc.

OK


> > +EMC size, it is generally beneficial to turn off SMC and turn on EMC. It is
> > +currently turned on by default.
> 
> Not sure the advice should be to turn off the SMC here. I guess it's
> almost zero overhead to keep the SMC enabled if the flows are cached in
> the EMC, and if they spill out of the EMC a bit due to collisions or
> increased flows it may then be beneficial to have the SMC enabled.
> 
> It might also be helpful initially if the SMC is still enabled at the
> point where the EMC is first enabled to avoid going to the dpcls, though
> I didn't check if it would populate the EMC from a hit in the SMC.

Good point. I think it could elaborate saying that there are few
possibilities like having both OFF, both ON, either one ON and the
other OFF.

Thanks for the review!
fbl
> 
> >  
> > -To turn on SMC::
> > +To turn off SMC::
> >  
> > -    $ ovs-vsctl --no-wait set Open_vSwitch . other_config:smc-enable=true
> > +    $ ovs-vsctl --no-wait set Open_vSwitch . other_config:smc-enable=false
> >  
> >  Datapath Classifier Performance
> >  -------------------------------
> > diff --git a/NEWS b/NEWS
> > index 2f67d5047..5b88937fb 100644
> > --- a/NEWS
> > +++ b/NEWS
> > @@ -1,6 +1,9 @@
> >  Post-v2.14.0
> >  ---------------------
> >  
> > +   - DPDK:
> > +     * The SMC cache is enabled by default.
> > +     * The EMC cache is disabled by default.
> >  
> >  v2.14.0 - 17 Aug 2020
> >  ---------------------
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 02df8f11e..0f2bb10fd 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -2079,7 +2079,7 @@ port_create(const char *devname, const char *type,
> >      port->netdev = netdev;
> >      port->type = xstrdup(type);
> >      port->sf = NULL;
> > -    port->emc_enabled = true;
> > +    port->emc_enabled = false;
> >      port->need_reconfigure = true;
> >      ovs_mutex_init(&port->txq_used_mutex);
> >  
> > @@ -4305,7 +4305,7 @@ dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
> >          }
> >      }
> >  
> > -    bool smc_enable = smap_get_bool(other_config, "smc-enable", false);
> > +    bool smc_enable = smap_get_bool(other_config, "smc-enable", true);
> >      bool cur_smc;
> >      atomic_read_relaxed(&dp->smc_enable_db, &cur_smc);
> >      if (smc_enable != cur_smc) {
> > @@ -4440,7 +4440,7 @@ dpif_netdev_port_set_config(struct dpif *dpif, odp_port_t port_no,
> >      struct dp_netdev_port *port;
> >      int error = 0;
> >      const char *affinity_list = smap_get(cfg, "pmd-rxq-affinity");
> > -    bool emc_enabled = smap_get_bool(cfg, "emc-enable", true);
> > +    bool emc_enabled = smap_get_bool(cfg, "emc-enable", false);
> >  
> >      ovs_mutex_lock(&dp->port_mutex);
> >      error = get_port_by_number(dp, port_no, &port);
> > diff --git a/tests/pmd.at b/tests/pmd.at
> > index 5b612f88f..20c179b39 100644
> > --- a/tests/pmd.at
> > +++ b/tests/pmd.at
> > @@ -238,8 +238,8 @@ pmd thread numa_id <cleared> core_id <cleared>:
> >    packets received: 20
> >    packet recirculations: 0
> >    avg. datapath passes per packet: 1.00
> > -  emc hits: 19
> > -  smc hits: 0
> > +  emc hits: 0
> > +  smc hits: 19
> >    megaflow hits: 0
> >    avg. subtable lookups per megaflow hit: 0.00
> >    miss with success upcall: 1
> > diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
> > index 81c84927f..009f0e550 100644
> > --- a/vswitchd/vswitch.xml
> > +++ b/vswitchd/vswitch.xml
> > @@ -513,7 +513,7 @@
> >            when flow count is larger than EMC capacity.
> >          </p>
> >          <p>
> > -          Defaults to false but can be changed at any time.
> > +          Defaults to true but can be changed at any time.
> >          </p>
> >        </column>
> >  
> > @@ -3296,7 +3296,7 @@ ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
> >            key="emc-insert-inv-prob"/> will have effect on this interface.
> >          </p>
> >          <p>
> > -          Defaults to true.
> > +          Defaults to false.
> >          </p>
> >        </column>
> >      </group>
> > 
>
diff mbox series

Patch

diff --git a/Documentation/topics/dpdk/bridge.rst b/Documentation/topics/dpdk/bridge.rst
index 526d5c959..0d9aed4eb 100644
--- a/Documentation/topics/dpdk/bridge.rst
+++ b/Documentation/topics/dpdk/bridge.rst
@@ -130,13 +130,13 @@  SMC cache
 SMC cache or signature match cache is a new cache level after EMC cache.
 The difference between SMC and EMC is SMC only stores a signature of a flow
 thus it is much more memory efficient. With same memory space, EMC can store 8k
-flows while SMC can store 1M flows. When traffic flow count is much larger than
-EMC size, it is generally beneficial to turn off EMC and turn on SMC. It is
-currently turned off by default.
+flows while SMC can store 1M flows. When traffic flow count is small than
+EMC size, it is generally beneficial to turn off SMC and turn on EMC. It is
+currently turned on by default.
 
-To turn on SMC::
+To turn off SMC::
 
-    $ ovs-vsctl --no-wait set Open_vSwitch . other_config:smc-enable=true
+    $ ovs-vsctl --no-wait set Open_vSwitch . other_config:smc-enable=false
 
 Datapath Classifier Performance
 -------------------------------
diff --git a/NEWS b/NEWS
index 2f67d5047..5b88937fb 100644
--- a/NEWS
+++ b/NEWS
@@ -1,6 +1,9 @@ 
 Post-v2.14.0
 ---------------------
 
+   - DPDK:
+     * The SMC cache is enabled by default.
+     * The EMC cache is disabled by default.
 
 v2.14.0 - 17 Aug 2020
 ---------------------
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 02df8f11e..0f2bb10fd 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -2079,7 +2079,7 @@  port_create(const char *devname, const char *type,
     port->netdev = netdev;
     port->type = xstrdup(type);
     port->sf = NULL;
-    port->emc_enabled = true;
+    port->emc_enabled = false;
     port->need_reconfigure = true;
     ovs_mutex_init(&port->txq_used_mutex);
 
@@ -4305,7 +4305,7 @@  dpif_netdev_set_config(struct dpif *dpif, const struct smap *other_config)
         }
     }
 
-    bool smc_enable = smap_get_bool(other_config, "smc-enable", false);
+    bool smc_enable = smap_get_bool(other_config, "smc-enable", true);
     bool cur_smc;
     atomic_read_relaxed(&dp->smc_enable_db, &cur_smc);
     if (smc_enable != cur_smc) {
@@ -4440,7 +4440,7 @@  dpif_netdev_port_set_config(struct dpif *dpif, odp_port_t port_no,
     struct dp_netdev_port *port;
     int error = 0;
     const char *affinity_list = smap_get(cfg, "pmd-rxq-affinity");
-    bool emc_enabled = smap_get_bool(cfg, "emc-enable", true);
+    bool emc_enabled = smap_get_bool(cfg, "emc-enable", false);
 
     ovs_mutex_lock(&dp->port_mutex);
     error = get_port_by_number(dp, port_no, &port);
diff --git a/tests/pmd.at b/tests/pmd.at
index 5b612f88f..20c179b39 100644
--- a/tests/pmd.at
+++ b/tests/pmd.at
@@ -238,8 +238,8 @@  pmd thread numa_id <cleared> core_id <cleared>:
   packets received: 20
   packet recirculations: 0
   avg. datapath passes per packet: 1.00
-  emc hits: 19
-  smc hits: 0
+  emc hits: 0
+  smc hits: 19
   megaflow hits: 0
   avg. subtable lookups per megaflow hit: 0.00
   miss with success upcall: 1
diff --git a/vswitchd/vswitch.xml b/vswitchd/vswitch.xml
index 81c84927f..009f0e550 100644
--- a/vswitchd/vswitch.xml
+++ b/vswitchd/vswitch.xml
@@ -513,7 +513,7 @@ 
           when flow count is larger than EMC capacity.
         </p>
         <p>
-          Defaults to false but can be changed at any time.
+          Defaults to true but can be changed at any time.
         </p>
       </column>
 
@@ -3296,7 +3296,7 @@  ovs-vsctl add-port br0 p0 -- set Interface p0 type=patch options:peer=p1 \
           key="emc-insert-inv-prob"/> will have effect on this interface.
         </p>
         <p>
-          Defaults to true.
+          Defaults to false.
         </p>
       </column>
     </group>