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 <f...@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> > > > -- fbl _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
