Han Zhou <zhou...@gmail.com> wrote on 04/01/2016 06:09:03 PM:
> From: Han Zhou <zhou...@gmail.com>
> To: Ryan Moats/Omaha/IBM@IBMUS
> Cc: "dev@openvswitch.org" <dev@openvswitch.org>, Lei Huang
<lhua...@ebay.com>
> Date: 04/01/2016 06:09 PM
> Subject: Re: [ovs-dev] [PATCH v13 0/8] Add incremental processing
>
> On Thu, Mar 31, 2016 at 8:05 AM, Ryan Moats <rmo...@us.ibm.com> wrote:
> From: RYAN D. MOATS <rmo...@us.ibm.com>
>
> It looks like v11 and v12 had some interesting rebase issues,
> so v13 is a rebase back to master only
>
> RYAN D. MOATS (8):
> Make flow table persistent in ovn controller
> Persist lport and mcgroup indexes
> Persist local_datapaths and patched_datapaths
> Add incremental proessing to lflow_run
> Change encaps_run to work incrementally
> Convert binding_run to incremental processing.
> Reset lflow processing when adding/removing patch ports
> Change physical_run to incremental processing
>
> lib/ofp-actions.c | 12 ++
> lib/ofp-actions.h | 2 +
> ovn/controller/binding.c | 99 +++++++++++++--
> ovn/controller/binding.h | 1 +
> ovn/controller/encaps.c | 163 +++++++++++++++++-------
> ovn/controller/lflow.c | 116 +++++++++++++++--
> ovn/controller/lflow.h | 6 +-
> ovn/controller/lport.c | 142 +++++++++++++++++++---
> ovn/controller/lport.h | 20 +++-
> ovn/controller/ofctrl.c | 264 ++++++++++++++++++++++++++
+-----------
> ovn/controller/ofctrl.h | 18 ++-
> ovn/controller/ovn-controller.c | 52 +++-----
> ovn/controller/ovn-controller.h | 2 +
> ovn/controller/patch.c | 7 +-
> ovn/controller/physical.c | 201 +++++++++++++++++++++++-------
> ovn/controller/physical.h | 6 +-
> 16 files changed, 855 insertions(+), 256 deletions(-)
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
> Here is the profiling result from Lei. Based on the testing result,
> this version is great! Thanks Ryan.
> version
> This test uses commit f48e869 on ovs/master as base and OVN
> incremental patch v13.
> Below patches are already included in ovs/master:
> ovn-controller: Loopback prevention flows for local ports only.
> ovn-controller: Optimize processing for non-local datapath without
> patch ports.
> ovn-controller: Optimize lex_token memory usage.
> Test case running time
> For the create and bind 500 lports (on top of 20k lports on 2k HVs),
> the running time as below:
>
> ovn_network.create_network
>
> 1.846
>
> 1.834
>
> ovn.create_lswitch
>
> 1.086
>
> 1.084
>
> ovn.create_lport
>
> 0.77
>
> 0.804
>
> ovn_network.bind_port
>
> 77.012
>
> 71.74
>
> ovn_network.wait_port_up
>
> 0.788
>
> 0.704
>
> ovn.create_lport (2)
>
> 0.758
>
> 0.75
>
> ovn_network.bind_port (2)
>
> 77.33
>
> 70.228
>
> ovn_network.wait_port_up (2)
>
> 0.803
>
> 0.698
>
> ovn.create_lport (3)
>
> 0.821
>
> 0.836
>
> ovn_network.bind_port (3)
>
> 79.842
>
> 72.493
>
> ovn_network.wait_port_up (3)
>
> 0.956
>
> 0.782
>
> ovn.create_lport (4)
>
> 0.795
>
> 0.843
>
> ovn_network.bind_port (4)
>
> 81.584
>
> 72.866
>
> ovn_network.wait_port_up (4)
>
> 0.78
>
> 0.807
>
> ovn.create_lport (5)
>
> 0.837
>
> 0.823
>
> ovn_network.bind_port (5)
>
> 83.603
>
> 72.483
>
> ovn_network.wait_port_up (5)
>
> 0.856
>
> 0.867
>
> total
>
> 405.199
>
> 365.701
>
>
> Profiling
> Before apply ovn incremental processing v13 patch:
>
> - 15.71% ovn-controller ovn-controller [.]
>
next_real_row
> - next_real_row
> - 47.10% add_logical_flows.isra.2
> lflow_run
> main
> + 21.55% patch_run
> + 10.13% physical_run
> + 9.85% binding_run
> + 9.77% lport_index_init
> + 1.03% encaps_run
> - 12.98% ovn-controller ovn-controller [.] ovsdb_idl_next_row
> - ovsdb_idl_next_row
> + 56.97% add_logical_flows.isra.2
> + 29.25% patch_run
> + 5.90% lport_index_init
> + 3.29% binding_run
> + 2.95% physical_run
> + 1.35% encaps_run
> - 8.54% ovn-controller ovn-controller [.] physical_run
> physical_run
> main
> - 7.89% ovn-controller ovn-controller [.]
add_logical_flows.isra.2
> add_logical_flows.isra.2
> lflow_run
> main
> - 7.15% ovn-controller libc-2.19.so [.] strlen
> - strlen
> + 26.47% simap_find
> + 24.20% shash_find
> + 23.55% lport_lookup_by_name
> + 12.42% sset_add
> + 4.70% smap_get_node
> + 3.77% port_hash
> + 1.83% physical_run
> + 1.35% lport_index_init
> + 0.84% chassis_tunnel_find
> + 0.54% mcgroup_lookup_by_dp_name
> - 4.71% ovn-controller ovn-controller [.] hash_bytes
> - hash_bytes
> + 42.36% smap_get_node
> + 15.57% simap_find_len
> + 9.25% shash_find
> + 8.28% lport_index_init
> + 7.98% lport_lookup_by_name
> + 7.60% sset_add
> + 4.08% port_hash
> + 3.78% chassis_tunnel_find
> + 0.66% physical_run
> - 3.26% ovn-controller ovn-controller [.] smap_find__
> + smap_find__
> + smap_get
>
>
> After apply the patch:
> - 5.82% ovn-controller libc-2.19.so [.] __memcmp_sse4_1
> - __memcmp_sse4_1
> - 43.03% ofpacts_equal
> ofctrl_put
> main
> - 30.66% match_equal
> ovn_flow_lookup
> ofctrl_put
> main
> - 26.31% flow_wildcards_equal
> match_equal
> - ovn_flow_lookup
> + 99.99% ofctrl_put
> - 5.70% ovn-controller [kernel.kallsyms] [k] clear_page_c_e
> - clear_page_c_e
> - __alloc_pages_nodemask
> - alloc_pages_vma
> - 99.98% handle_mm_fault
> __do_page_fault
> do_page_fault
> - page_fault
> - 85.06% ofctrl_add_flow
> + 58.27% lflow_run
> + 41.73% physical_run
> + 10.40% __memcpy_sse2_unaligned
> + 3.29% 0x7fcf6a546e65
> + 0.98% memset
> - 5.22% ovn-controller libjemalloc.so.1 [.] free
> - free
> + 27.46% lexer_get
> + 18.00% ofctrl_put
> + 14.46% make_cmp
> + 5.94% expr_annotate__
> + 5.39% crush_and_numeric
> + 4.36% ovn_flow_destroy
> + 3.83% expr_parse_assignment
> + 3.08% lflow_run
> + 2.75% expr_destroy
> + 2.17% expr_parse_string
> + 2.11% expr_constant_set_destroy
> + 1.82% lexer_lookahead
> + 1.79% expr_normalize_and
> + 1.73% expr_matches_destroy
> + 1.65% simap_clear
> + 1.48% expr_combine
> + 1.07% qsort_r
> - 5.07% ovn-controller ovn-controller [.] bitwise_rscan
> - bitwise_rscan
> - mf_subvalue_width
> - type_check.isra.15
> - 94.87% make_cmp
> expr_parse_primary
> expr_parse_not
> expr_parse__
> expr_parse
> + expr_parse_string
> + 5.13% expr_parse_assignment
>
Many thanks to Han and Lei for testing this, but I'm not quite
ready to declare victory just yet. One of the things this patch
set does that may be unexpected is that it turns on change
tracking and never turns it off.
This was originally done because of my not quite understanding
what change tracking did for me, and so I've started thinking
about what a refactor that uses change tracking properly would
look like. I'm a bit concerned because I seem to be ending up
with a code structure that looks like:
if I need to reset processing:
clear out persisted data
loop using SBREC_*_FOR_EACH
call to method that performs processing
else /* can do incremental processing */
loop using SBREC_*_FOR_EACH_TRACKED
lookup delete seqno, handle deletes and continue
call to method that performs processing
endif
The above looks rather ugly to me, so I wanted to throw the
issue out there for folks (I look pointedly at Ben) to comment
on. If folks think it makes sense, then I'll work on spinning
v14 with the corrected structure.
Ryan
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
