mbox series

[ovs-dev,ovn,0/2] Caching logical flow expr tree for each lflow

Message ID 20200109173629.1482618-1-numans@ovn.org
Headers show
Series Caching logical flow expr tree for each lflow | expand

Message

Numan Siddique Jan. 9, 2020, 5:36 p.m. UTC
From: Numan Siddique <numans@ovn.org>

This patch series tries to improve the time taken to proceess logical
flows by caching the expr tree. For large scale deployments with lots
of logical flows, the logical flow processing to OpenFlow rules
takes a lot of time as it is CPU intensive.

This patch series tries to improve this by caching the expr tree
generated for each logical flow so that we don't have to generate the
expr tree for each lflow_run().

Below are the details of the OVN resource in my setup
    
No of logical switches - 49
No of logical ports    - 1191
No of logical routers  - 7
No of logical ports    - 51
No of ACLs             - 1221
No of Logical flows    - 664736
    
On a chassis hosting 7 distributed router ports and around 1090
port bindings, the no of OVS rules was 921162.
    
Without this patch, the function add_logical_flows() took ~15 seconds.
And with this patch it took ~10 seconds. There is a good 34% improvement
in logical flow processing.


Numan Siddique (2):
  expr: Evaluate the condition expression in a separate step.
  ovn-controller: Cache logical flow expr tree for each lflow.

 controller/lflow.c          | 181 +++++++++++++++++++++++++++---------
 controller/lflow.h          |   9 +-
 controller/ovn-controller.c |  17 +++-
 include/ovn/expr.h          |  10 +-
 lib/expr.c                  |  55 ++++++++---
 tests/test-ovn.c            |  10 +-
 utilities/ovn-trace.c       |   5 +-
 7 files changed, 212 insertions(+), 75 deletions(-)

Comments

Mark Michelson Jan. 15, 2020, 10:31 p.m. UTC | #1
For the series:
Acked-by: Mark Michelson <mmichels@redhat.com>

I think the speedup is commendable. Great job!

That being said, I'm surprised it wasn't better. Do you know, with this 
patch applied, what the hit rate is for the cache? And assuming the hit 
rate is high, do you know what the new bottleneck in add_logical_flows() is?

On 1/9/20 12:36 PM, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> This patch series tries to improve the time taken to proceess logical
> flows by caching the expr tree. For large scale deployments with lots
> of logical flows, the logical flow processing to OpenFlow rules
> takes a lot of time as it is CPU intensive.
> 
> This patch series tries to improve this by caching the expr tree
> generated for each logical flow so that we don't have to generate the
> expr tree for each lflow_run().
> 
> Below are the details of the OVN resource in my setup
>      
> No of logical switches - 49
> No of logical ports    - 1191
> No of logical routers  - 7
> No of logical ports    - 51
> No of ACLs             - 1221
> No of Logical flows    - 664736
>      
> On a chassis hosting 7 distributed router ports and around 1090
> port bindings, the no of OVS rules was 921162.
>      
> Without this patch, the function add_logical_flows() took ~15 seconds.
> And with this patch it took ~10 seconds. There is a good 34% improvement
> in logical flow processing.
> 
> 
> Numan Siddique (2):
>    expr: Evaluate the condition expression in a separate step.
>    ovn-controller: Cache logical flow expr tree for each lflow.
> 
>   controller/lflow.c          | 181 +++++++++++++++++++++++++++---------
>   controller/lflow.h          |   9 +-
>   controller/ovn-controller.c |  17 +++-
>   include/ovn/expr.h          |  10 +-
>   lib/expr.c                  |  55 ++++++++---
>   tests/test-ovn.c            |  10 +-
>   utilities/ovn-trace.c       |   5 +-
>   7 files changed, 212 insertions(+), 75 deletions(-)
>
Numan Siddique Jan. 16, 2020, 2:15 p.m. UTC | #2
On Thu, Jan 16, 2020 at 4:02 AM Mark Michelson <mmichels@redhat.com> wrote:
>
> For the series:
> Acked-by: Mark Michelson <mmichels@redhat.com>
>
> I think the speedup is commendable. Great job!
>
> That being said, I'm surprised it wasn't better. Do you know, with this
> patch applied, what the hit rate is for the cache? And assuming the hit
> rate is high, do you know what the new bottleneck in add_logical_flows() is?

Thanks for the review.

The hit ratio will be 100% for subsequent flow calculations for a logical flow.
However if the address set/port group associated with a logical flow
gets updated,
the cache is destroyed and updated with the new one.

Actually even I was surprised too. I was expecting a better improvement.

Right now, we still clone the (normalized) expr as
expr_evaluate_condition() could
destroy the original expression.
I think major chunk is spent in the expr_to_matches() function.

Thanks
Numan


>
> On 1/9/20 12:36 PM, numans@ovn.org wrote:
> > From: Numan Siddique <numans@ovn.org>
> >
> > This patch series tries to improve the time taken to proceess logical
> > flows by caching the expr tree. For large scale deployments with lots
> > of logical flows, the logical flow processing to OpenFlow rules
> > takes a lot of time as it is CPU intensive.
> >
> > This patch series tries to improve this by caching the expr tree
> > generated for each logical flow so that we don't have to generate the
> > expr tree for each lflow_run().
> >
> > Below are the details of the OVN resource in my setup
> >
> > No of logical switches - 49
> > No of logical ports    - 1191
> > No of logical routers  - 7
> > No of logical ports    - 51
> > No of ACLs             - 1221
> > No of Logical flows    - 664736
> >
> > On a chassis hosting 7 distributed router ports and around 1090
> > port bindings, the no of OVS rules was 921162.
> >
> > Without this patch, the function add_logical_flows() took ~15 seconds.
> > And with this patch it took ~10 seconds. There is a good 34% improvement
> > in logical flow processing.
> >
> >
> > Numan Siddique (2):
> >    expr: Evaluate the condition expression in a separate step.
> >    ovn-controller: Cache logical flow expr tree for each lflow.
> >
> >   controller/lflow.c          | 181 +++++++++++++++++++++++++++---------
> >   controller/lflow.h          |   9 +-
> >   controller/ovn-controller.c |  17 +++-
> >   include/ovn/expr.h          |  10 +-
> >   lib/expr.c                  |  55 ++++++++---
> >   tests/test-ovn.c            |  10 +-
> >   utilities/ovn-trace.c       |   5 +-
> >   7 files changed, 212 insertions(+), 75 deletions(-)
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>