Thank you, that is a good idea. Here is a patch for it. I will add this to my series as well.
-8<--------------------------cut here-------------------------->8-- From: Ben Pfaff <[email protected]> Date: Thu, 6 May 2021 14:10:51 -0700 Subject: [PATCH ovn] configure: Check for correct ddlog version. Signed-off-by: Ben Pfaff <[email protected]> Suggested-by: Mark Michelson <[email protected]> --- acinclude.m4 | 20 +++++++++++++++++--- configure.ac | 2 +- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/acinclude.m4 b/acinclude.m4 index 5901d000af9b..30d108bd9b30 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -42,9 +42,14 @@ AC_DEFUN([OVS_ENABLE_WERROR], fi AC_SUBST([SPARSE_WERROR])]) -dnl OVS_CHECK_DDLOG +dnl OVS_CHECK_DDLOG([VERSION]) dnl -dnl Configure ddlog source tree +dnl Configure ddlog source tree, checking for the given DDlog VERSION. +dnl VERSION should be a major and minor, e.g. 0.36, which will accept +dnl 0.36.0, 0.36.1, and so on. Omit VERSION to accept any version of +dnl ddlog (which is probably only useful for developers who are trying +dnl different versions, since OVN is currently bound to a particular +dnl DDlog version). AC_DEFUN([OVS_CHECK_DDLOG], [ AC_ARG_WITH([ddlog], [AC_HELP_STRING([--with-ddlog=.../differential-datalog/lib], @@ -52,6 +57,7 @@ AC_DEFUN([OVS_CHECK_DDLOG], [ [DDLOGLIBDIR=$withval], [DDLOGLIBDIR=no]) AC_MSG_CHECKING([for DDlog library directory]) + AC_MSG_RESULT([$DDLOGLIBDIR]) if test "$DDLOGLIBDIR" != no; then if test ! -d "$DDLOGLIBDIR"; then AC_MSG_ERROR([ddlog library dir "$DDLOGLIBDIR" doesn't exist]) @@ -65,6 +71,15 @@ AC_DEFUN([OVS_CHECK_DDLOG], [ AC_MSG_ERROR([ddlog is required to build with DDlog]) fi + AC_MSG_CHECKING([$DDLOG version]) + $DDLOG --version >&AS_MESSAGE_LOG_FD 2>&1 + ddlog_version=$($DDLOG --version | sed -n 's/^DDlog v\([[^ ]]*\).*/\1/p') + AC_MSG_RESULT([$ddlog_version]) + m4_if([$1], [], [], [ + AS_CASE([$ddlog_version], + [$1 | $1.*], [], + [*], [AC_MSG_ERROR([DDlog version $1.x is required, but $ddlog_version is installed])])]) + AC_ARG_VAR([CARGO]) AC_CHECK_PROGS([CARGO], [cargo], [none]) if test X"$CARGO" = X"none"; then @@ -80,7 +95,6 @@ AC_DEFUN([OVS_CHECK_DDLOG], [ AC_SUBST([DDLOGLIBDIR]) AC_DEFINE([DDLOG], [1], [Build OVN daemons with ddlog.]) fi - AC_MSG_RESULT([$DDLOGLIBDIR]) AM_CONDITIONAL([DDLOG], [test "$DDLOGLIBDIR" != no]) ]) diff --git a/configure.ac b/configure.ac index ec5ee31df0da..02768699d3f1 100644 --- a/configure.ac +++ b/configure.ac @@ -169,7 +169,7 @@ OVS_CONDITIONAL_CC_OPTION([-Wno-unused-parameter], [HAVE_WNO_UNUSED_PARAMETER]) OVS_ENABLE_WERROR OVS_ENABLE_SPARSE -OVS_CHECK_DDLOG +OVS_CHECK_DDLOG([0.36]) OVS_CHECK_PRAGMA_MESSAGE OVN_CHECK_OVS OVS_CTAGS_IDENTIFIERS -- 2.31.1 On Fri, Apr 16, 2021 at 12:06:08PM -0400, Mark Michelson wrote: > Can we enhance the --with-ddlog check at configure time to check for the > expected version of DDlog? Othwerwise, if someone has the wrong version, > they'll see seemingly random compiler errors. > > On 4/1/21 7:20 PM, Ben Pfaff wrote: > > From: Leonid Ryzhyk <[email protected]> > > > > Upcoming commits will use a new --intern-table option of ovsdb2ddlog, > > so we need to upgrade to the version of ddlog that has that feature. > > > > To do so, we need to adapt the code to language changes in ddlog. This > > commit does that for a change in 0.37 in which, when iterating over a > > `Group` in a for-loop, the iterator returns `(value, weight)` tuples. > > > > This also adapts ovn-northd-ddlog.c to a slightly updated C API. > > > > Signed-off-by: Leonid Ryzhyk <[email protected]> > > Signed-off-by: Ben Pfaff <[email protected]> > > --- > > NEWS | 2 +- > > northd/lrouter.dl | 2 +- > > northd/ovn-northd-ddlog.c | 21 +++++++++++---------- > > 3 files changed, 13 insertions(+), 12 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index a98529ac4ebe..a75c44133b06 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -5,7 +5,7 @@ Post-v21.03.0 > > needed for the southbound database when northbound changes occur. It > > is > > expected to scale better than the C implementation, for large > > deployments. > > (This may take testing and tuning to be effective.) This version of > > OVN > > - requires DDLog 0.36. > > + requires DDLog 0.38. > > - Introduce ovn-controller incremetal processing engine statistics > > - Utilities: > > * ovn-nbctl daemon mode is no longer considered experimental. > > diff --git a/northd/lrouter.dl b/northd/lrouter.dl > > index 36cedd2dc219..e3afff72f41d 100644 > > --- a/northd/lrouter.dl > > +++ b/northd/lrouter.dl > > @@ -382,7 +382,7 @@ LogicalRouterSnatIP(lr, snat_ip, Some{nat}) :- > > function group_to_setunionmap(g: Group<'K1, ('K2,Set<'V>)>): > > Map<'K2,Set<'V>> { > > var map = map_empty(); > > - for (entry in g) { > > + for ((entry, _) in g) { > > (var key, var value) = entry; > > match (map.get(key)) { > > None -> map.insert(key, value), > > diff --git a/northd/ovn-northd-ddlog.c b/northd/ovn-northd-ddlog.c > > index ca1ab325448c..74f0eaccd5bb 100644 > > --- a/northd/ovn-northd-ddlog.c > > +++ b/northd/ovn-northd-ddlog.c > > @@ -79,10 +79,11 @@ static table_id WARNING_TABLE_ID; > > static table_id NB_CFG_TIMESTAMP_ID; > > /* Initialize frequently used table ids. */ > > -static void init_table_ids(void) > > +static void > > +init_table_ids(ddlog_prog ddlog) > > { > > - WARNING_TABLE_ID = ddlog_get_table_id("helpers::Warning"); > > - NB_CFG_TIMESTAMP_ID = ddlog_get_table_id("NbCfgTimestamp"); > > + WARNING_TABLE_ID = ddlog_get_table_id(ddlog, "helpers::Warning"); > > + NB_CFG_TIMESTAMP_ID = ddlog_get_table_id(ddlog, "NbCfgTimestamp"); > > } > > struct northd_ctx { > > @@ -347,7 +348,8 @@ ddlog_clear(struct northd_ctx *ctx) > > int n_failures = 0; > > for (int i = 0; ctx->input_relations[i]; i++) { > > char *table = xasprintf("%s%s", ctx->prefix, > > ctx->input_relations[i]); > > - if (ddlog_clear_relation(ctx->ddlog, ddlog_get_table_id(table))) { > > + if (ddlog_clear_relation(ctx->ddlog, ddlog_get_table_id(ctx->ddlog, > > + table))) { > > n_failures++; > > } > > free(table); > > @@ -611,7 +613,7 @@ northd_update_probe_interval(struct northd_ctx *nb, > > struct northd_ctx *sb) > > * Any other value is an explicit probe interval request from the > > * database. */ > > int probe_interval = 0; > > - table_id tid = ddlog_get_table_id("Northd_Probe_Interval"); > > + table_id tid = ddlog_get_table_id(nb->ddlog, "Northd_Probe_Interval"); > > ddlog_delta *probe_delta = ddlog_delta_remove_table(nb->delta, tid); > > ddlog_delta_enumerate(probe_delta, northd_update_probe_interval_cb, > > (uintptr_t) &probe_interval); > > ddlog_free_delta(probe_delta); > > @@ -670,7 +672,7 @@ ddlog_table_update_output(struct ds *ds, ddlog_prog > > ddlog, ddlog_delta *delta, > > return; > > } > > char *table_name = xasprintf("%s::Out_%s", db, table); > > - ddlog_delta_clear_table(delta, ddlog_get_table_id(table_name)); > > + ddlog_delta_clear_table(delta, ddlog_get_table_id(ddlog, table_name)); > > free(table_name); > > if (!updates[0]) { > > @@ -948,7 +950,7 @@ get_database_ops(struct northd_ctx *ctx) > > * We require output-only tables to have an accompanying index > > * named <table>_Index. */ > > char *index = xasprintf("%s_Index", table); > > - index_id idxid = ddlog_get_index_id(index); > > + index_id idxid = ddlog_get_index_id(ctx->ddlog, index); > > if (idxid == -1) { > > VLOG_WARN_RL(&rl, "%s: unknown index", index); > > free(index); > > @@ -1000,7 +1002,7 @@ get_database_ops(struct northd_ctx *ctx) > > static int64_t old_sb_cfg_timestamp = INT64_MIN; > > int64_t new_sb_cfg = old_sb_cfg; > > if (ctx->has_timestamp_columns) { > > - table_id sb_cfg_tid = ddlog_get_table_id("SbCfg"); > > + table_id sb_cfg_tid = ddlog_get_table_id(ctx->ddlog, "SbCfg"); > > ddlog_delta *sb_cfg_delta = ddlog_delta_remove_table(ctx->delta, > > sb_cfg_tid); > > ddlog_delta_enumerate(sb_cfg_delta, northd_update_sb_cfg_cb, > > @@ -1149,8 +1151,6 @@ main(int argc, char *argv[]) > > int retval; > > bool exiting; > > - init_table_ids(); > > - > > fatal_ignore_sigpipe(); > > ovs_cmdl_proctitle_init(argc, argv); > > set_program_name(argv[0]); > > @@ -1180,6 +1180,7 @@ main(int argc, char *argv[]) > > if (!ddlog) { > > ovs_fatal(0, "DDlog instance could not be created"); > > } > > + init_table_ids(ddlog); > > unixctl_command_register("enable-cpu-profiling", "", 0, 0, > > ovn_northd_enable_cpu_profiling, ddlog); > > > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
