On Mon, Sep 26, 2022 at 12:18 PM Dumitru Ceara <[email protected]> wrote:

> The nd_ra_opts and controller_event_ops are actually static they never
> change at runtime.  DHCP records can instead be computed when populating
> the lflow "input context" to be used during incremental processing.  This
> is likely more efficient than building the DHCP opts maps for every logical
> flow recomputed due to changes in resources they reference (e.g., port
> bindings, multicast groups).
>
> Signed-off-by: Dumitru Ceara <[email protected]>
> ---
> V2:
> - Patch 1/3 was merged.
> - Patch 3/3 was dropped.
> - Addressed Han's comments on Patch 2/3:
>   - added new I-P node for dhcp
>   - re-added missing dhcp/nd/controller_event refs when processing AS
>     updates.
>   - re-worded the commit message.
> ---
>  controller/lflow.c          | 211 +++---------------------------------
>  controller/lflow.h          |   8 +-
>  controller/ovn-controller.c |  86 +++++++++++++--
>  lib/ovn-l7.h                |  16 ++-
>  4 files changed, 111 insertions(+), 210 deletions(-)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index eef44389f..32664b080 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -90,9 +90,6 @@ add_matches_to_flow_table(const struct
> sbrec_logical_flow *,
>                            struct lflow_ctx_out *);
>  static void
>  consider_logical_flow(const struct sbrec_logical_flow *lflow,
> -                      struct hmap *dhcp_opts, struct hmap *dhcpv6_opts,
> -                      struct hmap *nd_ra_opts,
> -                      struct controller_event_options
> *controller_event_opts,
>                        bool is_recompute,
>                        struct lflow_ctx_in *l_ctx_in,
>                        struct lflow_ctx_out *l_ctx_out);
> @@ -371,40 +368,9 @@ add_logical_flows(struct lflow_ctx_in *l_ctx_in,
>                    struct lflow_ctx_out *l_ctx_out)
>  {
>      const struct sbrec_logical_flow *lflow;
> -
> -    struct hmap dhcp_opts = HMAP_INITIALIZER(&dhcp_opts);
> -    struct hmap dhcpv6_opts = HMAP_INITIALIZER(&dhcpv6_opts);
> -    const struct sbrec_dhcp_options *dhcp_opt_row;
> -    SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row,
> -                                       l_ctx_in->dhcp_options_table) {
> -        dhcp_opt_add(&dhcp_opts, dhcp_opt_row->name, dhcp_opt_row->code,
> -                     dhcp_opt_row->type);
> -    }
> -
> -
> -    const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
> -    SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH (dhcpv6_opt_row,
> -                                         l_ctx_in->dhcpv6_options_table) {
> -       dhcp_opt_add(&dhcpv6_opts, dhcpv6_opt_row->name,
> dhcpv6_opt_row->code,
> -                    dhcpv6_opt_row->type);
> -    }
> -
> -    struct hmap nd_ra_opts = HMAP_INITIALIZER(&nd_ra_opts);
> -    nd_ra_opts_init(&nd_ra_opts);
> -
> -    struct controller_event_options controller_event_opts;
> -    controller_event_opts_init(&controller_event_opts);
> -
>      SBREC_LOGICAL_FLOW_TABLE_FOR_EACH (lflow,
> l_ctx_in->logical_flow_table) {
> -        consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts,
> -                              &nd_ra_opts, &controller_event_opts, true,
> -                              l_ctx_in, l_ctx_out);
> +        consider_logical_flow(lflow, true, l_ctx_in, l_ctx_out);
>      }
> -
> -    dhcp_opts_destroy(&dhcp_opts);
> -    dhcp_opts_destroy(&dhcpv6_opts);
> -    nd_ra_opts_destroy(&nd_ra_opts);
> -    controller_event_opts_destroy(&controller_event_opts);
>  }
>
>  bool
> @@ -414,29 +380,6 @@ lflow_handle_changed_flows(struct lflow_ctx_in
> *l_ctx_in,
>      bool ret = true;
>      const struct sbrec_logical_flow *lflow;
>
> -    struct hmap dhcp_opts = HMAP_INITIALIZER(&dhcp_opts);
> -    struct hmap dhcpv6_opts = HMAP_INITIALIZER(&dhcpv6_opts);
> -    const struct sbrec_dhcp_options *dhcp_opt_row;
> -    SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row,
> -                                       l_ctx_in->dhcp_options_table) {
> -        dhcp_opt_add(&dhcp_opts, dhcp_opt_row->name, dhcp_opt_row->code,
> -                     dhcp_opt_row->type);
> -    }
> -
> -
> -    const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
> -    SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH (dhcpv6_opt_row,
> -                                         l_ctx_in->dhcpv6_options_table) {
> -       dhcp_opt_add(&dhcpv6_opts, dhcpv6_opt_row->name,
> dhcpv6_opt_row->code,
> -                    dhcpv6_opt_row->type);
> -    }
> -
> -    struct hmap nd_ra_opts = HMAP_INITIALIZER(&nd_ra_opts);
> -    nd_ra_opts_init(&nd_ra_opts);
> -
> -    struct controller_event_options controller_event_opts;
> -    controller_event_opts_init(&controller_event_opts);
> -
>      /* Flood remove the flows for all the tracked lflows.  Its possible
> that
>       * lflow_add_flows_for_datapath() may have been called before calling
>       * this function. */
> @@ -486,9 +429,7 @@ lflow_handle_changed_flows(struct lflow_ctx_in
> *l_ctx_in,
>                  lflows_processed_remove(l_ctx_out->lflows_processed,
> lfp_node);
>              }
>
> -            consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts,
> -                                  &nd_ra_opts, &controller_event_opts,
> false,
> -                                  l_ctx_in, l_ctx_out);
> +            consider_logical_flow(lflow, false, l_ctx_in, l_ctx_out);
>          }
>      }
>      HMAP_FOR_EACH_SAFE (ofrn, hmap_node, &flood_remove_nodes) {
> @@ -497,10 +438,6 @@ lflow_handle_changed_flows(struct lflow_ctx_in
> *l_ctx_in,
>      }
>      hmap_destroy(&flood_remove_nodes);
>
> -    dhcp_opts_destroy(&dhcp_opts);
> -    dhcp_opts_destroy(&dhcpv6_opts);
> -    nd_ra_opts_destroy(&nd_ra_opts);
> -    controller_event_opts_destroy(&controller_event_opts);
>      return ret;
>  }
>
> @@ -556,10 +493,6 @@ consider_lflow_for_added_as_ips__(
>                          const char *as_name,
>                          size_t as_ref_count,
>                          const struct expr_constant_set *as_diff_added,
> -                        struct hmap *dhcp_opts,
> -                        struct hmap *dhcpv6_opts,
> -                        struct hmap *nd_ra_opts,
> -                        struct controller_event_options
> *controller_event_opts,
>                          struct lflow_ctx_in *l_ctx_in,
>                          struct lflow_ctx_out *l_ctx_out)
>  {
> @@ -588,11 +521,10 @@ consider_lflow_for_added_as_ips__(
>      struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(ovnacts_stub);
>      struct ovnact_parse_params pp = {
>          .symtab = &symtab,
> -        .dhcp_opts = dhcp_opts,
> -        .dhcpv6_opts = dhcpv6_opts,
> -        .nd_ra_opts = nd_ra_opts,
> -        .controller_event_opts = controller_event_opts,
> -
> +        .dhcp_opts = l_ctx_in->dhcp_opts,
> +        .dhcpv6_opts = l_ctx_in->dhcpv6_opts,
> +        .nd_ra_opts = l_ctx_in->nd_ra_opts,
> +        .controller_event_opts = l_ctx_in->controller_event_opts,
>          .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
>          .n_tables = LOG_PIPELINE_LEN,
>          .cur_ltable = lflow->table_id,
> @@ -751,10 +683,6 @@ consider_lflow_for_added_as_ips(
>                          const char *as_name,
>                          size_t as_ref_count,
>                          const struct expr_constant_set *as_diff_added,
> -                        struct hmap *dhcp_opts,
> -                        struct hmap *dhcpv6_opts,
> -                        struct hmap *nd_ra_opts,
> -                        struct controller_event_options
> *controller_event_opts,
>                          struct lflow_ctx_in *l_ctx_in,
>                          struct lflow_ctx_out *l_ctx_out)
>  {
> @@ -771,17 +699,12 @@ consider_lflow_for_added_as_ips(
>      if (dp) {
>          return consider_lflow_for_added_as_ips__(lflow, dp, as_name,
>                                                   as_ref_count,
> as_diff_added,
> -                                                 dhcp_opts, dhcpv6_opts,
> -                                                 nd_ra_opts,
> -                                                 controller_event_opts,
>                                                   l_ctx_in, l_ctx_out);
>      }
>      for (size_t i = 0; dp_group && i < dp_group->n_datapaths; i++) {
>          if (!consider_lflow_for_added_as_ips__(lflow,
> dp_group->datapaths[i],
>                                                 as_name, as_ref_count,
> -                                               as_diff_added, dhcp_opts,
> -                                               dhcpv6_opts, nd_ra_opts,
> -                                               controller_event_opts,
> l_ctx_in,
> +                                               as_diff_added, l_ctx_in,
>                                                 l_ctx_out)) {
>              return false;
>          }
> @@ -886,30 +809,6 @@ lflow_handle_addr_set_update(const char *as_name,
>
>      *changed = false;
>
> -    struct hmap dhcp_opts = HMAP_INITIALIZER(&dhcp_opts);
> -    struct hmap dhcpv6_opts = HMAP_INITIALIZER(&dhcpv6_opts);
> -    struct hmap nd_ra_opts = HMAP_INITIALIZER(&nd_ra_opts);
> -    struct controller_event_options controller_event_opts;
> -
> -    if (as_diff->added) {
> -        const struct sbrec_dhcp_options *dhcp_opt_row;
> -        SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row,
> -                                           l_ctx_in->dhcp_options_table) {
> -            dhcp_opt_add(&dhcp_opts, dhcp_opt_row->name,
> dhcp_opt_row->code,
> -                         dhcp_opt_row->type);
> -        }
> -
> -        const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
> -        SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH(dhcpv6_opt_row,
> -
> l_ctx_in->dhcpv6_options_table) {
> -           dhcp_opt_add(&dhcpv6_opts, dhcpv6_opt_row->name,
> -                        dhcpv6_opt_row->code, dhcpv6_opt_row->type);
> -        }
> -
> -        nd_ra_opts_init(&nd_ra_opts);
> -        controller_event_opts_init(&controller_event_opts);
> -    }
> -
>      bool ret = true;
>      struct lflow_ref_list_node *lrln;
>      HMAP_FOR_EACH (lrln, hmap_node, &rlfn->lflow_uuids) {
> @@ -951,9 +850,7 @@ lflow_handle_addr_set_update(const char *as_name,
>          if (as_diff->added) {
>              if (!consider_lflow_for_added_as_ips(lflow, as_name,
>                                                   lrln->ref_count,
> -                                                 as_diff->added,
> &dhcp_opts,
> -                                                 &dhcpv6_opts,
> &nd_ra_opts,
> -                                                 &controller_event_opts,
> +                                                 as_diff->added,
>                                                   l_ctx_in, l_ctx_out)) {
>                  ret = false;
>                  goto done;
> @@ -962,12 +859,6 @@ lflow_handle_addr_set_update(const char *as_name,
>      }
>
>  done:
> -    if (as_diff->added) {
> -        dhcp_opts_destroy(&dhcp_opts);
> -        dhcp_opts_destroy(&dhcpv6_opts);
> -        nd_ra_opts_destroy(&nd_ra_opts);
> -        controller_event_opts_destroy(&controller_event_opts);
> -    }
>      return ret;
>  }
>
> @@ -1008,28 +899,6 @@ lflow_handle_changed_ref(enum ref_type ref_type,
> const char *ref_name,
>      }
>      *changed = true;
>
> -    struct hmap dhcp_opts = HMAP_INITIALIZER(&dhcp_opts);
> -    struct hmap dhcpv6_opts = HMAP_INITIALIZER(&dhcpv6_opts);
> -    const struct sbrec_dhcp_options *dhcp_opt_row;
> -    SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row,
> -                                       l_ctx_in->dhcp_options_table) {
> -        dhcp_opt_add(&dhcp_opts, dhcp_opt_row->name, dhcp_opt_row->code,
> -                     dhcp_opt_row->type);
> -    }
> -
> -    const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
> -    SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH(dhcpv6_opt_row,
> -                                        l_ctx_in->dhcpv6_options_table) {
> -       dhcp_opt_add(&dhcpv6_opts, dhcpv6_opt_row->name,
> dhcpv6_opt_row->code,
> -                    dhcpv6_opt_row->type);
> -    }
> -
> -    struct hmap nd_ra_opts = HMAP_INITIALIZER(&nd_ra_opts);
> -    nd_ra_opts_init(&nd_ra_opts);
> -
> -    struct controller_event_options controller_event_opts;
> -    controller_event_opts_init(&controller_event_opts);
> -
>      /* Re-parse the related lflows. */
>      /* Firstly, flood remove the flows from desired flow table. */
>      struct hmap flood_remove_nodes =
> HMAP_INITIALIZER(&flood_remove_nodes);
> @@ -1072,9 +941,7 @@ lflow_handle_changed_ref(enum ref_type ref_type,
> const char *ref_name,
>              lflows_processed_remove(l_ctx_out->lflows_processed,
> lfp_node);
>          }
>
> -        consider_logical_flow(lflow, &dhcp_opts, &dhcpv6_opts,
> -                              &nd_ra_opts, &controller_event_opts, false,
> -                              l_ctx_in, l_ctx_out);
> +        consider_logical_flow(lflow, false, l_ctx_in, l_ctx_out);
>      }
>      HMAP_FOR_EACH_SAFE (ofrn, hmap_node, &flood_remove_nodes) {
>          hmap_remove(&flood_remove_nodes, &ofrn->hmap_node);
> @@ -1082,10 +949,6 @@ lflow_handle_changed_ref(enum ref_type ref_type,
> const char *ref_name,
>      }
>      hmap_destroy(&flood_remove_nodes);
>
> -    dhcp_opts_destroy(&dhcp_opts);
> -    dhcp_opts_destroy(&dhcpv6_opts);
> -    nd_ra_opts_destroy(&nd_ra_opts);
> -    controller_event_opts_destroy(&controller_event_opts);
>      return ret;
>  }
>
> @@ -1308,9 +1171,6 @@ convert_match_to_expr(const struct
> sbrec_logical_flow *lflow,
>  static void
>  consider_logical_flow__(const struct sbrec_logical_flow *lflow,
>                          const struct sbrec_datapath_binding *dp,
> -                        struct hmap *dhcp_opts, struct hmap *dhcpv6_opts,
> -                        struct hmap *nd_ra_opts,
> -                        struct controller_event_options
> *controller_event_opts,
>                          struct lflow_ctx_in *l_ctx_in,
>                          struct lflow_ctx_out *l_ctx_out)
>  {
> @@ -1362,10 +1222,10 @@ consider_logical_flow__(const struct
> sbrec_logical_flow *lflow,
>      struct ofpbuf ovnacts = OFPBUF_STUB_INITIALIZER(ovnacts_stub);
>      struct ovnact_parse_params pp = {
>          .symtab = &symtab,
> -        .dhcp_opts = dhcp_opts,
> -        .dhcpv6_opts = dhcpv6_opts,
> -        .nd_ra_opts = nd_ra_opts,
> -        .controller_event_opts = controller_event_opts,
> +        .dhcp_opts = l_ctx_in->dhcp_opts,
> +        .dhcpv6_opts = l_ctx_in->dhcpv6_opts,
> +        .nd_ra_opts = l_ctx_in->nd_ra_opts,
> +        .controller_event_opts = l_ctx_in->controller_event_opts,
>
>          .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
>          .n_tables = LOG_PIPELINE_LEN,
> @@ -1580,9 +1440,6 @@ lflows_processed_destroy(struct hmap
> *lflows_processed)
>
>  static void
>  consider_logical_flow(const struct sbrec_logical_flow *lflow,
> -                      struct hmap *dhcp_opts, struct hmap *dhcpv6_opts,
> -                      struct hmap *nd_ra_opts,
> -                      struct controller_event_options
> *controller_event_opts,
>                        bool is_recompute,
>                        struct lflow_ctx_in *l_ctx_in,
>                        struct lflow_ctx_out *l_ctx_out)
> @@ -1606,16 +1463,11 @@ consider_logical_flow(const struct
> sbrec_logical_flow *lflow,
>      }
>
>      if (dp) {
> -        consider_logical_flow__(lflow, dp,
> -                                dhcp_opts, dhcpv6_opts, nd_ra_opts,
> -                                controller_event_opts,
> -                                l_ctx_in, l_ctx_out);
> +        consider_logical_flow__(lflow, dp, l_ctx_in, l_ctx_out);
>          return;
>      }
>      for (size_t i = 0; dp_group && i < dp_group->n_datapaths; i++) {
>          consider_logical_flow__(lflow, dp_group->datapaths[i],
> -                                dhcp_opts,  dhcpv6_opts, nd_ra_opts,
> -                                controller_event_opts,
>                                  l_ctx_in, l_ctx_out);
>      }
>  }
> @@ -2618,28 +2470,6 @@ lflow_add_flows_for_datapath(const struct
> sbrec_datapath_binding *dp,
>                               struct lflow_ctx_out *l_ctx_out)
>  {
>      bool handled = true;
> -    struct hmap dhcp_opts = HMAP_INITIALIZER(&dhcp_opts);
> -    struct hmap dhcpv6_opts = HMAP_INITIALIZER(&dhcpv6_opts);
> -    const struct sbrec_dhcp_options *dhcp_opt_row;
> -    SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row,
> -                                       l_ctx_in->dhcp_options_table) {
> -        dhcp_opt_add(&dhcp_opts, dhcp_opt_row->name, dhcp_opt_row->code,
> -                     dhcp_opt_row->type);
> -    }
> -
> -
> -    const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
> -    SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH (dhcpv6_opt_row,
> -                                         l_ctx_in->dhcpv6_options_table) {
> -       dhcp_opt_add(&dhcpv6_opts, dhcpv6_opt_row->name,
> dhcpv6_opt_row->code,
> -                    dhcpv6_opt_row->type);
> -    }
> -
> -    struct hmap nd_ra_opts = HMAP_INITIALIZER(&nd_ra_opts);
> -    nd_ra_opts_init(&nd_ra_opts);
> -
> -    struct controller_event_options controller_event_opts;
> -    controller_event_opts_init(&controller_event_opts);
>
>      struct sbrec_logical_flow *lf_row = sbrec_logical_flow_index_init_row(
>          l_ctx_in->sbrec_logical_flow_by_logical_datapath);
> @@ -2654,9 +2484,7 @@ lflow_add_flows_for_datapath(const struct
> sbrec_datapath_binding *dp,
>          }
>          lflows_processed_add(l_ctx_out->lflows_processed,
>                               &lflow->header_.uuid);
> -        consider_logical_flow__(lflow, dp, &dhcp_opts, &dhcpv6_opts,
> -                                &nd_ra_opts, &controller_event_opts,
> -                                l_ctx_in, l_ctx_out);
> +        consider_logical_flow__(lflow, dp, l_ctx_in, l_ctx_out);
>      }
>      sbrec_logical_flow_index_destroy_row(lf_row);
>
> @@ -2687,9 +2515,7 @@ lflow_add_flows_for_datapath(const struct
> sbrec_datapath_binding *dp,
>              /* Don't call lflows_processed_add() because here we process
> the
>               * lflow only for one of the DPs in the DP group, which may be
>               * incomplete. */
> -            consider_logical_flow__(lflow, dp, &dhcp_opts, &dhcpv6_opts,
> -                                    &nd_ra_opts, &controller_event_opts,
> -                                    l_ctx_in, l_ctx_out);
> +            consider_logical_flow__(lflow, dp, l_ctx_in, l_ctx_out);
>          }
>      }
>      sbrec_logical_flow_index_destroy_row(lf_row);
> @@ -2731,11 +2557,6 @@ lflow_add_flows_for_datapath(const struct
> sbrec_datapath_binding *dp,
>      }
>      sbrec_static_mac_binding_index_destroy_row(smb_index_row);
>
> -    dhcp_opts_destroy(&dhcp_opts);
> -    dhcp_opts_destroy(&dhcpv6_opts);
> -    nd_ra_opts_destroy(&nd_ra_opts);
> -    controller_event_opts_destroy(&controller_event_opts);
> -
>      /* Add load balancer hairpin flows if the datapath has any load
> balancers
>       * associated. */
>      for (size_t i = 0; i < n_dp_lbs; i++) {
> diff --git a/controller/lflow.h b/controller/lflow.h
> index 543d3cd96..a25634a88 100644
> --- a/controller/lflow.h
> +++ b/controller/lflow.h
> @@ -46,8 +46,6 @@ struct ovn_desired_flow_table;
>  struct hmap;
>  struct hmap_node;
>  struct sbrec_chassis;
> -struct sbrec_dhcp_options_table;
> -struct sbrec_dhcpv6_options_table;
>  struct sbrec_load_balancer;
>  struct sbrec_logical_flow_table;
>  struct sbrec_mac_binding_table;
> @@ -144,8 +142,6 @@ struct lflow_ctx_in {
>      struct ovsdb_idl_index *sbrec_mac_binding_by_datapath;
>      struct ovsdb_idl_index *sbrec_static_mac_binding_by_datapath;
>      const struct sbrec_port_binding_table *port_binding_table;
> -    const struct sbrec_dhcp_options_table *dhcp_options_table;
> -    const struct sbrec_dhcpv6_options_table *dhcpv6_options_table;
>      const struct sbrec_datapath_binding_table *dp_binding_table;
>      const struct sbrec_mac_binding_table *mac_binding_table;
>      const struct sbrec_logical_flow_table *logical_flow_table;
> @@ -162,6 +158,10 @@ struct lflow_ctx_in {
>      const struct sset *related_lport_ids;
>      const struct shash *binding_lports;
>      const struct hmap *chassis_tunnels;
> +    const struct hmap *nd_ra_opts;
> +    const struct hmap *dhcp_opts;
> +    const struct hmap *dhcpv6_opts;
> +    const struct controller_event_options *controller_event_opts;
>      bool lb_hairpin_use_ct_mark;
>  };
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index bc2c5ab63..e7bcb661e 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -76,6 +76,7 @@
>  #include "timer.h"
>  #include "stopwatch.h"
>  #include "lib/inc-proc-eng.h"
> +#include "lib/ovn-l7.h"
>  #include "hmapx.h"
>
>  VLOG_DEFINE_THIS_MODULE(main);
> @@ -2512,6 +2513,59 @@ en_northd_options_sb_sb_global_handler(struct
> engine_node *node, void *data)
>      return true;
>  }
>
> +struct ed_type_dhcp_options {
> +    struct hmap v4_opts;
> +    struct hmap v6_opts;
> +};
> +
> +static void *
> +en_dhcp_options_init(struct engine_node *node OVS_UNUSED,
> +                     struct engine_arg *arg OVS_UNUSED)
> +{
> +    struct ed_type_dhcp_options *dhcp_opts = xzalloc(sizeof *dhcp_opts);
> +
> +    hmap_init(&dhcp_opts->v4_opts);
> +    hmap_init(&dhcp_opts->v6_opts);
> +    return dhcp_opts;
> +}
> +
> +static void
> +en_dhcp_options_cleanup(void *data)
> +{
> +    struct ed_type_dhcp_options *dhcp_opts = data;
> +
> +    dhcp_opts_destroy(&dhcp_opts->v4_opts);
> +    dhcp_opts_destroy(&dhcp_opts->v6_opts);
> +}
> +
> +static void
> +en_dhcp_options_run(struct engine_node *node, void *data)
> +{
> +    struct ed_type_dhcp_options *dhcp_opts = data;
> +
> +    const struct sbrec_dhcp_options_table *dhcp_table =
> +        EN_OVSDB_GET(engine_get_input("SB_dhcp_options", node));
> +
> +    const struct sbrec_dhcpv6_options_table *dhcpv6_table =
> +        EN_OVSDB_GET(engine_get_input("SB_dhcpv6_options", node));
> +
> +    dhcp_opts_clear(&dhcp_opts->v4_opts);
> +    dhcp_opts_clear(&dhcp_opts->v6_opts);
> +
> +    const struct sbrec_dhcp_options *dhcp_opt_row;
> +    SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row, dhcp_table) {
> +        dhcp_opt_add(&dhcp_opts->v4_opts, dhcp_opt_row->name,
> +                     dhcp_opt_row->code, dhcp_opt_row->type);
> +    }
> +
> +    const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
> +    SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH (dhcpv6_opt_row, dhcpv6_table) {
> +       dhcp_opt_add(&dhcp_opts->v6_opts, dhcpv6_opt_row->name,
> +                    dhcpv6_opt_row->code, dhcpv6_opt_row->type);
> +    }
> +    engine_set_node_state(node, EN_UPDATED);
> +}
> +
>  struct lflow_output_persistent_data {
>      struct lflow_cache *lflow_cache;
>  };
> @@ -2544,6 +2598,12 @@ struct ed_type_lflow_output {
>
>      /* Data for managing hairpin flow conjunctive flow ids. */
>      struct lflow_output_hairpin_data hd;
> +
> +    /* Fixed neighbor discovery supported options. */
> +    struct hmap nd_ra_opts;
> +
> +    /* Fixed controller_event supported options. */
> +    struct controller_event_options controller_event_opts;
>  };
>
>  static void
> @@ -2590,12 +2650,6 @@ init_lflow_ctx(struct engine_node *node,
>      const struct sbrec_port_binding_table *port_binding_table =
>          EN_OVSDB_GET(engine_get_input("SB_port_binding", node));
>
> -    const struct sbrec_dhcp_options_table *dhcp_table =
> -        EN_OVSDB_GET(engine_get_input("SB_dhcp_options", node));
> -
> -    const struct sbrec_dhcpv6_options_table *dhcpv6_table =
> -        EN_OVSDB_GET(engine_get_input("SB_dhcpv6_options", node));
> -
>      const struct sbrec_mac_binding_table *mac_binding_table =
>          EN_OVSDB_GET(engine_get_input("SB_mac_binding", node));
>
> @@ -2649,6 +2703,9 @@ init_lflow_ctx(struct engine_node *node,
>      struct ed_type_northd_options *n_opts =
>          engine_get_input_data("northd_options", node);
>
> +    struct ed_type_dhcp_options *dhcp_opts =
> +        engine_get_input_data("dhcp_options", node);
> +
>      l_ctx_in->sbrec_multicast_group_by_name_datapath =
>          sbrec_mc_group_by_name_dp;
>      l_ctx_in->sbrec_logical_flow_by_logical_datapath =
> @@ -2661,8 +2718,6 @@ init_lflow_ctx(struct engine_node *node,
>      l_ctx_in->sbrec_static_mac_binding_by_datapath =
>          sbrec_static_mac_binding_by_datapath;
>      l_ctx_in->port_binding_table = port_binding_table;
> -    l_ctx_in->dhcp_options_table  = dhcp_table;
> -    l_ctx_in->dhcpv6_options_table = dhcpv6_table;
>      l_ctx_in->mac_binding_table = mac_binding_table;
>      l_ctx_in->logical_flow_table = logical_flow_table;
>      l_ctx_in->logical_dp_group_table = logical_dp_group_table;
> @@ -2679,6 +2734,10 @@ init_lflow_ctx(struct engine_node *node,
>      l_ctx_in->binding_lports = &rt_data->lbinding_data.lports;
>      l_ctx_in->chassis_tunnels = &non_vif_data->chassis_tunnels;
>      l_ctx_in->lb_hairpin_use_ct_mark = n_opts->lb_hairpin_use_ct_mark;
> +    l_ctx_in->nd_ra_opts = &fo->nd_ra_opts;
> +    l_ctx_in->dhcp_opts = &dhcp_opts->v4_opts;
> +    l_ctx_in->dhcpv6_opts = &dhcp_opts->v6_opts;
> +    l_ctx_in->controller_event_opts = &fo->controller_event_opts;
>
>      l_ctx_out->flow_table = &fo->flow_table;
>      l_ctx_out->group_table = &fo->group_table;
> @@ -2704,6 +2763,8 @@ en_lflow_output_init(struct engine_node *node
> OVS_UNUSED,
>      hmap_init(&data->lflows_processed);
>      simap_init(&data->hd.ids);
>      data->hd.pool = id_pool_create(1, UINT32_MAX - 1);
> +    nd_ra_opts_init(&data->nd_ra_opts);
> +    controller_event_opts_init(&data->controller_event_opts);
>      return data;
>  }
>
> @@ -2728,6 +2789,8 @@ en_lflow_output_cleanup(void *data)
>      lflow_cache_destroy(flow_output_data->pd.lflow_cache);
>      simap_destroy(&flow_output_data->hd.ids);
>      id_pool_destroy(flow_output_data->hd.pool);
> +    nd_ra_opts_destroy(&flow_output_data->nd_ra_opts);
> +
> controller_event_opts_destroy(&flow_output_data->controller_event_opts);
>  }
>
>  static void
> @@ -3645,6 +3708,7 @@ main(int argc, char *argv[])
>      ENGINE_NODE_WITH_CLEAR_TRACK_DATA(addr_sets, "addr_sets");
>      ENGINE_NODE_WITH_CLEAR_TRACK_DATA(port_groups, "port_groups");
>      ENGINE_NODE(northd_options, "northd_options");
> +    ENGINE_NODE(dhcp_options, "dhcp_options");
>
>  #define SB_NODE(NAME, NAME_STR) ENGINE_NODE_SB(NAME, NAME_STR);
>      SB_NODES
> @@ -3704,7 +3768,11 @@ main(int argc, char *argv[])
>      engine_add_input(&en_northd_options, &en_sb_sb_global,
>                       en_northd_options_sb_sb_global_handler);
>
> +    engine_add_input(&en_dhcp_options, &en_sb_dhcp_options, NULL);
> +    engine_add_input(&en_dhcp_options, &en_sb_dhcpv6_options, NULL);
> +
>      engine_add_input(&en_lflow_output, &en_northd_options, NULL);
> +    engine_add_input(&en_lflow_output, &en_dhcp_options, NULL);
>
>      /* Keep en_addr_sets before en_runtime_data because
>       * lflow_output_runtime_data_handler may *partially* reprocess a
> lflow when
> @@ -3747,8 +3815,6 @@ main(int argc, char *argv[])
>       * process all changes. */
>      engine_add_input(&en_lflow_output, &en_sb_logical_dp_group,
>                       engine_noop_handler);
> -    engine_add_input(&en_lflow_output, &en_sb_dhcp_options, NULL);
> -    engine_add_input(&en_lflow_output, &en_sb_dhcpv6_options, NULL);
>      engine_add_input(&en_lflow_output, &en_sb_dns, NULL);
>      engine_add_input(&en_lflow_output, &en_sb_load_balancer,
>                       lflow_output_sb_load_balancer_handler);
> diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
> index 0b2da9f7b..b03b945d8 100644
> --- a/lib/ovn-l7.h
> +++ b/lib/ovn-l7.h
> @@ -202,7 +202,7 @@ dhcp_opt_add(struct hmap *dhcp_opts, char *opt_name,
> size_t code, char *type)
>  }
>
>  static inline void
> -gen_opts_destroy(struct hmap *gen_opts)
> +gen_opts_clear(struct hmap *gen_opts)
>  {
>      struct gen_opts_map *gen_opt;
>      HMAP_FOR_EACH_POP (gen_opt, hmap_node, gen_opts) {
> @@ -210,6 +210,12 @@ gen_opts_destroy(struct hmap *gen_opts)
>          free(gen_opt->type);
>          free(gen_opt);
>      }
> +}
> +
> +static inline void
> +gen_opts_destroy(struct hmap *gen_opts)
> +{
> +    gen_opts_clear(gen_opts);
>      hmap_destroy(gen_opts);
>  }
>
> @@ -219,6 +225,12 @@ dhcp_opts_destroy(struct hmap *dhcp_opts)
>      gen_opts_destroy(dhcp_opts);
>  }
>
> +static inline void
> +dhcp_opts_clear(struct hmap *dhcp_opts)
> +{
> +    gen_opts_clear(dhcp_opts);
> +}
> +
>  OVS_PACKED(
>  struct dhcp_opt_header {
>      uint8_t code;
> @@ -412,6 +424,8 @@ nd_ra_opts_destroy(struct hmap *nd_ra_opts)
>  static inline void
>  nd_ra_opts_init(struct hmap *nd_ra_opts)
>  {
> +    hmap_init(nd_ra_opts);
> +
>      nd_ra_opt_add(nd_ra_opts, "addr_mode", ND_RA_FLAG_ADDR_MODE, "str");
>      nd_ra_opt_add(nd_ra_opts, "router_preference", ND_RA_FLAG_PRF, "str");
>      nd_ra_opt_add(nd_ra_opts, "slla", ND_OPT_SOURCE_LINKADDR, "mac");
> --
> 2.31.1
>
>
Looks good to me, thanks.

Acked-by: Ales Musil <[email protected]>

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA <https://www.redhat.com>

[email protected]    IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to