On Wed, Sep 14, 2022 at 6:09 AM Dumitru Ceara <dce...@redhat.com> wrote:
>
Thanks Dumitru for the improvement.
> 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 incrementally.
Maybe a slight correction here. Before this patch it wasn't rebuilding the
maps for *every logical flow*. For lflow changes, it was performed once for
all the changed lflows. However, it was indeed inefficient for "reference"
changes such as port-bindings, where it was performed for every
port-binding.
>
> An even more efficient solution would be to introduce proper incremental
> processing for the DHCP opt maps but that seems like too much complexity
> without enough benefit. It's probably OK to recompute the maps at every
> ovn-controller iteration.
Well, every iteration for every handler, but still, I think it is totally
OK in terms of performance (and already better than before).
However, there may be a benefit of splitting the DHCP opt to a separate I-P
node, as input to the lflow_output node. It would avoid introducing the
destroy_lflow_ctx(). I don't see complexity here because the dependency
looks quite clear. But I am ok either way for this patch.
For the destroy_lflow_ctx(), if we want to keep it, I'd remove the
l_ctx_out parameter, because the output is essentially the data of the
lflow_output engine node, and naturally shouldn't contain anything to be
destroyed here.
Please see one more finding below.
>
> Signed-off-by: Dumitru Ceara <dce...@redhat.com>
> ---
> controller/lflow.c | 206
+++----------------------------------------
> controller/lflow.h | 9 +-
> controller/ovn-controller.c | 128 +++++++++++++++++++--------
> lib/ovn-l7.h | 2
> 4 files changed, 110 insertions(+), 235 deletions(-)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index eef44389f..de9f17b9a 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,10 +521,6 @@ 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,
I think this should be replaced with l_ctx_in->xxx, instead of deleting
them.
Thanks,
Han
>
> .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS,
> .n_tables = LOG_PIPELINE_LEN,
> @@ -751,10 +680,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 +696,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 +806,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 +847,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 +856,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 +896,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 +938,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 +946,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 +1168,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 +1219,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 +1437,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 +1460,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 +2467,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 +2481,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 +2512,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 +2554,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..3d1bb38dd 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,7 +158,12 @@ 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 controller_event_options *controller_event_opts;
> bool lb_hairpin_use_ct_mark;
> +
> + struct hmap dhcp_opts;
> + struct hmap dhcpv6_opts;
> };
>
> struct lflow_ctx_out {
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index 6bedb91dc..18a01bbab 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);
> @@ -2538,6 +2539,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
> @@ -2655,8 +2662,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;
> @@ -2673,6 +2678,22 @@ 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->controller_event_opts = &fo->controller_event_opts;
> +
> + hmap_init(&l_ctx_in->dhcp_opts);
> + const struct sbrec_dhcp_options *dhcp_opt_row;
> + SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row, dhcp_table) {
> + dhcp_opt_add(&l_ctx_in->dhcp_opts, dhcp_opt_row->name,
> + dhcp_opt_row->code, dhcp_opt_row->type);
> + }
> +
> + hmap_init(&l_ctx_in->dhcpv6_opts);
> + const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
> + SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH (dhcpv6_opt_row, dhcpv6_table) {
> + dhcp_opt_add(&l_ctx_in->dhcpv6_opts, dhcpv6_opt_row->name,
> + dhcpv6_opt_row->code, dhcpv6_opt_row->type);
> + }
>
> l_ctx_out->flow_table = &fo->flow_table;
> l_ctx_out->group_table = &fo->group_table;
> @@ -2685,6 +2706,14 @@ init_lflow_ctx(struct engine_node *node,
> l_ctx_out->hairpin_lb_ids = &fo->hd.ids;
> }
>
> +static void
> +destroy_lflow_ctx(struct lflow_ctx_in *l_ctx_in,
> + struct lflow_ctx_out *l_ctx_out OVS_UNUSED)
> +{
> + dhcp_opts_destroy(&l_ctx_in->dhcp_opts);
> + dhcp_opts_destroy(&l_ctx_in->dhcpv6_opts);
> +}
> +
> static void *
> en_lflow_output_init(struct engine_node *node OVS_UNUSED,
> struct engine_arg *arg OVS_UNUSED)
> @@ -2698,6 +2727,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;
> }
>
> @@ -2722,6 +2753,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
> @@ -2771,6 +2804,7 @@ en_lflow_output_run(struct engine_node *node, void
*data)
> struct lflow_ctx_out l_ctx_out;
> init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out);
> lflow_run(&l_ctx_in, &l_ctx_out);
> + destroy_lflow_ctx(&l_ctx_in, &l_ctx_out);
>
> engine_set_node_state(node, EN_UPDATED);
> }
> @@ -2784,6 +2818,7 @@ lflow_output_sb_logical_flow_handler(struct
engine_node *node, void *data)
> init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out);
>
> bool handled = lflow_handle_changed_flows(&l_ctx_in, &l_ctx_out);
> + destroy_lflow_ctx(&l_ctx_in, &l_ctx_out);
>
> engine_set_node_state(node, EN_UPDATED);
> return handled;
> @@ -2846,12 +2881,12 @@ lflow_output_sb_multicast_group_handler(struct
engine_node *node, void *data)
> struct lflow_ctx_in l_ctx_in;
> struct lflow_ctx_out l_ctx_out;
> init_lflow_ctx(node, lfo, &l_ctx_in, &l_ctx_out);
> - if (!lflow_handle_changed_mc_groups(&l_ctx_in, &l_ctx_out)) {
> - return false;
> - }
> +
> + bool handled = lflow_handle_changed_mc_groups(&l_ctx_in, &l_ctx_out);
> + destroy_lflow_ctx(&l_ctx_in, &l_ctx_out);
>
> engine_set_node_state(node, EN_UPDATED);
> - return true;
> + return handled;
> }
>
> static bool
> @@ -2862,12 +2897,12 @@ lflow_output_sb_port_binding_handler(struct
engine_node *node, void *data)
> struct lflow_ctx_in l_ctx_in;
> struct lflow_ctx_out l_ctx_out;
> init_lflow_ctx(node, lfo, &l_ctx_in, &l_ctx_out);
> - if (!lflow_handle_changed_port_bindings(&l_ctx_in, &l_ctx_out)) {
> - return false;
> - }
> +
> + bool handled = lflow_handle_changed_port_bindings(&l_ctx_in,
&l_ctx_out);
> + destroy_lflow_ctx(&l_ctx_in, &l_ctx_out);
>
> engine_set_node_state(node, EN_UPDATED);
> - return true;
> + return handled;
> }
>
> static bool
> @@ -2876,23 +2911,24 @@ lflow_output_addr_sets_handler(struct engine_node
*node, void *data)
> struct ed_type_addr_sets *as_data =
> engine_get_input_data("addr_sets", node);
>
> - struct ed_type_lflow_output *fo = data;
> + if (!as_data->change_tracked) {
> + return false;
> + }
>
> - struct lflow_ctx_in l_ctx_in;
> + struct ed_type_lflow_output *fo = data;
> struct lflow_ctx_out l_ctx_out;
> - init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out);
> -
> - bool changed;
> + struct lflow_ctx_in l_ctx_in;
> const char *ref_name;
> + bool handled = true;
> + bool changed;
>
> - if (!as_data->change_tracked) {
> - return false;
> - }
> + init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out);
>
> SSET_FOR_EACH (ref_name, &as_data->deleted) {
> if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET, ref_name,
&l_ctx_in,
> &l_ctx_out, &changed)) {
> - return false;
> + handled = false;
> + goto done;
> }
> if (changed) {
> engine_set_node_state(node, EN_UPDATED);
> @@ -2907,7 +2943,8 @@ lflow_output_addr_sets_handler(struct engine_node
*node, void *data)
> " Reprocess related lflows.", shash_node->name);
> if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET,
shash_node->name,
> &l_ctx_in, &l_ctx_out,
&changed)) {
> - return false;
> + handled = false;
> + goto done;
> }
> }
> if (changed) {
> @@ -2917,14 +2954,17 @@ lflow_output_addr_sets_handler(struct engine_node
*node, void *data)
> SSET_FOR_EACH (ref_name, &as_data->new) {
> if (!lflow_handle_changed_ref(REF_TYPE_ADDRSET, ref_name,
&l_ctx_in,
> &l_ctx_out, &changed)) {
> - return false;
> + handled = false;
> + goto done;
> }
> if (changed) {
> engine_set_node_state(node, EN_UPDATED);
> }
> }
>
> - return true;
> +done:
> + destroy_lflow_ctx(&l_ctx_in, &l_ctx_out);
> + return handled;
> }
>
> static bool
> @@ -2933,23 +2973,24 @@ lflow_output_port_groups_handler(struct
engine_node *node, void *data)
> struct ed_type_port_groups *pg_data =
> engine_get_input_data("port_groups", node);
>
> - struct ed_type_lflow_output *fo = data;
> + if (!pg_data->change_tracked) {
> + return false;
> + }
>
> - struct lflow_ctx_in l_ctx_in;
> + struct ed_type_lflow_output *fo = data;
> struct lflow_ctx_out l_ctx_out;
> - init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out);
> -
> - bool changed;
> + struct lflow_ctx_in l_ctx_in;
> const char *ref_name;
> + bool handled = true;
> + bool changed;
>
> - if (!pg_data->change_tracked) {
> - return false;
> - }
> + init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out);
>
> SSET_FOR_EACH (ref_name, &pg_data->deleted) {
> if (!lflow_handle_changed_ref(REF_TYPE_PORTGROUP, ref_name,
&l_ctx_in,
> &l_ctx_out, &changed)) {
> - return false;
> + handled = false;
> + goto done;
> }
> if (changed) {
> engine_set_node_state(node, EN_UPDATED);
> @@ -2958,7 +2999,8 @@ lflow_output_port_groups_handler(struct engine_node
*node, void *data)
> SSET_FOR_EACH (ref_name, &pg_data->updated) {
> if (!lflow_handle_changed_ref(REF_TYPE_PORTGROUP, ref_name,
&l_ctx_in,
> &l_ctx_out, &changed)) {
> - return false;
> + handled = false;
> + goto done;
> }
> if (changed) {
> engine_set_node_state(node, EN_UPDATED);
> @@ -2967,14 +3009,17 @@ lflow_output_port_groups_handler(struct
engine_node *node, void *data)
> SSET_FOR_EACH (ref_name, &pg_data->new) {
> if (!lflow_handle_changed_ref(REF_TYPE_PORTGROUP, ref_name,
&l_ctx_in,
> &l_ctx_out, &changed)) {
> - return false;
> + handled = false;
> + goto done;
> }
> if (changed) {
> engine_set_node_state(node, EN_UPDATED);
> }
> }
>
> - return true;
> +done:
> + destroy_lflow_ctx(&l_ctx_in, &l_ctx_out);
> + return handled;
> }
>
> static bool
> @@ -3002,6 +3047,8 @@ lflow_output_runtime_data_handler(struct
engine_node *node,
> struct lflow_ctx_out l_ctx_out;
> struct ed_type_lflow_output *fo = data;
> struct hmap *lbs = NULL;
> + bool handled = true;
> +
> init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out);
>
> struct tracked_datapath *tdp;
> @@ -3018,7 +3065,8 @@ lflow_output_runtime_data_handler(struct
engine_node *node,
> tdp->dp, lbs_by_dp ? lbs_by_dp->dp_lbs : NULL,
> lbs_by_dp ? lbs_by_dp->n_dp_lbs : 0,
> &l_ctx_in, &l_ctx_out)) {
> - return false;
> + handled = false;
> + goto done;
> }
> }
> struct shash_node *shash_node;
> @@ -3026,14 +3074,18 @@ lflow_output_runtime_data_handler(struct
engine_node *node,
> struct tracked_lport *lport = shash_node->data;
> if (!lflow_handle_flows_for_lport(lport->pb, &l_ctx_in,
> &l_ctx_out)) {
> - return false;
> + handled = false;
> + goto done;
> }
> }
> }
>
> load_balancers_by_dp_cleanup(lbs);
> engine_set_node_state(node, EN_UPDATED);
> - return true;
> +
> +done:
> + destroy_lflow_ctx(&l_ctx_in, &l_ctx_out);
> + return handled;
> }
>
> static bool
> @@ -3045,6 +3097,7 @@ lflow_output_sb_load_balancer_handler(struct
engine_node *node, void *data)
> init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out);
>
> bool handled = lflow_handle_changed_lbs(&l_ctx_in, &l_ctx_out);
> + destroy_lflow_ctx(&l_ctx_in, &l_ctx_out);
>
> engine_set_node_state(node, EN_UPDATED);
> return handled;
> @@ -3059,6 +3112,7 @@ lflow_output_sb_fdb_handler(struct engine_node
*node, void *data)
> init_lflow_ctx(node, fo, &l_ctx_in, &l_ctx_out);
>
> bool handled = lflow_handle_changed_fdbs(&l_ctx_in, &l_ctx_out);
> + destroy_lflow_ctx(&l_ctx_in, &l_ctx_out);
>
> engine_set_node_state(node, EN_UPDATED);
> return handled;
> diff --git a/lib/ovn-l7.h b/lib/ovn-l7.h
> index 0b2da9f7b..2c7d0add4 100644
> --- a/lib/ovn-l7.h
> +++ b/lib/ovn-l7.h
> @@ -412,6 +412,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");
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
