The incremental processing engine might stop a run before the en_runtime_data node is processed. In such cases the ed_runtime_data fields might contain pointers to already deleted SB records. For example, if a port binding corresponding to a patch port is removed from the SB database and the incremental processing engine aborts before the en_runtime_data node is processed then the corresponding local_datapath hashtable entry in ed_runtime_data is stale and will store a pointer to the already freed sbrec_port_binding record.
This will cause invalid memory accesses in various places (e.g., pinctrl_run() -> prepare_ipv6_ras()). This series fixes the issue (patch4) but to make the fix generic and easier to debug it first refactors the incremental processing engine in the following way: - patch1: split engine_run() in smaller functional parts and simplify the logic of calling engine_run and engine_need_run in the main loop. - patch2: remove recursion from the I-P engine code. Introduce node states to track validity of node data. - patch3: move ct-zones to its own engine node in order to remove dependencies on other runtime data. CC: Han Zhou <hz...@ovn.org> Fixes: ca278d98a4f5 ("ovn-controller: Initial use of incremental engine - quiet mode.") Signed-off-by: Dumitru Ceara <dce...@redhat.com> Dumitru Ceara (4): ovn-controller: Refactor I-P engine_run() tracking. ovn-controller: Add per node states to I-P engine. ovn-controller: Add separate I-P engine node for processing ct-zones. ovn-controller: Fix use of dangling pointers in I-P runtime_data. controller/ovn-controller.c | 414 ++++++++++++++++++++++++------------------- lib/inc-proc-eng.c | 338 +++++++++++++++++++++++++++-------- lib/inc-proc-eng.h | 103 ++++++++--- 3 files changed, 570 insertions(+), 285 deletions(-) --- v6: - Address Han's comments: - Call engine_recompute only once for a node if at least one of its input nodes' change handler returns false. - Simplify the incremental engine API and internally store the topologically sorted engine nodes. - Change 'engine_abort_recompute' from global variable to argument to be passed to engine_run(). It's only relevant in one run context anyway as we used to reset it before every call to engine_run(). - engine_init_run() and engine_has_run() now check all the nodes in the engine instead of a single one. - Change 'engine_node_valid()' to call the node's 'is_valid' method only if the node is not in state EN_UPDATED or EN_VALID. v5: - Rebase. v4: - Address Numan's comments: - Fix engine_need_run(). v3: - split the change in series. - Address Han's comments: - fix the data encapsulation issue. - add is_valid method to nodes. - add internal_data/data fields to nodes as it makes it easier to write the code instead of adding an "engine_get_data()" API. v2: Address Han's comments: - call engine_node_valid() in all the places where node local data is used. - move out "global" data outside the engine nodes. Make a clear separation between data that can be safely used at any time and data that can be used only when the engine run was successful. - add a debug log for iterations when the engine didn't run. - refactor a bit more the incremental engine code. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev