Hi Dumitru,
I reviewed this changeset as a whole rather than trying to review each
individual patch.
Why does runtime_data_sb_port_binding_handler() in ovn-controller.c not
update node state? Other change handlers update the node state based on
whether data has changed. Is there some special reason why this
particular change handler does not or cannot update node state? I'm
guessing this is done on purpose, but because there is no comment it
seems like an omission.
On 11/18/19 9:06 AM, Dumitru Ceara wrote:
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 | 418 ++++++++++++++++++++++++-------------------
lib/inc-proc-eng.c | 314 +++++++++++++++++++++++++-------
lib/inc-proc-eng.h | 104 +++++++++--
3 files changed, 564 insertions(+), 272 deletions(-)
---
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
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
