In current OVN, an engine_node has two function callbacks that are
optional, is_valid() and clear_tracked_data(). There are individual
macros that allow for each of these optional functions to be defined in
the engine_node. With two optional callbacks, this means there are four
possible macros:
- vanilla engine node (no is_valid(), no clear_tracked_data())
- has clear_tracked_data(), does not have is_valid()
- has is_valid(), does not have clear_tracked_data()
- has both is_valid() and clear_tracked_data()
In OVN we have bespoke macros for the first, second, and fourth
scenarios [1].
The main issues with this are:
- This does not scale well if engine_nodes need new optional callbacks.
- The macro names are quite long.
This patch offers a new alternative. It alters the ENGINE_NODE macro so
that it can take a variable number of arguments. Some preprocessor magic
uses the number of arguments passed in to call a specific variant
(ENGINE_NODE2, ENGINE_NODE3, or ENGINE_NODE4 currently,
since these are the possible number of inputs to ENGINE_NODE). Depending
on the variant called, additional optional elements of the resulting
engine_node can be set. Convenience macros are provided for the two
optional callbacks.
Under the hood, there's a lot going on. But from the engine node
developer's perspective, it's much easier to declare engine nodes, since
there is only one macro to declare an engine node, and its arguments
help define which optional callbacks are defined.
[1] There is one engine_node (acl_id) that falls into the third
scenario but rather than use a custom macro, it just defines its
is_valid() callback after the initial engine_node definition.
Signed-off-by: Mark Michelson <mmich...@redhat.com>
Acked-by: Ales Musil <amu...@redhat.com>
---
v1 -> v2:
* Fixed checkpatch watching in ovn-controller.c
* Added Ales's ack
---
controller/ovn-controller.c | 23 +++++++++++-----------
lib/inc-proc-eng.h | 38 +++++++++++++++++++++++--------------
lib/ovn-util.h | 36 +++++++++++++++++++++++++++++++++++
northd/inc-proc-northd.c | 20 +++++++++----------
4 files changed, 81 insertions(+), 36 deletions(-)
diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index 4266d9a9c..43ac5b93c 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -5596,30 +5596,29 @@ main(int argc, char *argv[])
/* Define inc-proc-engine nodes. */
ENGINE_NODE(sb_ro, "sb_ro");
- ENGINE_NODE_WITH_CLEAR_TRACK_DATA(template_vars, "template_vars");
- ENGINE_NODE_WITH_CLEAR_TRACK_DATA_IS_VALID(ct_zones, "ct_zones");
- ENGINE_NODE_WITH_CLEAR_TRACK_DATA(ovs_interface_shadow,
- "ovs_interface_shadow");
- ENGINE_NODE_WITH_CLEAR_TRACK_DATA(runtime_data, "runtime_data");
+ ENGINE_NODE(template_vars, "template_vars", CLEAR_TRACKED_DATA);
+ ENGINE_NODE(ct_zones, "ct_zones", CLEAR_TRACKED_DATA, IS_VALID);
+ ENGINE_NODE(ovs_interface_shadow, "ovs_interface_shadow",
+ CLEAR_TRACKED_DATA);
+ ENGINE_NODE(runtime_data, "runtime_data", CLEAR_TRACKED_DATA);
ENGINE_NODE(non_vif_data, "non_vif_data");
ENGINE_NODE(mff_ovn_geneve, "mff_ovn_geneve");
ENGINE_NODE(ofctrl_is_connected, "ofctrl_is_connected");
- ENGINE_NODE_WITH_CLEAR_TRACK_DATA(activated_ports, "activated_ports");
+ ENGINE_NODE(activated_ports, "activated_ports", CLEAR_TRACKED_DATA);
ENGINE_NODE(postponed_ports, "postponed_ports");
ENGINE_NODE(pflow_output, "physical_flow_output");
- ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lflow_output, "logical_flow_output");
+ ENGINE_NODE(lflow_output, "logical_flow_output", CLEAR_TRACKED_DATA);
ENGINE_NODE(controller_output, "controller_output");
- ENGINE_NODE_WITH_CLEAR_TRACK_DATA(addr_sets, "addr_sets");
- ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_groups, "port_groups");
+ ENGINE_NODE(addr_sets, "addr_sets", CLEAR_TRACKED_DATA);
+ ENGINE_NODE(port_groups, "port_groups", CLEAR_TRACKED_DATA);
ENGINE_NODE(northd_options, "northd_options");
ENGINE_NODE(dhcp_options, "dhcp_options");
ENGINE_NODE(if_status_mgr, "if_status_mgr");
- ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lb_data, "lb_data");
+ ENGINE_NODE(lb_data, "lb_data", CLEAR_TRACKED_DATA);
ENGINE_NODE(mac_cache, "mac_cache");
ENGINE_NODE(bfd_chassis, "bfd_chassis");
ENGINE_NODE(dns_cache, "dns_cache");
- ENGINE_NODE(acl_id, "acl_id");
- en_acl_id.is_valid = en_acl_id_is_valid;
+ ENGINE_NODE(acl_id, "acl_id", IS_VALID);
ENGINE_NODE(route, "route");
ENGINE_NODE(route_table_notify, "route_table_notify");
ENGINE_NODE(route_exchange, "route_exchange");
diff --git a/lib/inc-proc-eng.h b/lib/inc-proc-eng.h
index 12e232020..e4c71d773 100644
--- a/lib/inc-proc-eng.h
+++ b/lib/inc-proc-eng.h
@@ -397,29 +397,39 @@ engine_noop_handler(struct engine_node *node OVS_UNUSED,
void *data OVS_UNUSED)
void engine_ovsdb_node_add_index(struct engine_node *, const char *name,
struct ovsdb_idl_index *);
-/* Macro to define an engine node. */
-#define ENGINE_NODE_DEF(NAME, NAME_STR, CLEAR_TRACKED_DATA, IS_VALID) \
+#define ENGINE_NODE_DEF_START(NAME, NAME_STR) \
struct engine_node en_##NAME = { \
.name = NAME_STR, \
.data = NULL, \
.state = EN_STALE, \
.init = en_##NAME##_init, \
.run = en_##NAME##_run, \
- .cleanup = en_##NAME##_cleanup, \
- .is_valid = IS_VALID, \
- .clear_tracked_data = CLEAR_TRACKED_DATA, \
- };
+ .cleanup = en_##NAME##_cleanup,
-#define ENGINE_NODE(NAME, NAME_STR) \
- ENGINE_NODE_DEF(NAME, NAME_STR, NULL, NULL)
+#define ENGINE_NODE_DEF_END };
-#define ENGINE_NODE_WITH_CLEAR_TRACK_DATA(NAME, NAME_STR) \
- ENGINE_NODE_DEF(NAME, NAME_STR, en_##NAME##_clear_tracked_data, NULL)
+#define ENGINE_NODE2(NAME, NAME_STR) \
+ ENGINE_NODE_DEF_START(NAME, NAME_STR) \
+ ENGINE_NODE_DEF_END
-#define ENGINE_NODE_WITH_CLEAR_TRACK_DATA_IS_VALID(NAME, NAME_STR) \
- ENGINE_NODE_DEF(NAME, NAME_STR, \
- en_##NAME##_clear_tracked_data, \
- en_##NAME##_is_valid)
+#define CLEAR_TRACKED_DATA(NAME) \
+ .clear_tracked_data = en_##NAME##_clear_tracked_data
+
+#define IS_VALID(NAME) \
+ .is_valid = en_##NAME##_is_valid
+
+#define ENGINE_NODE3(NAME, NAME_STR, ARG1) \
+ ENGINE_NODE_DEF_START(NAME, #NAME) \
+ ARG1(NAME), \
+ ENGINE_NODE_DEF_END
+
+#define ENGINE_NODE4(NAME, NAME_STR, ARG1, ARG2) \
+ ENGINE_NODE_DEF_START(NAME, #NAME) \
+ ARG1(NAME), \
+ ARG2(NAME), \
+ ENGINE_NODE_DEF_END
+
+#define ENGINE_NODE(...) VFUNC(ENGINE_NODE, __VA_ARGS__)
/* Macro to define member functions of an engine node which represents
* a table of OVSDB */
diff --git a/lib/ovn-util.h b/lib/ovn-util.h
index 0fff9b463..de15757a9 100644
--- a/lib/ovn-util.h
+++ b/lib/ovn-util.h
@@ -496,4 +496,40 @@ const struct sbrec_port_binding *lport_lookup_by_name(
struct ovsdb_idl_index *sbrec_port_binding_by_name,
const char *name);
+/* __NARG__ provides a way to count the number of arguments passed to a
+ * variadic macro. As defined below, it's capable of counting up to
+ * 16 arguments. This should be more than enough for our purposes. However
+ * the __ARG_N and __RSEQ_N macros can be updated to include more numbers
+ * if we wish to be able to count higher.
+ * */
+#define __NARG__(...) __NARG_I_(__VA_ARGS__,__RSEQ_N())
+#define __NARG_I_(...) __ARG_N(__VA_ARGS__)
+#define __ARG_N( \
+ _1, _2, _3, _4, _5, _6, _7, _8, _9, \
+ _10, _11, _12, _13, _14, _15, _16, N,...) N
+#define __RSEQ_N() \
+ 16, 15, 14, 13, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0
+
+/* VFUNC provides a way to overload macros so that they can
+ * accept a variable number of arguments.
+ *
+ * It works by using __NARG__ to count the number of arguments
+ * that were passed into the macro. Then this is concatenated
+ * to the end of the macro name. Then this concatenated macro
+ * is called.
+ *
+ * As an example, let's say we have
+ * # define FOO(...) VFUNC(FOO, __VA_ARGS)
+ *
+ * Then if a caller were to call FOO(bar)
+ * then we would count one argument (bar), concatenate
+ * the 1 to the end of FOO, and end up calling FOO1(bar).
+ *
+ * If a caller were to call FOO(bar, baz), then the
+ * result would be to call FOO2(bar, baz).
+ */
+#define _VFUNC_(name, n) name##n
+#define _VFUNC(name, n) _VFUNC_(name, n)
+#define VFUNC(func, ...) _VFUNC(func, __NARG__(__VA_ARGS__)) (__VA_ARGS__)
+
#endif /* OVN_UTIL_H */
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 7f92c0cb7..823593e7e 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -147,7 +147,7 @@ enum sb_engine_node {
/* Define engine nodes for other nodes. They should be defined as static to
* avoid sparse errors. */
-static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(northd, "northd");
+static ENGINE_NODE(northd, "northd", CLEAR_TRACKED_DATA);
static ENGINE_NODE(sync_from_sb, "sync_from_sb");
static ENGINE_NODE(sampling_app, "sampling_app");
static ENGINE_NODE(lflow, "lflow");
@@ -157,16 +157,16 @@ static ENGINE_NODE(northd_output, "northd_output");
static ENGINE_NODE(sync_meters, "sync_meters");
static ENGINE_NODE(sync_to_sb, "sync_to_sb");
static ENGINE_NODE(sync_to_sb_addr_set, "sync_to_sb_addr_set");
-static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_group, "port_group");
+static ENGINE_NODE(port_group, "port_group", CLEAR_TRACKED_DATA);
static ENGINE_NODE(fdb_aging, "fdb_aging");
static ENGINE_NODE(fdb_aging_waker, "fdb_aging_waker");
static ENGINE_NODE(sync_to_sb_lb, "sync_to_sb_lb");
static ENGINE_NODE(sync_to_sb_pb, "sync_to_sb_pb");
-static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(global_config, "global_config");
-static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lb_data, "lb_data");
-static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_nat, "lr_nat");
-static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(lr_stateful, "lr_stateful");
-static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(ls_stateful, "ls_stateful");
+static ENGINE_NODE(global_config, "global_config", CLEAR_TRACKED_DATA);
+static ENGINE_NODE(lb_data, "lb_data", CLEAR_TRACKED_DATA);
+static ENGINE_NODE(lr_nat, "lr_nat", CLEAR_TRACKED_DATA);
+static ENGINE_NODE(lr_stateful, "lr_stateful", CLEAR_TRACKED_DATA);
+static ENGINE_NODE(ls_stateful, "ls_stateful", CLEAR_TRACKED_DATA);
static ENGINE_NODE(route_policies, "route_policies");
static ENGINE_NODE(routes, "routes");
static ENGINE_NODE(bfd, "bfd");
@@ -175,10 +175,10 @@ static ENGINE_NODE(ecmp_nexthop, "ecmp_nexthop");
static ENGINE_NODE(multicast_igmp, "multicast_igmp");
static ENGINE_NODE(acl_id, "acl_id");
static ENGINE_NODE(advertised_route_sync, "advertised_route_sync");
-static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(learned_route_sync,
- "learned_route_sync");
+static ENGINE_NODE(learned_route_sync, "learned_route_sync",
+ CLEAR_TRACKED_DATA);
static ENGINE_NODE(dynamic_routes, "dynamic_routes");
-static ENGINE_NODE_WITH_CLEAR_TRACK_DATA(group_ecmp_route, "group_ecmp_route");
+static ENGINE_NODE(group_ecmp_route, "group_ecmp_route", CLEAR_TRACKED_DATA);
void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
struct ovsdb_idl_loop *sb)
--
2.47.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
