On 2/27/24 05:32, Ilya Maximets wrote:
On 2/27/24 07:51, Han Zhou wrote:


On Mon, Feb 26, 2024 at 9:44 AM Ilya Maximets <i.maxim...@ovn.org 
<mailto:i.maxim...@ovn.org>> wrote:

Currently, ovn-controller attempts to sync all the meters on each
ofctrl_put() call.  And the complexity of this logic is quadratic
because for each desired meter we perform a full scan of all the
rows in the Southbound Meter table in order to lookup a matching
meter.  This is very inefficient.  In a setup with 25K meters this
operation takes anywhere from 30 to 60 seconds to perform.  All
that time ovn-controller is blocked and doesn't process any updates.
So, addition of new ports in such a setup becomes very slow.

The meter lookup is performed by name and we have an index for it
in the database schema.  Might as well use it.

Using the index for lookup reduces complexity to O(n * log n).
And the time to process port addition on the same setup drops down
to just 100 - 300 ms.

We are still iterating over all the desired meters while they can
probably be processed incrementally instead.  But using an index
is a simpler fix for now.

Fixes: 885655e16e63 ("controller: reconfigure ovs meters for ovn meters")
Fixes: 999e1adfb572 ("ovn: Support configuring meters through SB Meter table.")
Reported-at: https://issues.redhat.com/browse/FDP-399 
<https://issues.redhat.com/browse/FDP-399>
Signed-off-by: Ilya Maximets <i.maxim...@ovn.org <mailto:i.maxim...@ovn.org>>
---

This is a "performance bug", so the decision to backport this or not
is on maintainers.  But it is severe enough, IMO.


Thanks Ilya. The fix looks good to me. And I think it is ok to backport, since 
the change is simple enough.

Acked-by: Han Zhou <hz...@ovn.org <mailto:hz...@ovn.org>>

Just curious, how would the OVS perform with this large number of meters?

Hi.  Thanks for review!

I don't think 25K would cause any noticeable issues on OVS side,
but I agree that if we scale further we may overload the classifier
at some point.  I don't have hard evidence to back this up, but
based on my prior experience with flow explosion with negative
matches, I think, we need about 200K rules in order for it to start
becoming a problem.  But also ACL rules are not normally conjunctive,
i.e. should be easier for classifier to process, so we may probably
handle more.  Needs some testing.

Best regards, Ilya Maximets.

Thank you Han and Ilya.

I pushed this change to main and all branches back to 23.03.

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

Reply via email to