On Thu, Jan 16, 2025 at 10:07 AM Lorenzo Bianconi <
[email protected]> wrote:

> > Add new I-P node that will store all the data for IGMP and
> > Multicast groups. This node allows to avoid full recompute of lflow
> > node when IGMP or Multicast SB table changes.
> >
> > The node itself still does full recompute for IGMP and Multicast
> > changes however this is a compromise between code complexity and
> > the time it takes for all lflow to be created. At the same time
> > thi brings the benefit of having the data available when there
> > is recompute of the lflow node.
> >
> > As design choice there is only single lflow_ref for all IGMP
> > lflows, that makes them not being thread safe and only main thread
> > can generate them during full recompute of lflow node. This shouldn't
> > be an issue, because the computation of igmp lflow is pretty simple.
> >
> > Reported-at: https://issues.redhat.com/browse/FDP-756
> > Co-authored-by: Jacob Tanenbaum <[email protected]>
> > Signed-off-by: Jacob Tanenbaum <[email protected]>
> > Suggested-by: Dumitru Ceara <[email protected]>
> > Signed-off-by: Ales Musil <[email protected]>
>
> Hi Ales,
>
> my only concern about this patch is moving the lflow code just in the main
> thread, but after internal discussion I guess it is fine since we can
> create
> just a limited number of IGMP group. Can you please add a comment
> specifying
> it explicitly?
> Moreover, I guess we can split this patch in two, separating the IP
> codebase
> from the one that manages the lflows, what do you think? (in this way it is
> easier to review). Other than that:
>
> Acked-by: Lorenzo Bianconi <[email protected]>
>
> Regards,
> Lorenzo
>

Hi Lorenzo,

thank you for the review. I have adjusted the comment and
tried to split it into two patches. Please take a look at v2.

Thanks,
Ales

