[ovs-dev,v4,5/6] northd: Add lflow node

Add an additional node that initially does nothing. This serves as a
template for how to add a new node. This node is inserted after
the northd_node.

This node will be updated in a later commit to generate logical
flows for the SBDB.

Signed-off-by: Mark Gray <mark.d.gray@redhat.com>
---
 northd/automake.mk       |  2 ++
 northd/en-lflow.c        | 53 ++++++++++++++++++++++++++++++++++++++++
 northd/en-lflow.h        | 16 ++++++++++++
 northd/inc-proc-northd.c |  5 +++-
 northd/inc-proc-northd.h |  2 --
 northd/northd.c          |  5 ----
 northd/ovn-northd.c      |  1 -
 7 files changed, 75 insertions(+), 9 deletions(-)
 create mode 100644 northd/en-lflow.c
 create mode 100644 northd/en-lflow.h

On Wed, Oct 27, 2021 at 10:24 AM Mark Gray <mark.d.gray@redhat.com> wrote:
>
> Add an additional node that initially does nothing. This serves as a
> template for how to add a new node. This node is inserted after
> the northd_node.
>
> This node will be updated in a later commit to generate logical
> flows for the SBDB.
>

Thanks Mark. As mentioned in the comment in the patch 4/6, this new node
should not access data that doesn't belong to its input defined in the I-P
engine. So with the comment for patch 4/6 addressed, this patch would need
to add its own input as dependencies in I-P engine, so that it can access
them, e.g. in build_lflows(). I understand that this node will be updated
later, but I think we need to make the input and output correctly reflected
when adding an engine node.

Thanks,
Han

> Signed-off-by: Mark Gray <mark.d.gray@redhat.com>
> ---
>  northd/automake.mk       |  2 ++
>  northd/en-lflow.c        | 53 ++++++++++++++++++++++++++++++++++++++++
>  northd/en-lflow.h        | 16 ++++++++++++
>  northd/inc-proc-northd.c |  5 +++-
>  northd/inc-proc-northd.h |  2 --
>  northd/northd.c          |  5 ----
>  northd/ovn-northd.c      |  1 -
>  7 files changed, 75 insertions(+), 9 deletions(-)
>  create mode 100644 northd/en-lflow.c
>  create mode 100644 northd/en-lflow.h
>
> diff --git a/northd/automake.mk b/northd/automake.mk
> index f0c1fb11c83a..4862ec7b7ff3 100644
> --- a/northd/automake.mk
> +++ b/northd/automake.mk
> @@ -6,6 +6,8 @@ northd_ovn_northd_SOURCES = \
>         northd/ovn-northd.c \
>         northd/en-northd.c \
>         northd/en-northd.h \
> +       northd/en-lflow.c \
> +       northd/en-lflow.h \
>         northd/inc-proc-northd.c \
>         northd/inc-proc-northd.h \
>         northd/ipam.c \
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> new file mode 100644
> index 000000000000..4cfb17834c7b
> --- /dev/null
> +++ b/northd/en-lflow.c
> @@ -0,0 +1,53 @@
> +/*
> + * Licensed under the Apache License, Version 2.0 (the "License");
> + * you may not use this file except in compliance with the License.
> + * You may obtain a copy of the License at:
> + *
> + *     http://www.apache.org/licenses/LICENSE-2.0
> + *
> + * Unless required by applicable law or agreed to in writing, software
> + * distributed under the License is distributed on an "AS IS" BASIS,
> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
implied.
> + * See the License for the specific language governing permissions and
> + * limitations under the License.
> + */
> +
> +#include <config.h>
> +
> +#include <getopt.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include "en-lflow.h"
> +#include "en-northd.h"
> +
> +#include "lib/inc-proc-eng.h"
> +#include "northd.h"
> +#include "stopwatch.h"
> +#include "lib/stopwatch-names.h"
> +#include "timeval.h"
> +#include "openvswitch/vlog.h"
> +
> +VLOG_DEFINE_THIS_MODULE(en_lflow);
> +
> +void en_lflow_run(struct engine_node *node, void *data OVS_UNUSED)
> +{
> +    const struct engine_context *eng_ctx = engine_get_context();
> +
> +    struct northd_data *northd_data = engine_get_input_data("northd",
node);
> +
> +    stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
> +    build_lflows(northd_data, eng_ctx->ovnsb_idl_txn);
> +    stopwatch_stop(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
> +
> +    engine_set_node_state(node, EN_UPDATED);
> +}
> +void *en_lflow_init(struct engine_node *node OVS_UNUSED,
> +                     struct engine_arg *arg OVS_UNUSED)
> +{
> +    return NULL;
> +}
> +
> +void en_lflow_cleanup(void *data OVS_UNUSED)
> +{
> +}
> diff --git a/northd/en-lflow.h b/northd/en-lflow.h
> new file mode 100644
> index 000000000000..0e4d522ff3fe
> --- /dev/null
> +++ b/northd/en-lflow.h
> @@ -0,0 +1,16 @@
> +#ifndef EN_LFLOW_H
> +#define EN_LFLOW_H 1
> +
> +#include <config.h>
> +
> +#include <getopt.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +
> +#include "lib/inc-proc-eng.h"
> +
> +void en_lflow_run(struct engine_node *node, void *data);
> +void *en_lflow_init(struct engine_node *node, struct engine_arg *arg);
> +void en_lflow_cleanup(void *data);
> +
> +#endif /* EN_LFLOW_H */
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index d2da0489cc1c..56c05a0fd6f3 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -28,6 +28,7 @@
>  #include "openvswitch/vlog.h"
>  #include "inc-proc-northd.h"
>  #include "en-northd.h"
> +#include "en-lflow.h"
>  #include "util.h"
>
>  VLOG_DEFINE_THIS_MODULE(inc_proc_northd);
> @@ -143,6 +144,7 @@ enum sb_engine_node {
>  /* Define engine nodes for other nodes. They should be defined as static
to
>   * avoid sparse errors. */
>  static ENGINE_NODE(northd, "northd");
> +static ENGINE_NODE(lflow, "lflow");
>
>  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>                            struct ovsdb_idl_loop *sb)
> @@ -207,6 +209,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>      engine_add_input(&en_northd, &en_sb_load_balancer, NULL);
>      engine_add_input(&en_northd, &en_sb_bfd, NULL);
>      engine_add_input(&en_northd, &en_sb_fdb, NULL);
> +    engine_add_input(&en_lflow, &en_northd, NULL);
>
>      struct engine_arg engine_arg = {
>          .nb_idl = nb->idl,
> @@ -224,7 +227,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>      struct ovsdb_idl_index *sbrec_chassis_by_hostname =
>          chassis_hostname_index_create(sb->idl);
>
> -    engine_init(&en_northd, &engine_arg);
> +    engine_init(&en_lflow, &engine_arg);
>
>      engine_ovsdb_node_add_index(&en_sb_chassis,
>                                  "sbrec_chassis_by_name",
> diff --git a/northd/inc-proc-northd.h b/northd/inc-proc-northd.h
> index b6c38e68749d..4aeb387b7b0f 100644
> --- a/northd/inc-proc-northd.h
> +++ b/northd/inc-proc-northd.h
> @@ -6,8 +6,6 @@
>  #include "northd.h"
>  #include "ovsdb-idl.h"
>
> -
> -
>  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>                            struct ovsdb_idl_loop *sb);
>  void inc_proc_northd_run(struct ovsdb_idl_txn *ovnnb_txn,
> diff --git a/northd/northd.c b/northd/northd.c
> index 9649d94772ad..7bccd054cdb4 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -13031,7 +13031,6 @@ struct lflows_thread_pool {
>      struct worker_pool *pool;
>  };
>
> -
>  static void *
>  build_lflows_thread(void *arg)
>  {
> @@ -14613,9 +14612,6 @@ ovnnb_db_run(struct northd_data *data,
>      build_meter_groups(data, &data->meter_groups);
>      build_bfd_table(data, ovnsb_txn, &data->bfd_connections,
&data->ports);
>      stopwatch_stop(BUILD_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
> -    stopwatch_start(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
> -    build_lflows(data, ovnsb_txn);
> -    stopwatch_stop(BUILD_LFLOWS_STOPWATCH_NAME, time_msec());
>      stopwatch_start(CLEAR_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
>      ovn_update_ipv6_prefix(&data->ports);
>
> @@ -14626,7 +14622,6 @@ ovnnb_db_run(struct northd_data *data,
>      cleanup_stale_fdp_entries(data, &data->datapaths);
>      bfd_cleanup_connections(data, &data->bfd_connections);
>      stopwatch_stop(CLEAR_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
> -
>  }
>
>  /* Stores the list of chassis which references an ha_chassis_group.
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index e291e26872ec..cce757fb57d9 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -1007,7 +1007,6 @@ main(int argc, char *argv[])
>                      check_and_update_rbac(
>                                   ovnsb_txn, ovnsb_idl_loop.idl);
>                  }
> -
>              }
>          } else {
>              /* ovn-northd is paused
> --
> 2.27.0
>