| Message ID | 20211018121403.842185-2-mark.d.gray@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | northd: Introduce incremental processing framework | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Mon, Oct 18, 2021 at 5:14 AM Mark Gray <mark.d.gray@redhat.com> wrote: > > Refactor ENGINE_NODE() macro to not assign function pointers. This > allows ENGINE_NODE() to be used outside functions, creating an > engine_node with global scope (but can be statically defined within a file). > This allows more flexibility in how the I-P engine can be used as > engine nodes can be defined. Allow this is not explicitly required for > I-P it does allow for a cleaner code base as the node definitions can > be kept together outside any functions. > > Additional function pointers (e.g. is_valid(), clear_tracked_data()), > may be assigned later, if required. > > Signed-off-by: Mark Gray <mark.d.gray@redhat.com> > --- > controller/ovn-controller.c | 2 +- > lib/inc-proc-eng.h | 8 ++++---- > 2 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 4202f32ccaf6..48121ccce082 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -3219,7 +3219,7 @@ main(int argc, char *argv[]) > stopwatch_create(BFD_RUN_STOPWATCH_NAME, SW_MS); > > /* Define inc-proc-engine nodes. */ > - ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA(ct_zones, "ct_zones"); > + ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA_IS_VALID(ct_zones, "ct_zones"); > ENGINE_NODE_WITH_CLEAR_TRACK_DATA(runtime_data, "runtime_data"); > ENGINE_NODE(non_vif_data, "non_vif_data"); > ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve"); > diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h > index 859b30a71c86..1ccae559dff6 100644 > --- a/lib/inc-proc-eng.h > +++ b/lib/inc-proc-eng.h > @@ -295,19 +295,19 @@ void engine_ovsdb_node_add_index(struct engine_node *, const char *name, > .init = en_##NAME##_init, \ > .run = en_##NAME##_run, \ > .cleanup = en_##NAME##_cleanup, \ > - .is_valid = en_##NAME##_is_valid, \ > + .is_valid = NULL, \ > .clear_tracked_data = NULL, \ > }; > > #define ENGINE_NODE_CUSTOM_DATA(NAME, NAME_STR) \ Thanks Mark. It took me a while to understand the macro again. ENGINE_NODE_CUSTOM_DATA was supposed to be used when is_valid is needed. Now that it is not the case anymore, and it becomes exactly the same as ENGINE_NODE, it can be removed. > ENGINE_NODE_DEF(NAME, NAME_STR) > > -#define ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA(NAME, NAME_STR) \ > +#define ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA_IS_VALID(NAME, NAME_STR) \ CUSTOM and IS_VALID have the same meaning, so it may be simplified to ENGINE_NODE_WITH_CLEAR_TRACK_DATA_IS_VALID. > ENGINE_NODE_CUSTOM_DATA(NAME, NAME_STR) \ Here we can use ENGINE_NODE instead of ENGINE_NODE_CUSTOM_DATA. With this addressed: Acked-by: Han Zhou <hzhou@ovn.org> > - en_##NAME.clear_tracked_data = en_##NAME##_clear_tracked_data; > + en_##NAME.clear_tracked_data = en_##NAME##_clear_tracked_data; \ > + en_##NAME.is_valid = en_##NAME##_is_valid; > > #define ENGINE_NODE(NAME, NAME_STR) \ > - 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) \ > -- > 2.27.0 >
On 21/10/2021 06:24, Han Zhou wrote: > On Mon, Oct 18, 2021 at 5:14 AM Mark Gray <mark.d.gray@redhat.com> wrote: >> >> Refactor ENGINE_NODE() macro to not assign function pointers. This >> allows ENGINE_NODE() to be used outside functions, creating an >> engine_node with global scope (but can be statically defined within a > file). >> This allows more flexibility in how the I-P engine can be used as >> engine nodes can be defined. Allow this is not explicitly required for >> I-P it does allow for a cleaner code base as the node definitions can >> be kept together outside any functions. >> >> Additional function pointers (e.g. is_valid(), clear_tracked_data()), >> may be assigned later, if required. >> >> Signed-off-by: Mark Gray <mark.d.gray@redhat.com> >> --- >> controller/ovn-controller.c | 2 +- >> lib/inc-proc-eng.h | 8 ++++---- >> 2 files changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c >> index 4202f32ccaf6..48121ccce082 100644 >> --- a/controller/ovn-controller.c >> +++ b/controller/ovn-controller.c >> @@ -3219,7 +3219,7 @@ main(int argc, char *argv[]) >> stopwatch_create(BFD_RUN_STOPWATCH_NAME, SW_MS); >> >> /* Define inc-proc-engine nodes. */ >> - ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA(ct_zones, "ct_zones"); >> + ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA_IS_VALID(ct_zones, > "ct_zones"); >> ENGINE_NODE_WITH_CLEAR_TRACK_DATA(runtime_data, "runtime_data"); >> ENGINE_NODE(non_vif_data, "non_vif_data"); >> ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve"); >> diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h >> index 859b30a71c86..1ccae559dff6 100644 >> --- a/lib/inc-proc-eng.h >> +++ b/lib/inc-proc-eng.h >> @@ -295,19 +295,19 @@ void engine_ovsdb_node_add_index(struct engine_node > *, const char *name, >> .init = en_##NAME##_init, \ >> .run = en_##NAME##_run, \ >> .cleanup = en_##NAME##_cleanup, \ >> - .is_valid = en_##NAME##_is_valid, \ >> + .is_valid = NULL, \ >> .clear_tracked_data = NULL, \ >> }; >> >> #define ENGINE_NODE_CUSTOM_DATA(NAME, NAME_STR) \ > > Thanks Mark. It took me a while to understand the macro again. > ENGINE_NODE_CUSTOM_DATA was supposed to be used when is_valid is needed. > Now that it is not the case anymore, and it becomes exactly the same as > ENGINE_NODE, it can be removed. > >> ENGINE_NODE_DEF(NAME, NAME_STR) >> >> -#define ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA(NAME, NAME_STR) \ >> +#define ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA_IS_VALID(NAME, > NAME_STR) \ > > CUSTOM and IS_VALID have the same meaning, so it may be simplified to > ENGINE_NODE_WITH_CLEAR_TRACK_DATA_IS_VALID. > >> ENGINE_NODE_CUSTOM_DATA(NAME, NAME_STR) \ > > Here we can use ENGINE_NODE instead of ENGINE_NODE_CUSTOM_DATA. > > With this addressed: > Acked-by: Han Zhou <hzhou@ovn.org> All this makes sense. Thanks > >> - en_##NAME.clear_tracked_data = en_##NAME##_clear_tracked_data; >> + en_##NAME.clear_tracked_data = en_##NAME##_clear_tracked_data; \ >> + en_##NAME.is_valid = en_##NAME##_is_valid; >> >> #define ENGINE_NODE(NAME, NAME_STR) \ >> - 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) \ >> -- >> 2.27.0 >> >
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 4202f32ccaf6..48121ccce082 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -3219,7 +3219,7 @@ main(int argc, char *argv[]) stopwatch_create(BFD_RUN_STOPWATCH_NAME, SW_MS); /* Define inc-proc-engine nodes. */ - ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA(ct_zones, "ct_zones"); + ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA_IS_VALID(ct_zones, "ct_zones"); ENGINE_NODE_WITH_CLEAR_TRACK_DATA(runtime_data, "runtime_data"); ENGINE_NODE(non_vif_data, "non_vif_data"); ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve"); diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h index 859b30a71c86..1ccae559dff6 100644 --- a/lib/inc-proc-eng.h +++ b/lib/inc-proc-eng.h @@ -295,19 +295,19 @@ void engine_ovsdb_node_add_index(struct engine_node *, const char *name, .init = en_##NAME##_init, \ .run = en_##NAME##_run, \ .cleanup = en_##NAME##_cleanup, \ - .is_valid = en_##NAME##_is_valid, \ + .is_valid = NULL, \ .clear_tracked_data = NULL, \ }; #define ENGINE_NODE_CUSTOM_DATA(NAME, NAME_STR) \ ENGINE_NODE_DEF(NAME, NAME_STR) -#define ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA(NAME, NAME_STR) \ +#define ENGINE_NODE_CUSTOM_WITH_CLEAR_TRACK_DATA_IS_VALID(NAME, NAME_STR) \ ENGINE_NODE_CUSTOM_DATA(NAME, NAME_STR) \ - en_##NAME.clear_tracked_data = en_##NAME##_clear_tracked_data; + en_##NAME.clear_tracked_data = en_##NAME##_clear_tracked_data; \ + en_##NAME.is_valid = en_##NAME##_is_valid; #define ENGINE_NODE(NAME, NAME_STR) \ - 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) \
Refactor ENGINE_NODE() macro to not assign function pointers. This allows ENGINE_NODE() to be used outside functions, creating an engine_node with global scope (but can be statically defined within a file). This allows more flexibility in how the I-P engine can be used as engine nodes can be defined. Allow this is not explicitly required for I-P it does allow for a cleaner code base as the node definitions can be kept together outside any functions. Additional function pointers (e.g. is_valid(), clear_tracked_data()), may be assigned later, if required. Signed-off-by: Mark Gray <mark.d.gray@redhat.com> --- controller/ovn-controller.c | 2 +- lib/inc-proc-eng.h | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-)