>
> > ---
> >  northd/en-lflow.c        |  52 ++++++-
> >  northd/en-lflow.h        |   1 +
> >  northd/en-multicast.c    | 289 +++++++++++++++++++++++++--------------
> >  northd/en-multicast.h    |  25 ++--
> >  northd/inc-proc-northd.c |  10 +-
> >  northd/northd.c          | 239 ++++++++++++++++----------------
> >  northd/northd.h          |  12 +-
> >  tests/ovn-northd.at      |  89 ++++++++++++
> >  8 files changed, 472 insertions(+), 245 deletions(-)
> >
> > diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> > index fa1f0236d..e2816f4da 100644
> > --- a/northd/en-lflow.c
> > +++ b/northd/en-lflow.c
> > @@ -23,6 +23,7 @@
> >  #include "en-lr-nat.h"
> >  #include "en-lr-stateful.h"
> >  #include "en-ls-stateful.h"
> > +#include "en-multicast.h"
> >  #include "en-northd.h"
> >  #include "en-meters.h"
> >  #include "en-sampling-app.h"
> > @@ -56,13 +57,11 @@ lflow_get_input_data(struct engine_node *node,
> >          engine_get_input_data("lr_stateful", node);
> >      struct ed_type_ls_stateful *ls_stateful_data =
> >          engine_get_input_data("ls_stateful", node);
> > +    struct multicast_igmp_data *multicat_igmp_data =
> > +        engine_get_input_data("multicast_igmp", node);
> >
> >      lflow_input->sbrec_logical_flow_table =
> >          EN_OVSDB_GET(engine_get_input("SB_logical_flow", node));
> > -    lflow_input->sbrec_multicast_group_table =
> > -        EN_OVSDB_GET(engine_get_input("SB_multicast_group", node));
> > -    lflow_input->sbrec_igmp_group_table =
> > -        EN_OVSDB_GET(engine_get_input("SB_igmp_group", node));
> >      lflow_input->sbrec_logical_dp_group_table =
> >          EN_OVSDB_GET(engine_get_input("SB_logical_dp_group", node));
> >
> > @@ -85,6 +84,8 @@ lflow_get_input_data(struct engine_node *node,
> >      lflow_input->parsed_routes = &routes_data->parsed_routes;
> >      lflow_input->route_tables = &routes_data->route_tables;
> >      lflow_input->route_policies = &route_policies_data->route_policies;
> > +    lflow_input->igmp_groups = &multicat_igmp_data->igmp_groups;
> > +    lflow_input->igmp_lflow_ref = multicat_igmp_data->lflow_ref;
> >
> >      struct ed_type_global_config *global_config =
> >          engine_get_input_data("global_config", node);
> > @@ -110,6 +111,7 @@ void en_lflow_run(struct engine_node *node, void
> *data)
> >      struct lflow_data *lflow_data = data;
> >      lflow_table_clear(lflow_data->lflow_table);
> >      lflow_reset_northd_refs(&lflow_input);
> > +    lflow_ref_clear(lflow_input.igmp_lflow_ref);
> >
> >      build_lflows(eng_ctx->ovnsb_idl_txn, &lflow_input,
> >                   lflow_data->lflow_table);
> > @@ -219,6 +221,48 @@ lflow_ls_stateful_handler(struct engine_node *node,
> void *data)
> >      return true;
> >  }
> >
> > +bool
> > +lflow_multicast_igmp_handler(struct engine_node *node, void *data)
> > +{
> > +    struct multicast_igmp_data *mcast_igmp_data =
> > +        engine_get_input_data("multicast_igmp", node);
> > +
> > +    const struct engine_context *eng_ctx = engine_get_context();
> > +    struct lflow_data *lflow_data = data;
> > +    struct lflow_input lflow_input;
> > +    lflow_get_input_data(node, &lflow_input);
> > +
> > +    if (!lflow_ref_resync_flows(mcast_igmp_data->lflow_ref,
> > +                                lflow_data->lflow_table,
> > +                                eng_ctx->ovnsb_idl_txn,
> > +                                lflow_input.ls_datapaths,
> > +                                lflow_input.lr_datapaths,
> > +
> lflow_input.ovn_internal_version_changed,
> > +                                lflow_input.sbrec_logical_flow_table,
> > +
> lflow_input.sbrec_logical_dp_group_table)) {
> > +        return false;
> > +    }
> > +
> > +    build_igmp_lflows(&mcast_igmp_data->igmp_groups,
> > +                      &lflow_input.ls_datapaths->datapaths,
> > +                      lflow_data->lflow_table,
> > +                      mcast_igmp_data->lflow_ref);
> > +
> > +    if (!lflow_ref_sync_lflows(mcast_igmp_data->lflow_ref,
> > +                               lflow_data->lflow_table,
> > +                               eng_ctx->ovnsb_idl_txn,
> > +                               lflow_input.ls_datapaths,
> > +                               lflow_input.lr_datapaths,
> > +                               lflow_input.ovn_internal_version_changed,
> > +                               lflow_input.sbrec_logical_flow_table,
> > +
>  lflow_input.sbrec_logical_dp_group_table)) {
> > +        return false;
> > +    }
> > +
> > +    engine_set_node_state(node, EN_UPDATED);
> > +    return true;
> > +}
> > +
> >  void *en_lflow_init(struct engine_node *node OVS_UNUSED,
> >                       struct engine_arg *arg OVS_UNUSED)
> >  {
> > diff --git a/northd/en-lflow.h b/northd/en-lflow.h
> > index 32cae6176..f90f5c61c 100644
> > --- a/northd/en-lflow.h
> > +++ b/northd/en-lflow.h
> > @@ -22,5 +22,6 @@ bool lflow_northd_handler(struct engine_node *, void
> *data);
> >  bool lflow_port_group_handler(struct engine_node *, void *data);
> >  bool lflow_lr_stateful_handler(struct engine_node *, void *data);
> >  bool lflow_ls_stateful_handler(struct engine_node *node, void *data);
> > +bool lflow_multicast_igmp_handler(struct engine_node *node, void *data);
> >
> >  #endif /* EN_LFLOW_H */
> > diff --git a/northd/en-multicast.c b/northd/en-multicast.c
> > index deb192a82..59a36f38b 100644
> > --- a/northd/en-multicast.c
> > +++ b/northd/en-multicast.c
> > @@ -22,6 +22,7 @@
> >
> >  /* OVN includes. */
> >  #include "en-multicast.h"
> > +#include "lflow-mgr.h"
> >  #include "lib/ip-mcast-index.h"
> >  #include "lib/mcast-group-index.h"
> >  #include "lib/ovn-l7.h"
> > @@ -47,6 +48,16 @@ static const struct multicast_group mc_unknown =
> >  static const struct multicast_group mc_flood_l2 =
> >      { MC_FLOOD_L2, OVN_MCAST_FLOOD_L2_TUNNEL_KEY };
> >
> > +static void build_mcast_groups(
> > +    struct multicast_igmp_data *, const struct sbrec_igmp_group_table *,
> > +    struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp,
> > +    const struct hmap *ls_datapaths, const struct hmap *ls_ports,
> > +    const struct hmap *lr_ports);
> > +static void sync_multicast_groups_to_sb(
> > +    struct multicast_igmp_data *, struct ovsdb_idl_txn *,
> > +    const struct sbrec_multicast_group_table *,
> > +    const struct hmap * ls_datapaths, const struct hmap *lr_datapaths);
> > +
> >  static bool multicast_group_equal(const struct multicast_group *,
> >                                    const struct multicast_group *);
> >  static uint32_t ovn_multicast_hash(const struct ovn_datapath *,
> > @@ -65,6 +76,7 @@ static void ovn_multicast_destroy(struct hmap
> *mcgroups,
> >                                    struct ovn_multicast *);
> >  static void ovn_multicast_update_sbrec(const struct ovn_multicast *,
> >                                         const struct
> sbrec_multicast_group *);
> > +static void ovn_multicast_groups_destroy(struct hmap *mcast_groups);
> >
> >  static uint32_t ovn_igmp_group_hash(const struct ovn_datapath *,
> >                                      const struct in6_addr *);
> > @@ -88,29 +100,123 @@ static void ovn_igmp_group_aggregate_ports(struct
> ovn_igmp_group *,
> >                                             struct hmap *mcast_groups);
> >  static void ovn_igmp_group_destroy(struct hmap *igmp_groups,
> >                                     struct ovn_igmp_group *);
> > +static void ovn_igmp_groups_destroy(struct hmap *igmp_groups);
> > +
> > +void *
> > +en_multicast_igmp_init(struct engine_node *node OVS_UNUSED,
> > +                       struct engine_arg *arg OVS_UNUSED)
> > +{
> > +    struct multicast_igmp_data *data =xmalloc(sizeof *data);
> > +    hmap_init(&data->mcast_groups);
> > +    hmap_init(&data->igmp_groups);
> > +    data->lflow_ref = lflow_ref_create();
> > +
> > +    return data;
> > +}
> >
> >  void
> > -build_mcast_groups(const struct sbrec_igmp_group_table
> *sbrec_igmp_group_table,
> > +en_multicast_igmp_run(struct engine_node *node, void *data_)
> > +{
> > +    struct multicast_igmp_data *data = data_;
> > +    struct northd_data *northd_data = engine_get_input_data("northd",
> node);
> > +    const struct sbrec_igmp_group_table *sbrec_igmp_group_table =
> > +        EN_OVSDB_GET(engine_get_input("SB_igmp_group", node));
> > +    const struct sbrec_multicast_group_table
> *sbrec_multicast_group_table =
> > +        EN_OVSDB_GET(engine_get_input("SB_multicast_group", node));
> > +    struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp =
> > +        engine_ovsdb_node_get_index(
> > +            engine_get_input("SB_multicast_group", node),
> > +            "sbrec_mcast_group_by_name");
> > +    const struct engine_context *eng_ctx = engine_get_context();
> > +
> > +    ovn_multicast_groups_destroy(&data->mcast_groups);
> > +    ovn_igmp_groups_destroy(&data->igmp_groups);
> > +
> > +    build_mcast_groups(data, sbrec_igmp_group_table,
> > +                      sbrec_mcast_group_by_name_dp,
> > +                      &northd_data->ls_datapaths.datapaths,
> > +                      &northd_data->ls_ports,
> > +                      &northd_data->lr_ports);
> > +    sync_multicast_groups_to_sb(data, eng_ctx->ovnsb_idl_txn,
> > +                                sbrec_multicast_group_table,
> > +                                &northd_data->ls_datapaths.datapaths,
> > +                                &northd_data->lr_datapaths.datapaths);
> > +
> > +    engine_set_node_state(node, EN_UPDATED);
> > +}
> > +
> > +bool
> > +multicast_igmp_northd_handler(struct engine_node *node, void *data
> OVS_UNUSED)
> > +{
> > +    struct northd_data *northd_data = engine_get_input_data("northd",
> node);
> > +    if (!northd_has_tracked_data(&northd_data->trk_data)) {
> > +        return false;
> > +    }
> > +
> > +    /* This node uses the below data from the en_northd engine node.
> > +     *      - northd_data->lr_datapaths
> > +     *      - northd_data->ls_ports
> > +     *      - northd_data->lr_ports
> > +     *
> > +     *      This data gets updated when a logical router is created or
> deleted.
> > +     *      northd engine node presently falls back to full recompute
> when
> > +     *      this happens and so does this node.
> > +     *      Note: When we add I-P to the created/deleted logical
> routers, we
> > +     *      need to revisit this handler.
> > +     *
> > +     *      This node also accesses the router ports of the logical
> router
> > +     *      (od->ports).  When these logical router ports gets updated,
> > +     *      en_northd engine recomputes and so does this node.
> > +     *      Note: When we add I-P to handle switch/router port changes,
> we
> > +     *      need to revisit this handler.
> > +     *
> > +     * */
> > +    return true;
> > +}
> > +
> > +void
> > +en_multicast_igmp_cleanup(void *data_)
> > +{
> > +    struct multicast_igmp_data *data = data_;
> > +
> > +    ovn_multicast_groups_destroy(&data->mcast_groups);
> > +    ovn_igmp_groups_destroy(&data->igmp_groups);
> > +    hmap_destroy(&data->mcast_groups);
> > +    hmap_destroy(&data->igmp_groups);
> > +    lflow_ref_destroy(data->lflow_ref);
> > +}
> > +
> > +struct sbrec_multicast_group *
> > +create_sb_multicast_group(struct ovsdb_idl_txn *ovnsb_txn,
> > +                          const struct sbrec_datapath_binding *dp,
> > +                          const char *name,
> > +                          int64_t tunnel_key)
> > +{
> > +    struct sbrec_multicast_group *sbmc =
> > +        sbrec_multicast_group_insert(ovnsb_txn);
> > +    sbrec_multicast_group_set_datapath(sbmc, dp);
> > +    sbrec_multicast_group_set_name(sbmc, name);
> > +    sbrec_multicast_group_set_tunnel_key(sbmc, tunnel_key);
> > +    return sbmc;
> > +}
> > +
> > +static void
> > +build_mcast_groups(struct multicast_igmp_data *data,
> > +                   const struct sbrec_igmp_group_table
> *sbrec_igmp_group_table,
> >                     struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp,
> >                     const struct hmap *ls_datapaths,
> >                     const struct hmap *ls_ports,
> > -                   const struct hmap *lr_ports,
> > -                   struct hmap *mcast_groups,
> > -                   struct hmap *igmp_groups)
> > -{
> > +                   const struct hmap *lr_ports) {
> >      struct ovn_datapath *od;
> >      struct ovn_port *op;
> >
> > -    hmap_init(mcast_groups);
> > -    hmap_init(igmp_groups);
> > -
> >      HMAP_FOR_EACH (op, key_node, lr_ports) {
> >          if (lrport_is_enabled(op->nbrp)) {
> >              /* If this port is configured to always flood multicast
> traffic
> >               * add it to the MC_STATIC group.
> >               */
> >              if (op->mcast_info.flood) {
> > -                ovn_multicast_add(mcast_groups, &mc_static, op);
> > +                ovn_multicast_add(&data->mcast_groups, &mc_static, op);
> >                  op->od->mcast_info.rtr.flood_static = true;
> >              }
> >          }
> > @@ -118,14 +224,14 @@ build_mcast_groups(const struct
> sbrec_igmp_group_table *sbrec_igmp_group_table,
> >
> >      HMAP_FOR_EACH (op, key_node, ls_ports) {
> >          if (lsp_is_enabled(op->nbsp)) {
> > -            ovn_multicast_add(mcast_groups, &mc_flood, op);
> > +            ovn_multicast_add(&data->mcast_groups, &mc_flood, op);
> >
> >              if (!lsp_is_router(op->nbsp)) {
> > -                ovn_multicast_add(mcast_groups, &mc_flood_l2, op);
> > +                ovn_multicast_add(&data->mcast_groups, &mc_flood_l2,
> op);
> >              }
> >
> >              if (op->has_unknown) {
> > -                ovn_multicast_add(mcast_groups, &mc_unknown, op);
> > +                ovn_multicast_add(&data->mcast_groups, &mc_unknown, op);
> >              }
> >
> >              /* If this port is connected to a multicast router then add
> it
> > @@ -133,7 +239,7 @@ build_mcast_groups(const struct
> sbrec_igmp_group_table *sbrec_igmp_group_table,
> >               */
> >              if (op->od->mcast_info.sw.flood_relay && op->peer &&
> >                  op->peer->od && op->peer->od->mcast_info.rtr.relay) {
> > -                ovn_multicast_add(mcast_groups, &mc_mrouter_flood, op);
> > +                ovn_multicast_add(&data->mcast_groups,
> &mc_mrouter_flood, op);
> >              }
> >
> >              /* If this port is configured to always flood multicast
> reports
> > @@ -141,7 +247,7 @@ build_mcast_groups(const struct
> sbrec_igmp_group_table *sbrec_igmp_group_table,
> >               * flooded to statically configured or learned mrouters).
> >               */
> >              if (op->mcast_info.flood_reports) {
> > -                ovn_multicast_add(mcast_groups, &mc_mrouter_flood, op);
> > +                ovn_multicast_add(&data->mcast_groups,
> &mc_mrouter_flood, op);
> >                  op->od->mcast_info.sw.flood_reports = true;
> >              }
> >
> > @@ -149,7 +255,7 @@ build_mcast_groups(const struct
> sbrec_igmp_group_table *sbrec_igmp_group_table,
> >               * add it to the MC_STATIC group.
> >               */
> >              if (op->mcast_info.flood) {
> > -                ovn_multicast_add(mcast_groups, &mc_static, op);
> > +                ovn_multicast_add(&data->mcast_groups, &mc_static, op);
> >                  op->od->mcast_info.sw.flood_static = true;
> >              }
> >          }
> > @@ -202,23 +308,30 @@ build_mcast_groups(const struct
> sbrec_igmp_group_table *sbrec_igmp_group_table,
> >           * if the multicast group already exists.
> >           */
> >          struct ovn_igmp_group *igmp_group =
> > -            ovn_igmp_group_add(sbrec_mcast_group_by_name_dp,
> igmp_groups, od,
> > +            ovn_igmp_group_add(sbrec_mcast_group_by_name_dp,
> > +                               &data->igmp_groups, od,
> >                                 &group_address, sb_igmp->address);
> >
> >          /* Add the extracted ports to the IGMP group. */
> >          ovn_igmp_group_add_entry(igmp_group, igmp_ports, n_igmp_ports);
> > -    }
> >
> > -    /* Build IGMP groups for multicast routers with relay enabled. The
> router
> > -     * IGMP groups are based on the groups learnt by their multicast
> enabled
> > -     * peers.
> > -     */
> > -    HMAP_FOR_EACH (od, key_node, ls_datapaths) {
> > +        /* Skip mrouter entries. */
> > +        if (!strcmp(igmp_group->mcgroup.name,
> OVN_IGMP_GROUP_MROUTERS)) {
> > +            continue;
> > +        }
> >
> > -        if (ovs_list_is_empty(&od->mcast_info.groups)) {
> > +        /* For IPv6 only relay routable multicast groups
> > +         * (RFC 4291 2.7).
> > +         */
> > +        if (!IN6_IS_ADDR_V4MAPPED(&group_address) &&
> > +            !ipv6_addr_is_routable_multicast(&group_address)) {
> >              continue;
> >          }
> >
> > +        /* Build IGMP groups for multicast routers with relay enabled.
> > +         * The router IGMP groups are based on the groups learnt by
> their
> > +         * multicast enabled peers.
> > +         */
> >          for (size_t i = 0; i < od->n_router_ports; i++) {
> >              struct ovn_port *router_port = od->router_ports[i]->peer;
> >
> > @@ -232,38 +345,19 @@ build_mcast_groups(const struct
> sbrec_igmp_group_table *sbrec_igmp_group_table,
> >                  continue;
> >              }
> >
> > -            struct ovn_igmp_group *igmp_group;
> > -            LIST_FOR_EACH (igmp_group, list_node,
> &od->mcast_info.groups) {
> > -                struct in6_addr *address = &igmp_group->address;
> > -
> > -                /* Skip mrouter entries. */
> > -                if (!strcmp(igmp_group->mcgroup.name,
> > -                            OVN_IGMP_GROUP_MROUTERS)) {
> > -                    continue;
> > -                }
> > -
> > -                /* For IPv6 only relay routable multicast groups
> > -                 * (RFC 4291 2.7).
> > -                 */
> > -                if (!IN6_IS_ADDR_V4MAPPED(address) &&
> > -                    !ipv6_addr_is_routable_multicast(address)) {
> > -                    continue;
> > -                }
> > -
> > -                struct ovn_igmp_group *igmp_group_rtr =
> > -                    ovn_igmp_group_add(sbrec_mcast_group_by_name_dp,
> > -                                       igmp_groups, router_port->od,
> > -                                       address, igmp_group->
> mcgroup.name);
> > -                struct ovn_port **router_igmp_ports =
> > -                    xmalloc(sizeof *router_igmp_ports);
> > -                /* Store the chassis redirect port  otherwise traffic
> will not
> > -                 * be tunneled properly.
> > -                 */
> > -                router_igmp_ports[0] = router_port->cr_port
> > -                                       ? router_port->cr_port
> > -                                       : router_port;
> > -                ovn_igmp_group_add_entry(igmp_group_rtr,
> router_igmp_ports, 1);
> > -            }
> > +            struct ovn_igmp_group *igmp_group_rtr =
> > +                ovn_igmp_group_add(sbrec_mcast_group_by_name_dp,
> > +                                   &data->igmp_groups, router_port->od,
> > +                                   &group_address, igmp_group->
> mcgroup.name);
> > +            struct ovn_port **router_igmp_ports =
> > +                xmalloc(sizeof *router_igmp_ports);
> > +            /* Store the chassis redirect port  otherwise traffic will
> not
> > +             * be tunneled properly.
> > +             */
> > +            router_igmp_ports[0] = router_port->cr_port
> > +                                   ? router_port->cr_port
> > +                                   : router_port;
> > +            ovn_igmp_group_add_entry(igmp_group_rtr, router_igmp_ports,
> 1);
> >          }
> >      }
> >
> > @@ -273,41 +367,41 @@ build_mcast_groups(const struct
> sbrec_igmp_group_table *sbrec_igmp_group_table,
> >       * explicitly.
> >       */
> >      struct ovn_igmp_group *igmp_group;
> > -    HMAP_FOR_EACH_SAFE (igmp_group, hmap_node, igmp_groups) {
> > +    HMAP_FOR_EACH_SAFE (igmp_group, hmap_node, &data->igmp_groups) {
> >
> >          /* If this is a mrouter entry just aggregate the mrouter ports
> >           * into the MC_MROUTER mcast_group and destroy the igmp_group;
> >           * no more processing needed. */
> >          if (!strcmp(igmp_group->mcgroup.name,
> OVN_IGMP_GROUP_MROUTERS)) {
> > -            ovn_igmp_mrouter_aggregate_ports(igmp_group, mcast_groups);
> > -            ovn_igmp_group_destroy(igmp_groups, igmp_group);
> > +            ovn_igmp_mrouter_aggregate_ports(igmp_group,
> &data->mcast_groups);
> > +            ovn_igmp_group_destroy(&data->igmp_groups, igmp_group);
> >              continue;
> >          }
> >
> >          if (!ovn_igmp_group_allocate_id(igmp_group)) {
> >              /* If we ran out of keys just destroy the entry. */
> > -            ovn_igmp_group_destroy(igmp_groups, igmp_group);
> > +            ovn_igmp_group_destroy(&data->igmp_groups, igmp_group);
> >              continue;
> >          }
> >
> >          /* Aggregate the ports from all entries corresponding to this
> >           * group.
> >           */
> > -        ovn_igmp_group_aggregate_ports(igmp_group, mcast_groups);
> > +        ovn_igmp_group_aggregate_ports(igmp_group, &data->mcast_groups);
> >      }
> >  }
> >
> > -void
> > +static void
> >  sync_multicast_groups_to_sb(
> > -    struct ovsdb_idl_txn *ovnsb_txn,
> > +    struct multicast_igmp_data *data, struct ovsdb_idl_txn *ovnsb_txn,
> >      const struct sbrec_multicast_group_table
> *sbrec_multicast_group_table,
> > -    const struct hmap * ls_datapaths, const struct hmap *lr_datapaths,
> > -    struct hmap *mcast_groups)
> > +    const struct hmap * ls_datapaths, const struct hmap *lr_datapaths)
> >  {
> > +    struct hmapx mcast_in_sb = HMAPX_INITIALIZER(&mcast_in_sb);
> > +
> >      /* Push changes to the Multicast_Group table to database. */
> >      const struct sbrec_multicast_group *sbmc;
> > -    SBREC_MULTICAST_GROUP_TABLE_FOR_EACH_SAFE (
> > -        sbmc, sbrec_multicast_group_table) {
> > +    SBREC_MULTICAST_GROUP_TABLE_FOR_EACH (sbmc,
> sbrec_multicast_group_table) {
> >          struct ovn_datapath *od = ovn_datapath_from_sbrec(ls_datapaths,
> >                                                            lr_datapaths,
> >
> sbmc->datapath);
> > @@ -319,55 +413,34 @@ sync_multicast_groups_to_sb(
> >
> >          struct multicast_group group = { .name = sbmc->name,
> >              .key = sbmc->tunnel_key };
> > -        struct ovn_multicast *mc = ovn_multicast_find(mcast_groups,
> > +        struct ovn_multicast *mc =
> ovn_multicast_find(&data->mcast_groups,
> >                                                        od, &group);
> >          if (mc) {
> >              ovn_multicast_update_sbrec(mc, sbmc);
> > -            ovn_multicast_destroy(mcast_groups, mc);
> > +            hmapx_add(&mcast_in_sb, mc);
> >          } else {
> >              sbrec_multicast_group_delete(sbmc);
> >          }
> >      }
> >      struct ovn_multicast *mc;
> > -    HMAP_FOR_EACH_SAFE (mc, hmap_node, mcast_groups) {
> > +    HMAP_FOR_EACH_SAFE (mc, hmap_node, &data->mcast_groups) {
> >          if (!mc->datapath) {
> > -            ovn_multicast_destroy(mcast_groups, mc);
> > +            ovn_multicast_destroy(&data->mcast_groups, mc);
> >              continue;
> >          }
> > +
> > +        if (hmapx_contains(&mcast_in_sb, mc)) {
> > +            continue;
> > +        }
> > +
> >          sbmc = create_sb_multicast_group(ovnsb_txn, mc->datapath->sb,
> >                                           mc->group->name,
> mc->group->key);
> >          ovn_multicast_update_sbrec(mc, sbmc);
> > -        ovn_multicast_destroy(mcast_groups, mc);
> >      }
> >
> > -    hmap_destroy(mcast_groups);
> > +    hmapx_destroy(&mcast_in_sb);
> >  }
> >
> > -void
> > -ovn_igmp_groups_destroy(struct hmap *igmp_groups)
> > -{
> > -    struct ovn_igmp_group *igmp_group;
> > -    HMAP_FOR_EACH_SAFE (igmp_group, hmap_node, igmp_groups) {
> > -        ovn_igmp_group_destroy(igmp_groups, igmp_group);
> > -    }
> > -    hmap_destroy(igmp_groups);
> > -}
> > -
> > -struct sbrec_multicast_group *
> > -create_sb_multicast_group(struct ovsdb_idl_txn *ovnsb_txn,
> > -                          const struct sbrec_datapath_binding *dp,
> > -                          const char *name,
> > -                          int64_t tunnel_key)
> > -{
> > -    struct sbrec_multicast_group *sbmc =
> > -        sbrec_multicast_group_insert(ovnsb_txn);
> > -    sbrec_multicast_group_set_datapath(sbmc, dp);
> > -    sbrec_multicast_group_set_name(sbmc, name);
> > -    sbrec_multicast_group_set_tunnel_key(sbmc, tunnel_key);
> > -    return sbmc;
> > -}
> > -
> > -
> >  static bool
> >  multicast_group_equal(const struct multicast_group *a,
> >                        const struct multicast_group *b)
> > @@ -375,7 +448,6 @@ multicast_group_equal(const struct multicast_group
> *a,
> >      return !strcmp(a->name, b->name) && a->key == b->key;
> >  }
> >
> > -
> >  static uint32_t
> >  ovn_multicast_hash(const struct ovn_datapath *datapath,
> >                     const struct multicast_group *group)
> > @@ -465,6 +537,15 @@ ovn_multicast_update_sbrec(const struct
> ovn_multicast *mc,
> >      free(ports);
> >  }
> >
> > +static void
> > +ovn_multicast_groups_destroy(struct hmap *mcast_groups)
> > +{
> > +    struct ovn_multicast *mc;
> > +    HMAP_FOR_EACH_SAFE (mc, hmap_node, mcast_groups) {
> > +        ovn_multicast_destroy(mcast_groups, mc);
> > +    }
> > +}
> > +
> >  static uint32_t
> >  ovn_igmp_group_hash(const struct ovn_datapath *datapath,
> >                      const struct in6_addr *address)
> > @@ -522,8 +603,6 @@ ovn_igmp_group_add(struct ovsdb_idl_index
> *sbrec_mcast_group_by_name_dp,
> >
> >          hmap_insert(igmp_groups, &igmp_group->hmap_node,
> >                      ovn_igmp_group_hash(datapath, address));
> > -        ovs_list_push_back(&datapath->mcast_info.groups,
> > -                           &igmp_group->list_node);
> >      }
> >
> >      return igmp_group;
> > @@ -656,7 +735,15 @@ ovn_igmp_group_destroy(struct hmap *igmp_groups,
> >              free(entry);
> >          }
> >          hmap_remove(igmp_groups, &igmp_group->hmap_node);
> > -        ovs_list_remove(&igmp_group->list_node);
> >          free(igmp_group);
> >      }
> >  }
> > +
> > +static void
> > +ovn_igmp_groups_destroy(struct hmap *igmp_groups)
> > +{
> > +    struct ovn_igmp_group *igmp_group;
> > +    HMAP_FOR_EACH_SAFE (igmp_group, hmap_node, igmp_groups) {
> > +        ovn_igmp_group_destroy(igmp_groups, igmp_group);
> > +    }
> > +}
> > diff --git a/northd/en-multicast.h b/northd/en-multicast.h
> > index 5fa4d8976..3932b4b08 100644
> > --- a/northd/en-multicast.h
> > +++ b/northd/en-multicast.h
> > @@ -62,7 +62,6 @@ struct ovn_igmp_group_entry {
> >   */
> >  struct ovn_igmp_group {
> >      struct hmap_node hmap_node; /* Index on 'datapath' and 'address'. */
> > -    struct ovs_list list_node;  /* Linkage in the per-dp igmp group
> list. */
> >
> >      struct ovn_datapath *datapath;
> >      struct in6_addr address; /* Multicast IPv6-mapped-IPv4 or IPv4
> address. */
> > @@ -71,20 +70,16 @@ struct ovn_igmp_group {
> >      struct ovs_list entries; /* List of SB entries for this group. */
> >  };
> >
> > -void build_mcast_groups(
> > -    const struct sbrec_igmp_group_table *sbrec_igmp_group_table,
> > -    struct ovsdb_idl_index *sbrec_mcast_group_by_name_dp,
> > -    const struct hmap *ls_datapaths,
> > -    const struct hmap *ls_ports,
> > -    const struct hmap *lr_ports,
> > -    struct hmap *mcast_groups,
> > -    struct hmap *igmp_groups);
> > -void sync_multicast_groups_to_sb(
> > -    struct ovsdb_idl_txn *ovnsb_txn,
> > -    const struct sbrec_multicast_group_table
> *sbrec_multicast_group_table,
> > -    const struct hmap * ls_datapaths, const struct hmap *lr_datapaths,
> > -    struct hmap *mcast_groups);
> > -void ovn_igmp_groups_destroy(struct hmap *igmp_groups);
> > +struct multicast_igmp_data {
> > +    struct hmap mcast_groups;
> > +    struct hmap igmp_groups;
> > +    struct lflow_ref *lflow_ref;
> > +};
> > +
> > +void *en_multicast_igmp_init(struct engine_node *,struct engine_arg *);
> > +void en_multicast_igmp_run(struct engine_node *, void *);
> > +bool multicast_igmp_northd_handler(struct engine_node *, void *);
> > +void en_multicast_igmp_cleanup(void *);
> >  struct sbrec_multicast_group *create_sb_multicast_group(
> >      struct ovsdb_idl_txn *ovnsb_txn, const struct
> sbrec_datapath_binding *,
> >      const char *name, int64_t tunnel_key);
> > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> > index 6e0aa04c4..a9990974c 100644
> > --- a/northd/inc-proc-northd.c
> > +++ b/northd/inc-proc-northd.c
> > @@ -34,6 +34,7 @@
> >  #include "en-lr-stateful.h"
> >  #include "en-lr-nat.h"
> >  #include "en-ls-stateful.h"
> > +#include "en-multicast.h"
> >  #include "en-northd.h"
> >  #include "en-lflow.h"
> >  #include "en-northd-output.h"
> > @@ -161,6 +162,7 @@ static ENGINE_NODE(route_policies, "route_policies");
> >  static ENGINE_NODE(routes, "routes");
> >  static ENGINE_NODE(bfd, "bfd");
> >  static ENGINE_NODE(bfd_sync, "bfd_sync");
> > +static ENGINE_NODE(multicast_igmp, "multicast_igmp");
> >
> >  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> >                            struct ovsdb_idl_loop *sb)
> > @@ -267,11 +269,15 @@ void inc_proc_northd_init(struct ovsdb_idl_loop
> *nb,
> >      engine_add_input(&en_sync_meters, &en_nb_meter, NULL);
> >      engine_add_input(&en_sync_meters, &en_sb_meter, NULL);
> >
> > +    engine_add_input(&en_multicast_igmp, &en_northd,
> > +                     multicast_igmp_northd_handler);
> > +    engine_add_input(&en_multicast_igmp, &en_sb_multicast_group, NULL);
> > +    engine_add_input(&en_multicast_igmp, &en_sb_igmp_group, NULL);
> > +
> >      engine_add_input(&en_lflow, &en_nb_acl, NULL);
> >      engine_add_input(&en_lflow, &en_sync_meters, NULL);
> >      engine_add_input(&en_lflow, &en_sb_logical_flow, NULL);
> >      engine_add_input(&en_lflow, &en_sb_multicast_group, NULL);
> > -    engine_add_input(&en_lflow, &en_sb_igmp_group, NULL);
> >      engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL);
> >      engine_add_input(&en_lflow, &en_bfd_sync, NULL);
> >      engine_add_input(&en_lflow, &en_route_policies, NULL);
> > @@ -285,6 +291,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> >      engine_add_input(&en_lflow, &en_port_group,
> lflow_port_group_handler);
> >      engine_add_input(&en_lflow, &en_lr_stateful,
> lflow_lr_stateful_handler);
> >      engine_add_input(&en_lflow, &en_ls_stateful,
> lflow_ls_stateful_handler);
> > +    engine_add_input(&en_lflow, &en_multicast_igmp,
> > +                     lflow_multicast_igmp_handler);
> >
> >      engine_add_input(&en_sync_to_sb_addr_set, &en_northd, NULL);
> >      engine_add_input(&en_sync_to_sb_addr_set, &en_lr_stateful, NULL);
> > diff --git a/northd/northd.c b/northd/northd.c
> > index ddd05f7e4..fc352e397 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -668,7 +668,6 @@ init_mcast_info_for_datapath(struct ovn_datapath *od)
> >      hmap_init(&od->mcast_info.group_tnlids);
> >      /* allocations start from hint + 1 */
> >      od->mcast_info.group_tnlid_hint = OVN_MIN_IP_MULTICAST - 1;
> > -    ovs_list_init(&od->mcast_info.groups);
> >
> >      if (od->nbs) {
> >          init_mcast_info_for_switch_datapath(od);
> > @@ -9815,39 +9814,6 @@ build_lswitch_destination_lookup_bmcast(struct
> ovn_datapath *od,
> >                        "ip6.mcast_flood",
> >                        "outport = \""MC_FLOOD"\"; output;",
> >                        lflow_ref);
> > -
> > -        /* Forward uregistered IP multicast to routers with relay
> enabled
> > -         * and to any ports configured to flood IP multicast traffic.
> > -         * If configured to flood unregistered traffic this will be
> > -         * handled by the L2 multicast flow.
> > -         */
> > -        if (!mcast_sw_info->flood_unregistered) {
> > -            ds_clear(actions);
> > -
> > -            if (mcast_sw_info->flood_relay) {
> > -                ds_put_cstr(actions,
> > -                            "clone { "
> > -                                "outport = \""MC_MROUTER_FLOOD"\"; "
> > -                                "output; "
> > -                            "}; ");
> > -            }
> > -
> > -            if (mcast_sw_info->flood_static) {
> > -                ds_put_cstr(actions, "outport =\""MC_STATIC"\";
> output;");
> > -            }
> > -
> > -            /* Explicitly drop the traffic if relay or static flooding
> > -             * is not configured.
> > -             */
> > -            if (!mcast_sw_info->flood_relay &&
> > -                    !mcast_sw_info->flood_static) {
> > -                ds_put_cstr(actions, debug_drop_action());
> > -            }
> > -
> > -            ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 80,
> > -                          "ip4.mcast || ip6.mcast",
> > -                          ds_cstr(actions), lflow_ref);
> > -        }
> >      }
> >
> >      if (!smap_get_bool(&od->nbs->other_config,
> > @@ -9862,6 +9828,48 @@ build_lswitch_destination_lookup_bmcast(struct
> ovn_datapath *od,
> >                    "outport = \""MC_FLOOD"\"; output;", lflow_ref);
> >  }
> >
> > +/* Ingress table destination lookup, multicast handling (priority 80).
> */
> > +static void
> > +build_mcast_flood_lswitch(struct ovn_datapath *od, struct lflow_table
> *lflows,
> > +                          struct ds *actions, struct lflow_ref
> *lflow_ref)
> > +{
> > +    ovs_assert(od->nbs);
> > +    struct mcast_switch_info *mcast_sw_info = &od->mcast_info.sw;
> > +    if (!mcast_sw_info->enabled || mcast_sw_info->flood_unregistered) {
> > +        return;
> > +    }
> > +
> > +    ds_clear(actions);
> > +
> > +    /* Forward unregistered IP multicast to routers with relay enabled
> > +     * and to any ports configured to flood IP multicast traffic.
> > +     * If configured to flood unregistered traffic this will be
> > +     * handled by the L2 multicast flow.
> > +     */
> > +    if (mcast_sw_info->flood_relay) {
> > +        ds_put_cstr(actions,
> > +                    "clone { "
> > +                        "outport = \""MC_MROUTER_FLOOD"\"; "
> > +                        "output; "
> > +                    "}; ");
> > +    }
> > +
> > +    if (mcast_sw_info->flood_static) {
> > +        ds_put_cstr(actions, "outport =\""MC_STATIC"\"; output;");
> > +    }
> > +
> > +    /* Explicitly drop the traffic if relay or static flooding
> > +     * is not configured.
> > +     */
> > +    if (!mcast_sw_info->flood_relay &&
> > +        !mcast_sw_info->flood_static) {
> > +        ds_put_cstr(actions, debug_drop_action());
> > +    }
> > +
> > +    ovn_lflow_add(lflows, od, S_SWITCH_IN_L2_LKUP, 80,
> > +                  "ip4.mcast || ip6.mcast", ds_cstr(actions),
> lflow_ref);
> > +}
> > +
> >
> >  /* Ingress table 25: Add IP multicast flows learnt from IGMP/MLD
> >   * (priority 90). */
> > @@ -9869,11 +9877,10 @@ static void
> >  build_lswitch_ip_mcast_igmp_mld(struct ovn_igmp_group *igmp_group,
> >                                  struct lflow_table *lflows,
> >                                  struct ds *actions,
> > -                                struct ds *match)
> > +                                struct ds *match,
> > +                                struct lflow_ref *lflow_ref)
> >  {
> > -    if (!(igmp_group->datapath && igmp_group->datapath->nbs)) {
> > -        return;
> > -    }
> > +    ovs_assert(igmp_group->datapath->nbs);
> >
> >      uint64_t dummy;
> >
> > @@ -9943,7 +9950,7 @@ build_lswitch_ip_mcast_igmp_mld(struct
> ovn_igmp_group *igmp_group,
> >                    igmp_group->mcgroup.name);
> >
> >      ovn_lflow_add(lflows, igmp_group->datapath, S_SWITCH_IN_L2_LKUP,
> > -                  90, ds_cstr(match), ds_cstr(actions), NULL);
> > +                  90, ds_cstr(match), ds_cstr(actions), lflow_ref);
> >  }
> >
> >  /* Ingress table 25: Destination lookup, unicast handling (priority
> 50), */
> > @@ -13546,14 +13553,49 @@ build_route_flows_for_lrouter(
> >      unique_routes_destroy(&unique_routes);
> >  }
> >
> > +static void
> > +build_igmp_flows_for_lrouter(struct ovn_igmp_group *igmp_group,
> > +                             struct lflow_table *lflows,
> > +                             struct ds *match, struct ds *actions,
> > +                             struct lflow_ref *lflow_ref)
> > +{
> > +    ovs_assert(igmp_group->datapath->nbr);
> > +
> > +    if (!igmp_group->datapath->mcast_info.rtr.relay) {
> > +        return;
> > +    }
> > +
> > +    ds_clear(match);
> > +    ds_clear(actions);
> > +    if (IN6_IS_ADDR_V4MAPPED(&igmp_group->address)) {
> > +        ds_put_format(match, "ip4 && ip4.dst == %s ",
> > +                      igmp_group->mcgroup.name);
> > +    } else {
> > +        ds_put_format(match, "ip6 && ip6.dst == %s ",
> > +                      igmp_group->mcgroup.name);
> > +    }
> > +    if (igmp_group->datapath->mcast_info.rtr.flood_static) {
> > +        ds_put_cstr(actions,
> > +                    "clone { "
> > +                    "outport = \""MC_STATIC"\"; "
> > +                    "ip.ttl--; "
> > +                    "next; "
> > +                    "};");
> > +    }
> > +    ds_put_format(actions, "outport = \"%s\"; ip.ttl--; next;",
> > +                  igmp_group->mcgroup.name);
> > +    ovn_lflow_add(lflows, igmp_group->datapath, S_ROUTER_IN_IP_ROUTING,
> 10500,
> > +                  ds_cstr(match), ds_cstr(actions),
> > +                  lflow_ref);
> > +}
> > +
> >  /* IP Multicast lookup. Here we set the output port, adjust TTL and
> >   * advance to next table (priority 500).
> >   */
> >  static void
> >  build_mcast_lookup_flows_for_lrouter(
> >          struct ovn_datapath *od, struct lflow_table *lflows,
> > -        struct ds *match, struct ds *actions,
> > -        struct lflow_ref *lflow_ref)
> > +        struct ds *match, struct lflow_ref *lflow_ref)
> >  {
> >      ovs_assert(od->nbr);
> >
> > @@ -13567,33 +13609,6 @@ build_mcast_lookup_flows_for_lrouter(
> >          return;
> >      }
> >
> > -    struct ovn_igmp_group *igmp_group;
> > -
> > -    LIST_FOR_EACH (igmp_group, list_node, &od->mcast_info.groups) {
> > -        ds_clear(match);
> > -        ds_clear(actions);
> > -        if (IN6_IS_ADDR_V4MAPPED(&igmp_group->address)) {
> > -            ds_put_format(match, "ip4 && ip4.dst == %s ",
> > -                        igmp_group->mcgroup.name);
> > -        } else {
> > -            ds_put_format(match, "ip6 && ip6.dst == %s ",
> > -                        igmp_group->mcgroup.name);
> > -        }
> > -        if (od->mcast_info.rtr.flood_static) {
> > -            ds_put_cstr(actions,
> > -                        "clone { "
> > -                            "outport = \""MC_STATIC"\"; "
> > -                            "ip.ttl--; "
> > -                            "next; "
> > -                        "};");
> > -        }
> > -        ds_put_format(actions, "outport = \"%s\"; ip.ttl--; next;",
> > -                      igmp_group->mcgroup.name);
> > -        ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10500,
> > -                      ds_cstr(match), ds_cstr(actions),
> > -                      lflow_ref);
> > -    }
> > -
> >      /* If needed, flood unregistered multicast on statically configured
> >       * ports. Otherwise drop any multicast traffic.
> >       */
> > @@ -16964,7 +16979,6 @@ struct lswitch_flow_build_info {
> >      const struct lr_stateful_table *lr_stateful_table;
> >      const struct ls_stateful_table *ls_stateful_table;
> >      struct lflow_table *lflows;
> > -    struct hmap *igmp_groups;
> >      const struct shash *meter_groups;
> >      const struct hmap *lb_dps_map;
> >      const struct hmap *svc_monitor_map;
> > @@ -17026,8 +17040,7 @@ build_lswitch_and_lrouter_iterate_by_lr(struct
> ovn_datapath *od,
> >      build_route_flows_for_lrouter(od, lsi->lflows,
> >                                    lsi->parsed_routes, lsi->route_tables,
> >                                    lsi->bfd_ports, NULL);
> > -    build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> > -                                         &lsi->actions, NULL);
> > +    build_mcast_lookup_flows_for_lrouter(od, lsi->lflows, &lsi->match,
> NULL);
> >      build_ingress_policy_flows_for_lrouter(od, lsi->lflows,
> lsi->lr_ports,
> >                                             lsi->route_policies, NULL);
> >      build_arp_resolve_flows_for_lrouter(od, lsi->lflows, NULL);
> > @@ -17129,7 +17142,6 @@ build_lflows_thread(void *arg)
> >      const struct lr_stateful_record *lr_stateful_rec;
> >      const struct ls_stateful_record *ls_stateful_rec;
> >      struct lswitch_flow_build_info *lsi;
> > -    struct ovn_igmp_group *igmp_group;
> >      struct ovn_lb_datapaths *lb_dps;
> >      struct ovn_datapath *od;
> >      struct ovn_port *op;
> > @@ -17280,20 +17292,6 @@ build_lflows_thread(void *arg)
> >                  }
> >              }
> >
> > -            for (bnum = control->id;
> > -                    bnum <= lsi->igmp_groups->mask;
> > -                    bnum += control->pool->size)
> > -            {
> > -                HMAP_FOR_EACH_IN_PARALLEL (
> > -                        igmp_group, hmap_node, bnum, lsi->igmp_groups) {
> > -                    if (stop_parallel_processing()) {
> > -                        return NULL;
> > -                    }
> > -                    build_lswitch_ip_mcast_igmp_mld(igmp_group,
> lsi->lflows,
> > -                                                    &lsi->match,
> > -                                                    &lsi->actions);
> > -                }
> > -            }
> >              lsi->thread_lflow_counter = thread_lflow_counter;
> >          }
> >          post_completed_work(control);
> > @@ -17343,7 +17341,6 @@ build_lswitch_and_lrouter_flows(
> >      const struct lr_stateful_table *lr_stateful_table,
> >      const struct ls_stateful_table *ls_stateful_table,
> >      struct lflow_table *lflows,
> > -    struct hmap *igmp_groups,
> >      const struct shash *meter_groups,
> >      const struct hmap *lb_dps_map,
> >      const struct hmap *svc_monitor_map,
> > @@ -17378,7 +17375,6 @@ build_lswitch_and_lrouter_flows(
> >              lsiv[index].ls_port_groups = ls_pgs;
> >              lsiv[index].lr_stateful_table = lr_stateful_table;
> >              lsiv[index].ls_stateful_table = ls_stateful_table;
> > -            lsiv[index].igmp_groups = igmp_groups;
> >              lsiv[index].meter_groups = meter_groups;
> >              lsiv[index].lb_dps_map = lb_dps_map;
> >              lsiv[index].svc_monitor_map = svc_monitor_map;
> > @@ -17409,7 +17405,6 @@ build_lswitch_and_lrouter_flows(
> >      } else {
> >          const struct lr_stateful_record *lr_stateful_rec;
> >          const struct ls_stateful_record *ls_stateful_rec;
> > -        struct ovn_igmp_group *igmp_group;
> >          struct ovn_lb_datapaths *lb_dps;
> >          struct ovn_datapath *od;
> >          struct ovn_port *op;
> > @@ -17423,7 +17418,6 @@ build_lswitch_and_lrouter_flows(
> >              .lr_stateful_table = lr_stateful_table,
> >              .ls_stateful_table = ls_stateful_table,
> >              .lflows = lflows,
> > -            .igmp_groups = igmp_groups,
> >              .meter_groups = meter_groups,
> >              .lb_dps_map = lb_dps_map,
> >              .svc_monitor_map = svc_monitor_map,
> > @@ -17513,14 +17507,6 @@ build_lswitch_and_lrouter_flows(
> >                                      lsi.lflows);
> >          }
> >          stopwatch_stop(LFLOWS_LS_STATEFUL_STOPWATCH_NAME, time_msec());
> > -        stopwatch_start(LFLOWS_IGMP_STOPWATCH_NAME, time_msec());
> > -        HMAP_FOR_EACH (igmp_group, hmap_node, igmp_groups) {
> > -            build_lswitch_ip_mcast_igmp_mld(igmp_group,
> > -                                            lsi.lflows,
> > -                                            &lsi.actions,
> > -                                            &lsi.match);
> > -        }
> > -        stopwatch_stop(LFLOWS_IGMP_STOPWATCH_NAME, time_msec());
> >
> >          ds_destroy(&lsi.match);
> >          ds_destroy(&lsi.actions);
> > @@ -17529,6 +17515,38 @@ build_lswitch_and_lrouter_flows(
> >      free(svc_check_match);
> >  }
> >
> > +/* The IGMP flows have to be built in main thread because there is
> > + * single lflow_ref for all of them which isn't thread safe. */
> > +void
> > +build_igmp_lflows(struct hmap *igmp_groups, const struct hmap
> *ls_datapaths,
> > +                  struct lflow_table *lflows, struct lflow_ref
> *lflow_ref)
> > +{
> > +    struct ds actions = DS_EMPTY_INITIALIZER;
> > +    struct ds match = DS_EMPTY_INITIALIZER;
> > +
> > +    struct ovn_datapath *od;
> > +    HMAP_FOR_EACH (od, key_node, ls_datapaths) {
> > +        init_mcast_flow_count(od);
> > +        build_mcast_flood_lswitch(od, lflows, &actions, lflow_ref);
> > +    }
> > +
> > +    stopwatch_start(LFLOWS_IGMP_STOPWATCH_NAME, time_msec());
> > +    struct ovn_igmp_group *igmp_group;
> > +    HMAP_FOR_EACH (igmp_group, hmap_node, igmp_groups) {
> > +        if (igmp_group->datapath->nbs) {
> > +            build_lswitch_ip_mcast_igmp_mld(igmp_group, lflows,
> &actions,
> > +                                            &match, lflow_ref);
> > +        } else {
> > +            build_igmp_flows_for_lrouter(igmp_group, lflows, &actions,
> > +                                         &match, lflow_ref);
> > +        }
> > +    }
> > +    stopwatch_stop(LFLOWS_IGMP_STOPWATCH_NAME, time_msec());
> > +
> > +    ds_destroy(&actions);
> > +    ds_destroy(&match);
> > +}
> > +
> >  void run_update_worker_pool(int n_threads)
> >  {
> >      /* If number of threads has been updated (or initially set),
> > @@ -17553,20 +17571,6 @@ void build_lflows(struct ovsdb_idl_txn
> *ovnsb_txn,
> >                    struct lflow_input *input_data,
> >                    struct lflow_table *lflows)
> >  {
> > -    struct hmap mcast_groups;
> > -    struct hmap igmp_groups;
> > -
> > -    struct ovn_datapath *od;
> > -    HMAP_FOR_EACH (od, key_node, &input_data->ls_datapaths->datapaths) {
> > -        init_mcast_flow_count(od);
> > -    }
> > -
> > -    build_mcast_groups(input_data->sbrec_igmp_group_table,
> > -                       input_data->sbrec_mcast_group_by_name_dp,
> > -                       &input_data->ls_datapaths->datapaths,
> > -                       input_data->ls_ports, input_data->lr_ports,
> > -                       &mcast_groups, &igmp_groups);
> > -
> >      build_lswitch_and_lrouter_flows(input_data->ls_datapaths,
> >                                      input_data->lr_datapaths,
> >                                      input_data->ls_ports,
> > @@ -17575,7 +17579,6 @@ void build_lflows(struct ovsdb_idl_txn
> *ovnsb_txn,
> >                                      input_data->lr_stateful_table,
> >                                      input_data->ls_stateful_table,
> >                                      lflows,
> > -                                    &igmp_groups,
> >                                      input_data->meter_groups,
> >                                      input_data->lb_datapaths_map,
> >                                      input_data->svc_monitor_map,
> > @@ -17586,6 +17589,9 @@ void build_lflows(struct ovsdb_idl_txn
> *ovnsb_txn,
> >                                      input_data->parsed_routes,
> >                                      input_data->route_policies,
> >                                      input_data->route_tables);
> > +    build_igmp_lflows(input_data->igmp_groups,
> > +                      &input_data->ls_datapaths->datapaths,
> > +                      lflows, input_data->igmp_lflow_ref);
> >
> >      if (parallelization_state == STATE_INIT_HASH_SIZES) {
> >          parallelization_state = STATE_USE_PARALLELIZATION;
> > @@ -17603,13 +17609,6 @@ void build_lflows(struct ovsdb_idl_txn
> *ovnsb_txn,
> >                             input_data->sbrec_logical_dp_group_table);
> >
> >      stopwatch_stop(LFLOWS_TO_SB_STOPWATCH_NAME, time_msec());
> > -
> > -    sync_multicast_groups_to_sb(ovnsb_txn,
> > -                                input_data->sbrec_multicast_group_table,
> > -                                &input_data->ls_datapaths->datapaths,
> > -                                &input_data->lr_datapaths->datapaths,
> > -                                &mcast_groups);
> > -    ovn_igmp_groups_destroy(&igmp_groups);
> >  }
> >
> >  void
> > diff --git a/northd/northd.h b/northd/northd.h
> > index 044704c70..cce2cb4a9 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -26,6 +26,7 @@
> >  #include "simap.h"
> >  #include "ovs-thread.h"
> >
> > +
> >  struct northd_input {
> >      /* Northbound table references */
> >      const struct nbrec_logical_switch_table *nbrec_logical_switch_table;
> > @@ -198,13 +199,12 @@ struct bfd_sync_data {
> >      struct sset bfd_ports;
> >  };
> >
> > +struct lflow_ref;
> >  struct lr_nat_table;
> >
> >  struct lflow_input {
> >      /* Southbound table references */
> >      const struct sbrec_logical_flow_table *sbrec_logical_flow_table;
> > -    const struct sbrec_multicast_group_table
> *sbrec_multicast_group_table;
> > -    const struct sbrec_igmp_group_table *sbrec_igmp_group_table;
> >      const struct sbrec_logical_dp_group_table
> *sbrec_logical_dp_group_table;
> >
> >      /* Indexes */
> > @@ -228,6 +228,8 @@ struct lflow_input {
> >      struct hmap *parsed_routes;
> >      struct hmap *route_policies;
> >      struct simap *route_tables;
> > +    struct hmap *igmp_groups;
> > +    struct lflow_ref *igmp_lflow_ref;
> >  };
> >
> >  extern int parallelization_state;
> > @@ -292,7 +294,6 @@ struct mcast_info {
> >
> >      struct hmap group_tnlids;  /* Group tunnel IDs in use on this DP. */
> >      uint32_t group_tnlid_hint; /* Hint for allocating next group tunnel
> ID. */
> > -    struct ovs_list groups;    /* List of groups learnt on this DP. */
> >
> >      union {
> >          struct mcast_switch_info sw;  /* Switch specific multicast
> info. */
> > @@ -897,5 +898,8 @@ lsp_is_router(const struct nbrec_logical_switch_port
> *nbsp)
> >  }
> >
> >  struct ovn_port *ovn_port_find(const struct hmap *ports, const char
> *name);
> > -
> > +void build_igmp_lflows(struct hmap *igmp_groups,
> > +                       const struct hmap *ls_datapaths,
> > +                       struct lflow_table *lflows,
> > +                       struct lflow_ref *lflow_ref);
> >  #endif /* NORTHD_H */
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 507cc302f..91ba5b736 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -14384,3 +14384,92 @@ AT_CHECK([ovn-sbctl lflow-list S1 | grep
> ls_out_acl_action | grep priority=500 |
> >
> >  AT_CLEANUP
> >  ])
> > +
> > +OVN_FOR_EACH_NORTHD_NO_HV([
> > +AT_SETUP([IGMP incremental processing])
> > +
> > +check_recompute_counter() {
> > +    lflow_recomp=$(as northd ovn-appctl -t ovn-northd
> inc-engine/show-stats lflow recompute)
> > +    AT_CHECK([test x$lflow_recomp = x$1])
> > +}
> > +ovn_start
> > +
> > +net_add n1
> > +sim_add hv1
> > +as hv1
> > +
> > +ovs-vsctl add-br br-phys
> > +ovn_attach n1 br-phys 192.168.0.11
> > +
> > +sim_add hv2
> > +as hv2
> > +
> > +check ovs-vsctl add-br br-phys
> > +ovn_attach n1 br-phys 192.168.0.2
> > +
> > +check ovn-nbctl ls-add sw1
> > +check ovn-nbctl ls-add sw2
> > +
> > +check ovn-nbctl lsp-add sw1 sw1-p11
> > +check ovn-nbctl lsp-add sw2 sw2-p21
> > +
> > +check ovn-nbctl lr-add rtr
> > +check ovn-nbctl lrp-add rtr rtr-sw1 00:00:00:00:01:00 10.0.0.254/24
> > +check ovn-nbctl lrp-add rtr rtr-sw2 00:00:00:00:02:00 10.0.0.254/24
> > +
> > +
> > +check ovn-nbctl lsp-add sw1 sw1-rtr \
> > +    -- lsp-set-type sw1-rtr router  \
> > +    -- lsp-set-addresses sw1-rtr 00:00:00:00:01:00 \
> > +    -- lsp-set-options sw1-rtr router-port=rtr-sw1
> > +
> > +check ovn-nbctl lsp-add sw2 sw2-rtr \
> > +    -- lsp-set-type sw2-rtr router  \
> > +    -- lsp-set-addresses sw1-rtr 00:00:00:00:02:00 \
> > +    -- lsp-set-options sw2-rtr router-port=rtr-sw2
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +
> > +# Create IGMP_Group 239.0.1.68 with port sw1-p11
> > +ovn-sbctl create IGMP_Group address=239.0.1.68 \
> > +    datapath=$(fetch_column Datapath_Binding _uuid
> external_ids:name=sw1) \
> > +    chassis=$(fetch_column Chassis _uuid name=hv1) \
> > +    chassis_name=hv1 \
> > +    ports=$(fetch_column Port_Binding _uuid logical_port=sw1-p11)
> > +igmp_uuid=$(fetch_column IGMP_GROUP _uuid address=239.0.1.68)
> > +
> > +check ovn-nbctl --wait=sb sync
> > +wait_row_count Igmp_Group 1 address=239.0.1.68
> > +wait_row_count Multicast_Group 1 name="239.0.1.68"
> > +wait_row_count Multicast_Group  1 name="239.0.1.68"
> ports='[['$(fetch_column Port_Binding _uuid logical_port=sw1-p11)']]'
> > +ovn-sbctl list igmp_group
> > +check_recompute_counter 0
> > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > +
> > +check ovn-nbctl set logical_router rtr \
> > +    options:mcast_relay="true"
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +# Update IGMP_Group 239.0.1.68 to include sw2-p21
> > +ovn-sbctl add IGMP_Group $igmp_uuid ports $(fetch_column Port_Binding
> _uuid logical_port=sw2-p21)
> > +
> > +check ovn-nbctl --wait=sb sync
> > +wait_row_count IGMP_Group 1 address=239.0.1.68
> > +
> > +# Check that new Multicast_Group is created
> > +wait_row_count Multicast_Group 2 name=239.0.1.68
> > +check_recompute_counter 0
> > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +# Delete IGMP_Group 239.0.1.68
> > +ovn-sbctl destroy IGMP_Group $igmp_uuid
> > +check ovn-nbctl --wait=sb sync
> > +check_recompute_counter 0
> > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > +
> > +wait_row_count IGMP_Group 0 address=239.0.1.68
> > +wait_row_count Multicast_Group 0 name=239.0.1.68
> > +
> > +OVN_CLEANUP([hv1], [hv2])
> > +AT_CLEANUP
> > +])
> > --
> > 2.47.1
> >
> > _______________________________________________
> > 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

Reply via email to