On Tue, Jun 9, 2020 at 1:30 PM Dumitru Ceara <[email protected]> wrote:
> On 6/8/20 7:34 PM, Numan Siddique wrote: > > > > > > On Mon, Jun 8, 2020 at 9:42 PM Dumitru Ceara <[email protected] > > <mailto:[email protected]>> wrote: > > > > On 6/8/20 3:50 PM, [email protected] <mailto:[email protected]> wrote: > > > From: Numan Siddique <[email protected] <mailto:[email protected]>> > > > > > > A new function is added in the engine node called - > > clear_tracked_data() to > > > clear any engine data which was tracked during the engine run. > > This tracked data > > > has to be part of the engine 'data'. engine_init_run() calls > > clear_tracked_data() > > > and each engine node interested in tracking the data needs to > > implement the > > > en_<ENGINE_NODE_NAME>clear_tracked_data() function. > > > > > > With this patch, an engine node can store any changes done to the > > engine data > > > separately in the engine change handlers. The parent of this > > engine node > > > can use this tracked data for incrementally processing the > > changes. Upcoming > > > patches in the series will make use of this. > > > > > > Signed-off-by: Numan Siddique <[email protected] <mailto: > [email protected]>> > > > --- > > > lib/inc-proc-eng.c | 6 +++++- > > > lib/inc-proc-eng.h | 9 +++++++++ > > > 2 files changed, 14 insertions(+), 1 deletion(-) > > > > > > diff --git a/lib/inc-proc-eng.c b/lib/inc-proc-eng.c > > > index 9b1479a1c..a9389f75b 100644 > > > --- a/lib/inc-proc-eng.c > > > +++ b/lib/inc-proc-eng.c > > > @@ -260,6 +260,10 @@ engine_init_run(void) > > > VLOG_DBG("Initializing new run"); > > > for (size_t i = 0; i < engine_n_nodes; i++) { > > > engine_set_node_state(engine_nodes[i], EN_STALE); > > > + > > > + if (engine_nodes[i]->clear_tracked_data) { > > > + > > engine_nodes[i]->clear_tracked_data(engine_nodes[i]->data); > > > + } > > > } > > > } > > > > > > @@ -370,7 +374,7 @@ engine_run(bool recompute_allowed) > > > > > > if (engine_nodes[i]->state == EN_ABORTED) { > > > engine_run_aborted = true; > > > - return; > > > + break; > > > > I guess this is a leftover from a previous iteration of the series. > It > > should be fine to return here. > > > > > > Oops. Thanks. I'll fix it. > > > > Thanks > > Numan > > Hi Numan, > > While reviewing the following patches I realized that it might be useful > if in this patch we'd also call engine_nodes[i]->clear_tracked_data() in > engine_cleanup(), what do you think? > I think we can call. But since its part of engine data, engine data cleanup should free up any memory right ? Even if clear_tracked_data() is not called, engine_data_cleanup() should not leak the memory. The patch 6 in this series does that. Thanks Numan > Thanks, > Dumitru > > > > > > > > > With this fixed: > > > > Acked-by: Dumitru Ceara <[email protected] <mailto:[email protected] > >> > > > > Thanks, > > Dumitru > > > > > } > > > } > > > } > > > diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h > > > index 780c3cd22..552f2051a 100644 > > > --- a/lib/inc-proc-eng.h > > > +++ b/lib/inc-proc-eng.h > > > @@ -149,6 +149,10 @@ struct engine_node { > > > * doesn't store pointers to DB records it's still safe to > use). > > > */ > > > bool (*is_valid)(struct engine_node *); > > > + > > > + /* Method to clear up tracked data maintained by the engine > > node in the > > > + * engine 'data'. It may be NULL. */ > > > + void (*clear_tracked_data)(void *tracked_data); > > > }; > > > > > > /* Initialize the data for the engine nodes. It calls each node's > > > @@ -275,6 +279,7 @@ void engine_ovsdb_node_add_index(struct > > engine_node *, const char *name, > > > .run = en_##NAME##_run, \ > > > .cleanup = en_##NAME##_cleanup, \ > > > .is_valid = en_##NAME##_is_valid, \ > > > + .clear_tracked_data = NULL, \ > > > }; > > > > > > #define ENGINE_NODE_CUSTOM_DATA(NAME, NAME_STR) \ > > > @@ -284,6 +289,10 @@ void engine_ovsdb_node_add_index(struct > > engine_node *, const char *name, > > > static bool (*en_##NAME##_is_valid)(struct engine_node *node) > > = NULL; \ > > > ENGINE_NODE_DEF(NAME, NAME_STR) > > > > > > +#define ENGINE_NODE_WITH_CLEAR_TRACK_DATA(NAME, NAME_STR) \ > > > + ENGINE_NODE(NAME, NAME_STR) \ > > > + en_##NAME.clear_tracked_data = en_##NAME##_clear_tracked_data; > > > + > > > /* Macro to define member functions of an engine node which > > represents > > > * a table of OVSDB */ > > > #define ENGINE_FUNC_OVSDB(DB_NAME, TBL_NAME) \ > > > > > > > _______________________________________________ > > dev mailing list > > [email protected] <mailto:[email protected]> > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
