[ovs-build] Failed: dceara/ovn#87 (bug-inc-processing-stale-data-v2 - 92be4f2)

[Travis CI]

Build Update for dceara/ovn
-------------------------------------

Build: #87
Status: Failed

Duration: 6 mins and 24 secs
Commit: 92be4f2 (bug-inc-processing-stale-data-v2)
Author: Dumitru Ceara
Message: ovn-controller: Fix use of dangling pointers in I-P runtime_data.

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()).

To fix the issue we need a way to track how each node was processed
during an engine run. This commit transforms the 'changed' field in
struct engine_node in a 'state' field. Possible node states are:
- "New": the node is not yet initialized.
- "Stale": data in the node is not up to date with the DB.
- "Updated": data in the node is valid but was updated during
  the last run of the engine.
- "Valid": data in the node is valid and didn't change during
  the last run of the engine.
- "Aborted": during the last run, processing was aborted for
  this node.
- "Destroyed": the node was already cleaned up.

We also add a separation between engine node data that can be accessed
at any time (regardless if the last engine run was successful or not)
and data that may be accessed only if the nodes are up to date. This
helps avoiding custom "engine_node_valid" handlers for different
nodes.

The commit also simplifies the logic of calling engine_run and
engine_need_run in order to reduce the number of external variables
required to track the result of the last engine execution.

Functions that need to be called from the main loop and depend on
various data contents of the engine's nodes are now called only if
the data is up to date.

CC: Han Zhou <hzhou8 at ebay.com>
Fixes: ca278d98a4f5 ("ovn-controller: Initial use of incremental engine - quiet mode.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>

---
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.

View the changeset: https://github.com/dceara/ovn/compare/5b7cc608c0c7^...92be4f2088f8

View the full build log and details: https://travis-ci.org/dceara/ovn/builds/607101218?utm_medium=notification&utm_source=email

--

You can unsubscribe from build emails from the dceara/ovn repository going to https://travis-ci.org/account/preferences/unsubscribe?repository=25507942&utm_medium=notification&utm_source=email.
Or unsubscribe from *all* email updating your settings at https://travis-ci.org/account/preferences/unsubscribe?utm_medium=notification&utm_source=email.
Or configure specific recipients for build notifications in your .travis.yml file. See https://docs.travis-ci.com/user/notifications.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openvswitch.org/pipermail/ovs-build/attachments/20191104/deaedede/attachment-0001.html>