| Message ID | 20220926101816.36944-1-dceara@redhat.com |
|---|---|
| State | Accepted |
| Delegated to: | Dumitru Ceara |
| Headers | show |
| Series | [ovs-dev,v2] controller: Avoid building dhcp/nd_ra/controller_event opt maps every time. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On Mon, Sep 26, 2022 at 12:18 PM Dumitru Ceara <dceara@redhat.com> 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 <dceara@redhat.com> > --- > 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 <amusil@redhat.com>
On 9/30/22 08:16, Ales Musil wrote: > Looks good to me, thanks. > > Acked-by: Ales Musil <amusil@redhat.com> Thanks, Ales and Han! I applied this to the main branch. Regards, Dumitru
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");
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 <dceara@redhat.com> --- 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(-)