As discussed at last OVN meeting, I am reposting the patch series after rebasing on latest master branch. There has been a lot a discussion since last review. The reason it was hold on were: 1. The concern of long term maintenance effort, since the change handler implementation can be complex if we want to support more incremental processing. 2. DDlog is showing promissing progress and we concluded that it not only fits for ovn-northd but also for ovn-controller. Once implemented it should be easier to maintain than the approach taken by this series.
These points are still valid and unchanged. The reason we are revisiting this patch series are: 1. Incremental processing is critical for scalability in production. Although DDlog is the long term solution, it can't be ready soon enough to solve current problem. 2. The current approach taken in this series is proved stable and effective in production, and it doesn't necessarily cause maintenance problems because it allows easily fall-back to full recomputing path. 3. This series sorted out dependency graph in flow compute related components which will help future DDlog convertion. So it doesn't conflict with the DDlog path we are targeting and could provide a smooth transition. Below are the notes from v5: ------------------------------------------------------------------------ ovn-controller currently recomputes everything when there are any changes of input, which leads to high CPU usages and slow in end-to-end flow enforcement in response to changes. It even wastes CPU to recompute flows for unrelated inputs such as pinctrl events. This patch series implements incremental processing in ovn-controller to solve above problems. There has been a similar attempt of solve the problem earlier but was reverted (see commit: 926c34fd). This patch series takes a different approach with an incremental processing engine, to make the dependencies clear and easier to maintain. For performance test results please find them in commit messages. The major change from v3 to v5 is better dependency maintenance. Firstly, it forces explicit dependency exposure for OVSDB tables used by any engine nodes, thanks for Ben's patches that supported passing individual tables. Since we are not passing IDL directly to the engine, missing dependency will either make compiling failed or result in assert failure in the first run. Secondly, it forces implicit dependency exposure for OVSDB table references through change tracking implementation, so that any referenced row change will cause the referencing node change, which will trigger proper change handling in the engine. Because of this capability, in change handler of port-binding, the logic is simplied a lot so that we don't have to re- consider multicast_group when port-binding is processed. Instead, change handler of multicast_group will automatically be triggered, and it will be incrementally processed, too. Some more notes: * In engine node any global variable access should be wrapped as engine node input. One example is mff_ovn_geneve, which is implemented as a separate engine node (an input node). However, currently there is no way to force this. So it depends on developers' awareness and code reviews. (Currently I don't see any global variables directly accessed from engine.) * Although dependencies between nodes are clear, the correctness of implementing change handler within an engine node relies on developers only - there is no mechanism yet to guarantee the outcome of change handler implementation is equivalent to full recompute, i.e. the run() function. * There can be further optimization when tables being referenced is not accessed, so that a change may not have to be handled by a specific engine node. This may be achieved by defining different versions of tables with different access permissions to referenced tables, as demonstrated by Ben's RFC: https://mail.openvswitch.org/pipermail/ovs-dev/2018-June/348454.html. However, it seems not necessary at this point for most frequent use cases that we are targeting for performance improvement. For example, port-binding is referenced by multicast-group, but we do need to access port-binding from multicast-group in flow-output engine node, and both of them are incrementally processed. There is no other table references for the incremental processing we have in this patch series (port-binding, multicast-group, logical-flow, address-set, port-group). If we do have such use cases in the future, we may need to implement change tracking for different versions of the same table. -------------------------------------------------------------------------- Han Zhou (15): ovn-controller: Incremental processing engine ovn-controller: Track OVSDB changes ovn-controller: Initial use of incremental engine - quiet mode. ovn-controller: Incremental logical flow processing ovn-controller: runtime_data change handler for SB port-binding ovn-controller: port-binding incremental processing for physical flows ovn-controller: incremental processing for multicast group changes ovsdb-idl: Tracking - preserve data for deleted rows. ovn-controller: Split addr_sets from runtime_data. ovn-controller: Maintain resource references for logical flows. ovn-controller: Incremental processing for address-set changes. ovn-controller: Split port_groups from runtime_data. ovn-controller: Incremental processing for port-group changes. ovn-performance.at: Test port group incremental processing. ovn-controller: mac_binding incremental processing. Jakub Sitnicki (2): coverage: Add command for reading counter value. ovn: Test for full logical flow processing in ovn-controller. include/ovn/actions.h | 3 + include/ovn/expr.h | 5 +- lib/coverage-unixctl.man | 2 + lib/coverage.c | 42 ++ lib/ovsdb-idl-provider.h | 2 + lib/ovsdb-idl.c | 36 +- ovn/controller/bfd.c | 4 +- ovn/controller/binding.c | 117 ++- ovn/controller/binding.h | 6 + ovn/controller/encaps.c | 12 +- ovn/controller/lflow.c | 456 +++++++++++- ovn/controller/lflow.h | 103 ++- ovn/controller/ofctrl.c | 270 +++++-- ovn/controller/ofctrl.h | 41 +- ovn/controller/ovn-controller.c | 1571 +++++++++++++++++++++++++++++++++------ ovn/controller/physical.c | 186 +++-- ovn/controller/physical.h | 18 +- ovn/lib/actions.c | 11 +- ovn/lib/automake.mk | 4 +- ovn/lib/expr.c | 21 +- ovn/lib/extend-table.c | 60 +- ovn/lib/extend-table.h | 16 +- ovn/lib/inc-proc-eng.c | 201 +++++ ovn/lib/inc-proc-eng.h | 234 ++++++ ovn/utilities/ovn-trace.c | 2 +- tests/automake.mk | 3 +- tests/ovn-performance.at | 420 +++++++++++ tests/ovn.at | 75 ++ tests/test-ovn.c | 7 +- tests/testsuite.at | 1 + 30 files changed, 3442 insertions(+), 487 deletions(-) create mode 100644 ovn/lib/inc-proc-eng.c create mode 100644 ovn/lib/inc-proc-eng.h create mode 100644 tests/ovn-performance.at -- 2.1.0 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
