diff mbox series

[ovs-dev,v3,11/16] northd: Handle lb changes in lflow engine.

Message ID 20231128023711.569923-1-numans@ovn.org
State Changes Requested
Headers show
Series northd lflow incremental processing | expand

Checks

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
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes success github build: passed

Commit Message

Numan Siddique Nov. 28, 2023, 2:37 a.m. UTC
From: Numan Siddique <numans@ovn.org>

Since northd tracked data has the changed lb data, northd
engine handler for lflow engine now handles the lb changes
incrementally.  All the lflows generated for each lb is
stored in the ovn_lb_datapaths->lflow_ref and this is used
similar to how we handle ovn_port changes.

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 northd/en-lflow.c   | 11 +++---
 northd/lflow-mgr.c  | 41 ++++++++++++++++++-
 northd/lflow-mgr.h  |  3 ++
 northd/northd.c     | 95 +++++++++++++++++++++++++++++++++++++++++----
 northd/northd.h     | 28 +++++++++++++
 tests/ovn-northd.at | 30 ++++++++++----
 6 files changed, 186 insertions(+), 22 deletions(-)

Comments

Dumitru Ceara Dec. 13, 2023, 12:15 p.m. UTC | #1
On 11/28/23 03:37, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> Since northd tracked data has the changed lb data, northd
> engine handler for lflow engine now handles the lb changes
> incrementally.  All the lflows generated for each lb is
> stored in the ovn_lb_datapaths->lflow_ref and this is used
> similar to how we handle ovn_port changes.
> 
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>  northd/en-lflow.c   | 11 +++---
>  northd/lflow-mgr.c  | 41 ++++++++++++++++++-
>  northd/lflow-mgr.h  |  3 ++
>  northd/northd.c     | 95 +++++++++++++++++++++++++++++++++++++++++----
>  northd/northd.h     | 28 +++++++++++++
>  tests/ovn-northd.at | 30 ++++++++++----
>  6 files changed, 186 insertions(+), 22 deletions(-)
> 
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 65d2f45ebc..13284b5556 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -125,11 +125,6 @@ lflow_northd_handler(struct engine_node *node,
>          return false;
>      }
>  
> -    /* Fall back to recompute if load balancers have changed. */
> -    if (northd_has_lbs_in_tracked_data(&northd_data->trk_data)) {
> -        return false;
> -    }
> -
>      const struct engine_context *eng_ctx = engine_get_context();
>      struct lflow_data *lflow_data = data;
>  
> @@ -142,6 +137,12 @@ lflow_northd_handler(struct engine_node *node,
>          return false;
>      }
>  
> +    if (!lflow_handle_northd_lb_changes(eng_ctx->ovnsb_idl_txn,
> +                                &northd_data->trk_data.trk_lbs,
> +                                &lflow_input, lflow_data->lflow_table)) {
> +        return false;
> +    }
> +
>      engine_set_node_state(node, EN_UPDATED);
>      return true;
>  }
> diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
> index 08962e9172..d779e7e087 100644
> --- a/northd/lflow-mgr.c
> +++ b/northd/lflow-mgr.c
> @@ -105,6 +105,10 @@ static void ovn_dp_group_add_with_reference(struct ovn_lflow *,
>                                              size_t bitmap_len);
>  
>  static void unlink_lflows_from_datapath(struct lflow_ref *);
> +static void unlink_lflows_from_all_datapaths(struct lflow_ref *,
> +                                             size_t n_ls_datapaths,
> +                                             size_t n_lr_datapaths);
> +

lflow_ref_clear_and_sync_lflows() takes 'const struct ovn_datapaths
*ls_datapaths' and 'const struct ovn_datapaths *lr_datapaths'.  Let's do
that here too to be consistent.

>  static void lflow_ref_sync_lflows_to_sb__(struct lflow_ref  *,
>                              struct lflow_table *,
>                              struct ovsdb_idl_txn *ovnsb_txn,
> @@ -394,6 +398,15 @@ lflow_ref_clear_lflows(struct lflow_ref *lflow_ref)
>      unlink_lflows_from_datapath(lflow_ref);
>  }
>  
> +void
> +lflow_ref_clear_lflows_for_all_dps(struct lflow_ref *lflow_ref,
> +                                   size_t n_ls_datapaths,
> +                                   size_t n_lr_datapaths)

Same comment about consistency here too.

While we're at it, all we care is the max bitmap index for a 'struct
ovn_lflow'->dpg_bitmap.  We don't really need to pass all these
ls_datapaths/lr_datapaths everywhere, I think.  Can't we just use
'struct ovn_lflow'->n_ods?

Alternatively, can 'struct lflow_ref' store a pointer to switch/router
'struct ovn_datapaths'?

> +{
> +    unlink_lflows_from_all_datapaths(lflow_ref, n_ls_datapaths,
> +                                     n_lr_datapaths);
> +}
> +
>  void
>  lflow_ref_clear_and_sync_lflows(struct lflow_ref *lflow_ref,
>                      struct lflow_table *lflow_table,
> @@ -462,7 +475,9 @@ lflow_table_add_lflow(struct lflow_table *lflow_table,
>              /*  lflow_ref_node for this lflow doesn't exist yet.  Add it. */
>              struct lflow_ref_node *ref_node = xzalloc(sizeof *ref_node);
>              ref_node->lflow = lflow;
> -            ref_node->dp_index = od->index;
> +            if (od) {
> +                ref_node->dp_index = od->index;
> +            }
>              ovs_list_insert(&lflow_ref->lflows_ref_list,
>                              &ref_node->ref_list_node);
>  
> @@ -1047,6 +1062,30 @@ unlink_lflows_from_datapath(struct lflow_ref *lflow_ref)
>      }
>  }
>  
> +static void
> +unlink_lflows_from_all_datapaths(struct lflow_ref *lflow_ref,
> +                                 size_t n_ls_datapaths,
> +                                 size_t n_lr_datapaths)
> +{
> +    struct lflow_ref_node *ref_node;
> +    struct ovn_lflow *lflow;
> +    LIST_FOR_EACH (ref_node, ref_list_node, &lflow_ref->lflows_ref_list) {
> +        size_t n_datapaths;
> +        size_t index;
> +
> +        lflow = ref_node->lflow;
> +        if (ovn_stage_to_datapath_type(lflow->stage) == DP_SWITCH) {
> +            n_datapaths = n_ls_datapaths;
> +        } else {
> +            n_datapaths = n_lr_datapaths;
> +        }
> +
> +        BITMAP_FOR_EACH_1 (index, n_datapaths, lflow->dpg_bitmap) {
> +            bitmap_set0(lflow->dpg_bitmap, index);
> +        }
> +    }
> +}
> +
>  static void
>  lflow_ref_sync_lflows_to_sb__(struct lflow_ref  *lflow_ref,
>                          struct lflow_table *lflow_table,
> diff --git a/northd/lflow-mgr.h b/northd/lflow-mgr.h
> index 02b74aa131..c65cd70e71 100644
> --- a/northd/lflow-mgr.h
> +++ b/northd/lflow-mgr.h
> @@ -54,6 +54,9 @@ struct lflow_ref *lflow_ref_alloc(const char *res_name);
>  void lflow_ref_destroy(struct lflow_ref *);
>  void lflow_ref_reset(struct lflow_ref *lflow_ref);
>  void lflow_ref_clear_lflows(struct lflow_ref *);
> +void lflow_ref_clear_lflows_for_all_dps(struct lflow_ref *,
> +                                        size_t n_ls_datapaths,
> +                                        size_t n_lr_datapaths);
>  void lflow_ref_clear_and_sync_lflows(struct lflow_ref *,
>                                  struct lflow_table *lflow_table,
>                                  struct ovsdb_idl_txn *ovnsb_txn,
> diff --git a/northd/northd.c b/northd/northd.c
> index 104068e293..f56916b3da 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -3523,6 +3523,7 @@ ovn_lb_datapaths_create(const struct ovn_northd_lb *lb, size_t n_ls_datapaths,
>      lb_dps->lb = lb;
>      lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths);
>      lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths);
> +    lb_dps->lflow_ref = lflow_ref_alloc(lb->nlb->name);
>  
>      return lb_dps;
>  }
> @@ -3532,6 +3533,7 @@ ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *lb_dps)
>  {
>      bitmap_free(lb_dps->nb_lr_map);
>      bitmap_free(lb_dps->nb_ls_map);
> +    lflow_ref_destroy(lb_dps->lflow_ref);
>      free(lb_dps);
>  }
>  
> @@ -16063,11 +16065,11 @@ build_lflows_thread(void *arg)
>                                                           lsi->lflows,
>                                                           &lsi->match,
>                                                           &lsi->actions,
> -                                                         NULL);
> +                                                         lb_dps->lflow_ref);
>                      build_lrouter_defrag_flows_for_lb(lb_dps, lsi->lflows,
>                                                        lsi->lr_datapaths,
>                                                        &lsi->match,
> -                                                      NULL);
> +                                                      lb_dps->lflow_ref);
>                      build_lrouter_flows_for_lb(lb_dps, lsi->lflows,
>                                                 lsi->meter_groups,
>                                                 lsi->lr_datapaths,
> @@ -16075,14 +16077,14 @@ build_lflows_thread(void *arg)
>                                                 lsi->features,
>                                                 lsi->svc_monitor_map,
>                                                 &lsi->match, &lsi->actions,
> -                                               NULL);
> +                                               lb_dps->lflow_ref);
>                      build_lswitch_flows_for_lb(lb_dps, lsi->lflows,
>                                                 lsi->meter_groups,
>                                                 lsi->ls_datapaths,
>                                                 lsi->features,
>                                                 lsi->svc_monitor_map,
>                                                 &lsi->match, &lsi->actions,
> -                                               NULL);
> +                                               lb_dps->lflow_ref);
>                  }
>              }
>              for (bnum = control->id;
> @@ -16298,20 +16300,20 @@ build_lswitch_and_lrouter_flows(const struct ovn_datapaths *ls_datapaths,
>              build_lswitch_arp_nd_service_monitor(lb_dps->lb, lsi.ls_ports,
>                                                   lsi.lflows, &lsi.actions,
>                                                   &lsi.match,
> -                                                 NULL);
> +                                                 lb_dps->lflow_ref);
>              build_lrouter_defrag_flows_for_lb(lb_dps, lsi.lflows,
>                                                lsi.lr_datapaths, &lsi.match,
> -                                              NULL);
> +                                              lb_dps->lflow_ref);
>              build_lrouter_flows_for_lb(lb_dps, lsi.lflows, lsi.meter_groups,
>                                         lsi.lr_datapaths, lsi.lr_sful_table,
>                                         lsi.features, lsi.svc_monitor_map,
>                                         &lsi.match, &lsi.actions,
> -                                       NULL);
> +                                       lb_dps->lflow_ref);
>              build_lswitch_flows_for_lb(lb_dps, lsi.lflows, lsi.meter_groups,
>                                         lsi.ls_datapaths, lsi.features,
>                                         lsi.svc_monitor_map,
>                                         &lsi.match, &lsi.actions,
> -                                       NULL);
> +                                       lb_dps->lflow_ref);
>          }
>          stopwatch_stop(LFLOWS_LBS_STOPWATCH_NAME, time_msec());
>  
> @@ -16483,11 +16485,17 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn,
>  void
>  reset_lflow_refs_for_northd_resources(struct lflow_input *lflow_input)
>  {
> +    struct ovn_lb_datapaths *lb_dps;
>      struct ovn_port *op;
> +

Nit: this whitespace change should've went in patch 08/16 (northd:
Refactor lflow management into a separate module.)

>      HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) {
>          lflow_ref_reset(op->lflow_ref);
>          lflow_ref_reset(op->stateful_lflow_ref);
>      }
> +
> +    HMAP_FOR_EACH (lb_dps, hmap_node, lflow_input->lb_datapaths_map) {
> +        lflow_ref_reset(lb_dps->lflow_ref);
> +    }
>  }
>  

[...]

Regards,
Dumitru
Han Zhou Dec. 18, 2023, 7:13 a.m. UTC | #2
On Mon, Nov 27, 2023 at 6:38 PM <numans@ovn.org> wrote:
>
> From: Numan Siddique <numans@ovn.org>
>
> Since northd tracked data has the changed lb data, northd
> engine handler for lflow engine now handles the lb changes
> incrementally.  All the lflows generated for each lb is
> stored in the ovn_lb_datapaths->lflow_ref and this is used
> similar to how we handle ovn_port changes.
>
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---
>  northd/en-lflow.c   | 11 +++---
>  northd/lflow-mgr.c  | 41 ++++++++++++++++++-
>  northd/lflow-mgr.h  |  3 ++
>  northd/northd.c     | 95 +++++++++++++++++++++++++++++++++++++++++----
>  northd/northd.h     | 28 +++++++++++++
>  tests/ovn-northd.at | 30 ++++++++++----
>  6 files changed, 186 insertions(+), 22 deletions(-)
>
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 65d2f45ebc..13284b5556 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -125,11 +125,6 @@ lflow_northd_handler(struct engine_node *node,
>          return false;
>      }
>
> -    /* Fall back to recompute if load balancers have changed. */
> -    if (northd_has_lbs_in_tracked_data(&northd_data->trk_data)) {
> -        return false;
> -    }
> -
>      const struct engine_context *eng_ctx = engine_get_context();
>      struct lflow_data *lflow_data = data;
>
> @@ -142,6 +137,12 @@ lflow_northd_handler(struct engine_node *node,
>          return false;
>      }
>
> +    if (!lflow_handle_northd_lb_changes(eng_ctx->ovnsb_idl_txn,
> +                                &northd_data->trk_data.trk_lbs,
> +                                &lflow_input, lflow_data->lflow_table)) {
> +        return false;
> +    }
> +
>      engine_set_node_state(node, EN_UPDATED);
>      return true;
>  }
> diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
> index 08962e9172..d779e7e087 100644
> --- a/northd/lflow-mgr.c
> +++ b/northd/lflow-mgr.c
> @@ -105,6 +105,10 @@ static void ovn_dp_group_add_with_reference(struct
ovn_lflow *,
>                                              size_t bitmap_len);
>
>  static void unlink_lflows_from_datapath(struct lflow_ref *);
> +static void unlink_lflows_from_all_datapaths(struct lflow_ref *,
> +                                             size_t n_ls_datapaths,
> +                                             size_t n_lr_datapaths);
> +
>  static void lflow_ref_sync_lflows_to_sb__(struct lflow_ref  *,
>                              struct lflow_table *,
>                              struct ovsdb_idl_txn *ovnsb_txn,
> @@ -394,6 +398,15 @@ lflow_ref_clear_lflows(struct lflow_ref *lflow_ref)
>      unlink_lflows_from_datapath(lflow_ref);
>  }
>
> +void
> +lflow_ref_clear_lflows_for_all_dps(struct lflow_ref *lflow_ref,
> +                                   size_t n_ls_datapaths,
> +                                   size_t n_lr_datapaths)
> +{
> +    unlink_lflows_from_all_datapaths(lflow_ref, n_ls_datapaths,
> +                                     n_lr_datapaths);
> +}
> +
>  void
>  lflow_ref_clear_and_sync_lflows(struct lflow_ref *lflow_ref,
>                      struct lflow_table *lflow_table,
> @@ -462,7 +475,9 @@ lflow_table_add_lflow(struct lflow_table *lflow_table,
>              /*  lflow_ref_node for this lflow doesn't exist yet.  Add
it. */
>              struct lflow_ref_node *ref_node = xzalloc(sizeof *ref_node);
>              ref_node->lflow = lflow;
> -            ref_node->dp_index = od->index;
> +            if (od) {
> +                ref_node->dp_index = od->index;
> +            }
>              ovs_list_insert(&lflow_ref->lflows_ref_list,
>                              &ref_node->ref_list_node);
>
> @@ -1047,6 +1062,30 @@ unlink_lflows_from_datapath(struct lflow_ref
*lflow_ref)
>      }
>  }
>
> +static void
> +unlink_lflows_from_all_datapaths(struct lflow_ref *lflow_ref,
> +                                 size_t n_ls_datapaths,
> +                                 size_t n_lr_datapaths)
> +{
> +    struct lflow_ref_node *ref_node;
> +    struct ovn_lflow *lflow;
> +    LIST_FOR_EACH (ref_node, ref_list_node, &lflow_ref->lflows_ref_list)
{
> +        size_t n_datapaths;
> +        size_t index;
> +
> +        lflow = ref_node->lflow;
> +        if (ovn_stage_to_datapath_type(lflow->stage) == DP_SWITCH) {
> +            n_datapaths = n_ls_datapaths;
> +        } else {
> +            n_datapaths = n_lr_datapaths;
> +        }
> +
> +        BITMAP_FOR_EACH_1 (index, n_datapaths, lflow->dpg_bitmap) {
> +            bitmap_set0(lflow->dpg_bitmap, index);
> +        }

It would be way more efficient to replace the loop by ULLONG_SET0.

Thanks,
Han

> +    }
> +}
> +
>  static void
>  lflow_ref_sync_lflows_to_sb__(struct lflow_ref  *lflow_ref,
>                          struct lflow_table *lflow_table,
> diff --git a/northd/lflow-mgr.h b/northd/lflow-mgr.h
> index 02b74aa131..c65cd70e71 100644
> --- a/northd/lflow-mgr.h
> +++ b/northd/lflow-mgr.h
> @@ -54,6 +54,9 @@ struct lflow_ref *lflow_ref_alloc(const char *res_name);
>  void lflow_ref_destroy(struct lflow_ref *);
>  void lflow_ref_reset(struct lflow_ref *lflow_ref);
>  void lflow_ref_clear_lflows(struct lflow_ref *);
> +void lflow_ref_clear_lflows_for_all_dps(struct lflow_ref *,
> +                                        size_t n_ls_datapaths,
> +                                        size_t n_lr_datapaths);
>  void lflow_ref_clear_and_sync_lflows(struct lflow_ref *,
>                                  struct lflow_table *lflow_table,
>                                  struct ovsdb_idl_txn *ovnsb_txn,
> diff --git a/northd/northd.c b/northd/northd.c
> index 104068e293..f56916b3da 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -3523,6 +3523,7 @@ ovn_lb_datapaths_create(const struct ovn_northd_lb
*lb, size_t n_ls_datapaths,
>      lb_dps->lb = lb;
>      lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths);
>      lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths);
> +    lb_dps->lflow_ref = lflow_ref_alloc(lb->nlb->name);
>
>      return lb_dps;
>  }
> @@ -3532,6 +3533,7 @@ ovn_lb_datapaths_destroy(struct ovn_lb_datapaths
*lb_dps)
>  {
>      bitmap_free(lb_dps->nb_lr_map);
>      bitmap_free(lb_dps->nb_ls_map);
> +    lflow_ref_destroy(lb_dps->lflow_ref);
>      free(lb_dps);
>  }
>
> @@ -16063,11 +16065,11 @@ build_lflows_thread(void *arg)
>                                                           lsi->lflows,
>                                                           &lsi->match,
>                                                           &lsi->actions,
> -                                                         NULL);
> +
lb_dps->lflow_ref);
>                      build_lrouter_defrag_flows_for_lb(lb_dps,
lsi->lflows,
>                                                        lsi->lr_datapaths,
>                                                        &lsi->match,
> -                                                      NULL);
> +                                                      lb_dps->lflow_ref);
>                      build_lrouter_flows_for_lb(lb_dps, lsi->lflows,
>                                                 lsi->meter_groups,
>                                                 lsi->lr_datapaths,
> @@ -16075,14 +16077,14 @@ build_lflows_thread(void *arg)
>                                                 lsi->features,
>                                                 lsi->svc_monitor_map,
>                                                 &lsi->match,
&lsi->actions,
> -                                               NULL);
> +                                               lb_dps->lflow_ref);
>                      build_lswitch_flows_for_lb(lb_dps, lsi->lflows,
>                                                 lsi->meter_groups,
>                                                 lsi->ls_datapaths,
>                                                 lsi->features,
>                                                 lsi->svc_monitor_map,
>                                                 &lsi->match,
&lsi->actions,
> -                                               NULL);
> +                                               lb_dps->lflow_ref);
>                  }
>              }
>              for (bnum = control->id;
> @@ -16298,20 +16300,20 @@ build_lswitch_and_lrouter_flows(const struct
ovn_datapaths *ls_datapaths,
>              build_lswitch_arp_nd_service_monitor(lb_dps->lb,
lsi.ls_ports,
>                                                   lsi.lflows,
&lsi.actions,
>                                                   &lsi.match,
> -                                                 NULL);
> +                                                 lb_dps->lflow_ref);
>              build_lrouter_defrag_flows_for_lb(lb_dps, lsi.lflows,
>                                                lsi.lr_datapaths,
&lsi.match,
> -                                              NULL);
> +                                              lb_dps->lflow_ref);
>              build_lrouter_flows_for_lb(lb_dps, lsi.lflows,
lsi.meter_groups,
>                                         lsi.lr_datapaths,
lsi.lr_sful_table,
>                                         lsi.features, lsi.svc_monitor_map,
>                                         &lsi.match, &lsi.actions,
> -                                       NULL);
> +                                       lb_dps->lflow_ref);
>              build_lswitch_flows_for_lb(lb_dps, lsi.lflows,
lsi.meter_groups,
>                                         lsi.ls_datapaths, lsi.features,
>                                         lsi.svc_monitor_map,
>                                         &lsi.match, &lsi.actions,
> -                                       NULL);
> +                                       lb_dps->lflow_ref);
>          }
>          stopwatch_stop(LFLOWS_LBS_STOPWATCH_NAME, time_msec());
>
> @@ -16483,11 +16485,17 @@ void build_lflows(struct ovsdb_idl_txn
*ovnsb_txn,
>  void
>  reset_lflow_refs_for_northd_resources(struct lflow_input *lflow_input)
>  {
> +    struct ovn_lb_datapaths *lb_dps;
>      struct ovn_port *op;
> +
>      HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) {
>          lflow_ref_reset(op->lflow_ref);
>          lflow_ref_reset(op->stateful_lflow_ref);
>      }
> +
> +    HMAP_FOR_EACH (lb_dps, hmap_node, lflow_input->lb_datapaths_map) {
> +        lflow_ref_reset(lb_dps->lflow_ref);
> +    }
>  }
>
>  bool
> @@ -16626,6 +16634,77 @@ lflow_handle_northd_port_changes(struct
ovsdb_idl_txn *ovnsb_txn,
>      return true;
>  }
>
> +bool
> +lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn,
> +                               struct tracked_lbs *trk_lbs,
> +                               struct lflow_input *lflow_input,
> +                               struct lflow_table *lflows)
> +{
> +    struct ovn_lb_datapaths *lb_dps;
> +    struct hmapx_node *hmapx_node;
> +    HMAPX_FOR_EACH (hmapx_node, &trk_lbs->deleted) {
> +        lb_dps = hmapx_node->data;
> +
> +        lflow_ref_clear_lflows_for_all_dps(lb_dps->lflow_ref,
> +
 ods_size(lflow_input->ls_datapaths),
> +
 ods_size(lflow_input->lr_datapaths));
> +        lflow_ref_sync_lflows_to_sb(lb_dps->lflow_ref, lflows, ovnsb_txn,
> +                             lflow_input->ls_datapaths,
> +                             lflow_input->lr_datapaths,
> +                             lflow_input->ovn_internal_version_changed,
> +                             lflow_input->sbrec_logical_flow_table,
> +                             lflow_input->sbrec_logical_dp_group_table);
> +    }
> +
> +    HMAPX_FOR_EACH (hmapx_node, &trk_lbs->crupdated) {
> +        lb_dps = hmapx_node->data;
> +
> +        /* unlink old lflows. */
> +        lflow_ref_clear_lflows_for_all_dps(lb_dps->lflow_ref,
> +
 ods_size(lflow_input->ls_datapaths),
> +
 ods_size(lflow_input->lr_datapaths));
> +
> +        /* Generate new lflows. */
> +        struct ds match = DS_EMPTY_INITIALIZER;
> +        struct ds actions = DS_EMPTY_INITIALIZER;
> +
> +        build_lswitch_arp_nd_service_monitor(lb_dps->lb,
lflow_input->ls_ports,
> +                                             lflows, &actions,
> +                                             &match, lb_dps->lflow_ref);
> +        build_lrouter_defrag_flows_for_lb(lb_dps, lflows,
> +                                          lflow_input->lr_datapaths,
&match,
> +                                          lb_dps->lflow_ref);
> +        build_lrouter_flows_for_lb(lb_dps, lflows,
> +                                   lflow_input->meter_groups,
> +                                   lflow_input->lr_datapaths,
> +                                   lflow_input->lr_sful_table,
> +                                   lflow_input->features,
> +                                   lflow_input->svc_monitor_map,
> +                                   &match, &actions,
> +                                   lb_dps->lflow_ref);
> +        build_lswitch_flows_for_lb(lb_dps, lflows,
> +                                   lflow_input->meter_groups,
> +                                   lflow_input->ls_datapaths,
> +                                   lflow_input->features,
> +                                   lflow_input->svc_monitor_map,
> +                                   &match, &actions,
> +                                   lb_dps->lflow_ref);
> +
> +        ds_destroy(&match);
> +        ds_destroy(&actions);
> +
> +        /* Sync the new flows to SB. */
> +        lflow_ref_sync_lflows_to_sb(lb_dps->lflow_ref, lflows, ovnsb_txn,
> +                             lflow_input->ls_datapaths,
> +                             lflow_input->lr_datapaths,
> +                             lflow_input->ovn_internal_version_changed,
> +                             lflow_input->sbrec_logical_flow_table,
> +                             lflow_input->sbrec_logical_dp_group_table);
> +    }
> +
> +    return true;
> +}
> +
>  static bool
>  mirror_needs_update(const struct nbrec_mirror *nb_mirror,
>                      const struct sbrec_mirror *sb_mirror)
> diff --git a/northd/northd.h b/northd/northd.h
> index e2df53e6f8..863bcce001 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -101,6 +101,30 @@ struct ovn_lb_datapaths {
>
>      size_t n_nb_lr;
>      unsigned long *nb_lr_map;
> +
> +    /* Reference of lflows generated for this load balancer.
> +     *
> +     * This data is initialized and destroyed by the en_northd node, but
> +     * populated and used only by the en_lflow node. Ideally this data
should
> +     * be maintained as part of en_lflow's data (struct lflow_data): a
hash
> +     * index from ovn_port key to lflows.  However, it would be less
efficient
> +     * and more complex:
> +     *
> +     * 1. It would require an extra search (using the index) to find the
> +     * lflows.
> +     *
> +     * 2. Building the index needs to be thread-safe, using either a
global
> +     * lock which is obviously less efficient, or hash-based lock array
which
> +     * is more complex.
> +     *
> +     * Maintaining the lflow_ref here is more straightforward. The
drawback is
> +     * that we need to keep in mind that this data belongs to en_lflow
node,
> +     * so never access it from any other nodes.
> +     *
> +     * 'lflow_ref' is used to reference logical flows generated for this
> +     *  load balancer.
> +     */
> +    struct lflow_ref *lflow_ref;
>  };
>
>  struct ovn_lb_group_datapaths {
> @@ -700,6 +724,10 @@ bool lflow_handle_northd_port_changes(struct
ovsdb_idl_txn *ovnsb_txn,
>                                        struct tracked_ovn_ports *,
>                                        struct lflow_input *,
>                                        struct lflow_table *lflows);
> +bool lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn,
> +                                    struct tracked_lbs *,
> +                                    struct lflow_input *,
> +                                    struct lflow_table *lflows);
>  bool northd_handle_sb_port_binding_changes(
>      const struct sbrec_port_binding_table *, struct hmap *ls_ports,
>      struct hmap *lr_ports);
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 03d62695db..50d88fecd5 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -10507,7 +10507,7 @@ check ovn-nbctl --wait=sb lb-add lb1 10.0.0.10:80
10.0.0.3:80
>
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute nocompute
>
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> @@ -10517,21 +10517,26 @@ check ovn-nbctl --wait=sb set load_balancer .
ip_port_mappings:10.0.0.3=sw0-p1:1
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
>  check_engine_stats lr_stateful norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb set load_balancer . options:foo=bar
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
>  check_engine_stats lr_stateful norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>
>  check ovn-nbctl --wait=sb -- lb-add lb2 20.0.0.10:80 20.0.0.20:80 --
lb-add lb3 30.0.0.10:80 30.0.0.20:80
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
>  check_engine_stats lr_stateful norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute nocompute
>
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> @@ -10541,7 +10546,7 @@ check ovn-nbctl --wait=sb -- lb-del lb2 -- lb-del
lb3
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
>  check_engine_stats lr_stateful norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute nocompute
>
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> @@ -10753,8 +10758,9 @@ check ovn-nbctl --wait=sb add load_balancer_group
. load_Balancer $lb1_uuid
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
>  check_engine_stats lr_stateful norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb clear load_balancer_group . load_Balancer
> @@ -10769,7 +10775,7 @@ check as northd ovn-appctl -t NORTHD_TYPE
inc-engine/clear-stats
>  check ovn-nbctl --wait=sb add load_balancer_group . load_Balancer
$lb1_uuid
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -10778,6 +10784,7 @@ check ovn-nbctl --wait=sb add logical_switch sw0
load_balancer_group $lbg1_uuid
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
>  check_engine_stats lr_stateful norecompute compute
> +check_engine_stats ls_stateful norecompute compute
>  check_engine_stats lflow recompute nocompute
>  check_engine_stats sync_to_sb_lb recompute compute
>
> @@ -10786,6 +10793,7 @@ check as northd ovn-appctl -t NORTHD_TYPE
inc-engine/clear-stats
>  check ovn-nbctl --wait=sb set load_balancer . options:bar=foo
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
> +check_engine_stats ls_stateful norecompute compute
>  check_engine_stats lflow recompute nocompute
>  check_engine_stats sync_to_sb_lb recompute compute
>
> @@ -10795,6 +10803,7 @@ check ovn-nbctl --wait=sb set load_balancer lb1
vips:'"10.0.0.10:80"'='"10.0.0.1
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
>  check_engine_stats lr_stateful norecompute compute
> +check_engine_stats ls_stateful norecompute compute
>  check_engine_stats lflow recompute nocompute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
> @@ -10892,6 +10901,7 @@ check_engine_stats northd recompute nocompute
>  check_engine_stats lr_stateful recompute nocompute
>  check_engine_stats lflow recompute nocompute
>  check_engine_stats sync_to_sb_lb recompute compute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  # Add back lb group to logical switch and then delete it.
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
> @@ -10901,6 +10911,7 @@ check_engine_stats northd norecompute compute
>  check_engine_stats lr_stateful norecompute compute
>  check_engine_stats lflow recompute nocompute
>  check_engine_stats sync_to_sb_lb recompute compute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb clear logical_switch sw0 load_balancer_group
-- \
> @@ -10934,14 +10945,17 @@ check_engine_stats northd norecompute compute
>  check_engine_stats lr_stateful norecompute compute
>  check_engine_stats lflow norecompute nocompute
>  check_engine_stats sync_to_sb_lb norecompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb set load_balancer_group .
load_balancer="$lb2_uuid,$lb3_uuid,$lb4_uuid"
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd norecompute compute
> +check_engine_stats ls_stateful norecompute compute
>  check_engine_stats lr_stateful norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute nocompute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
>  check ovn-nbctl --wait=sb set logical_switch sw0
load_balancer_group=$lbg1_uuid
> --
> 2.41.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
diff mbox series

Patch

diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 65d2f45ebc..13284b5556 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -125,11 +125,6 @@  lflow_northd_handler(struct engine_node *node,
         return false;
     }
 
-    /* Fall back to recompute if load balancers have changed. */
-    if (northd_has_lbs_in_tracked_data(&northd_data->trk_data)) {
-        return false;
-    }
-
     const struct engine_context *eng_ctx = engine_get_context();
     struct lflow_data *lflow_data = data;
 
@@ -142,6 +137,12 @@  lflow_northd_handler(struct engine_node *node,
         return false;
     }
 
+    if (!lflow_handle_northd_lb_changes(eng_ctx->ovnsb_idl_txn,
+                                &northd_data->trk_data.trk_lbs,
+                                &lflow_input, lflow_data->lflow_table)) {
+        return false;
+    }
+
     engine_set_node_state(node, EN_UPDATED);
     return true;
 }
diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
index 08962e9172..d779e7e087 100644
--- a/northd/lflow-mgr.c
+++ b/northd/lflow-mgr.c
@@ -105,6 +105,10 @@  static void ovn_dp_group_add_with_reference(struct ovn_lflow *,
                                             size_t bitmap_len);
 
 static void unlink_lflows_from_datapath(struct lflow_ref *);
+static void unlink_lflows_from_all_datapaths(struct lflow_ref *,
+                                             size_t n_ls_datapaths,
+                                             size_t n_lr_datapaths);
+
 static void lflow_ref_sync_lflows_to_sb__(struct lflow_ref  *,
                             struct lflow_table *,
                             struct ovsdb_idl_txn *ovnsb_txn,
@@ -394,6 +398,15 @@  lflow_ref_clear_lflows(struct lflow_ref *lflow_ref)
     unlink_lflows_from_datapath(lflow_ref);
 }
 
+void
+lflow_ref_clear_lflows_for_all_dps(struct lflow_ref *lflow_ref,
+                                   size_t n_ls_datapaths,
+                                   size_t n_lr_datapaths)
+{
+    unlink_lflows_from_all_datapaths(lflow_ref, n_ls_datapaths,
+                                     n_lr_datapaths);
+}
+
 void
 lflow_ref_clear_and_sync_lflows(struct lflow_ref *lflow_ref,
                     struct lflow_table *lflow_table,
@@ -462,7 +475,9 @@  lflow_table_add_lflow(struct lflow_table *lflow_table,
             /*  lflow_ref_node for this lflow doesn't exist yet.  Add it. */
             struct lflow_ref_node *ref_node = xzalloc(sizeof *ref_node);
             ref_node->lflow = lflow;
-            ref_node->dp_index = od->index;
+            if (od) {
+                ref_node->dp_index = od->index;
+            }
             ovs_list_insert(&lflow_ref->lflows_ref_list,
                             &ref_node->ref_list_node);
 
@@ -1047,6 +1062,30 @@  unlink_lflows_from_datapath(struct lflow_ref *lflow_ref)
     }
 }
 
+static void
+unlink_lflows_from_all_datapaths(struct lflow_ref *lflow_ref,
+                                 size_t n_ls_datapaths,
+                                 size_t n_lr_datapaths)
+{
+    struct lflow_ref_node *ref_node;
+    struct ovn_lflow *lflow;
+    LIST_FOR_EACH (ref_node, ref_list_node, &lflow_ref->lflows_ref_list) {
+        size_t n_datapaths;
+        size_t index;
+
+        lflow = ref_node->lflow;
+        if (ovn_stage_to_datapath_type(lflow->stage) == DP_SWITCH) {
+            n_datapaths = n_ls_datapaths;
+        } else {
+            n_datapaths = n_lr_datapaths;
+        }
+
+        BITMAP_FOR_EACH_1 (index, n_datapaths, lflow->dpg_bitmap) {
+            bitmap_set0(lflow->dpg_bitmap, index);
+        }
+    }
+}
+
 static void
 lflow_ref_sync_lflows_to_sb__(struct lflow_ref  *lflow_ref,
                         struct lflow_table *lflow_table,
diff --git a/northd/lflow-mgr.h b/northd/lflow-mgr.h
index 02b74aa131..c65cd70e71 100644
--- a/northd/lflow-mgr.h
+++ b/northd/lflow-mgr.h
@@ -54,6 +54,9 @@  struct lflow_ref *lflow_ref_alloc(const char *res_name);
 void lflow_ref_destroy(struct lflow_ref *);
 void lflow_ref_reset(struct lflow_ref *lflow_ref);
 void lflow_ref_clear_lflows(struct lflow_ref *);
+void lflow_ref_clear_lflows_for_all_dps(struct lflow_ref *,
+                                        size_t n_ls_datapaths,
+                                        size_t n_lr_datapaths);
 void lflow_ref_clear_and_sync_lflows(struct lflow_ref *,
                                 struct lflow_table *lflow_table,
                                 struct ovsdb_idl_txn *ovnsb_txn,
diff --git a/northd/northd.c b/northd/northd.c
index 104068e293..f56916b3da 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -3523,6 +3523,7 @@  ovn_lb_datapaths_create(const struct ovn_northd_lb *lb, size_t n_ls_datapaths,
     lb_dps->lb = lb;
     lb_dps->nb_ls_map = bitmap_allocate(n_ls_datapaths);
     lb_dps->nb_lr_map = bitmap_allocate(n_lr_datapaths);
+    lb_dps->lflow_ref = lflow_ref_alloc(lb->nlb->name);
 
     return lb_dps;
 }
@@ -3532,6 +3533,7 @@  ovn_lb_datapaths_destroy(struct ovn_lb_datapaths *lb_dps)
 {
     bitmap_free(lb_dps->nb_lr_map);
     bitmap_free(lb_dps->nb_ls_map);
+    lflow_ref_destroy(lb_dps->lflow_ref);
     free(lb_dps);
 }
 
@@ -16063,11 +16065,11 @@  build_lflows_thread(void *arg)
                                                          lsi->lflows,
                                                          &lsi->match,
                                                          &lsi->actions,
-                                                         NULL);
+                                                         lb_dps->lflow_ref);
                     build_lrouter_defrag_flows_for_lb(lb_dps, lsi->lflows,
                                                       lsi->lr_datapaths,
                                                       &lsi->match,
-                                                      NULL);
+                                                      lb_dps->lflow_ref);
                     build_lrouter_flows_for_lb(lb_dps, lsi->lflows,
                                                lsi->meter_groups,
                                                lsi->lr_datapaths,
@@ -16075,14 +16077,14 @@  build_lflows_thread(void *arg)
                                                lsi->features,
                                                lsi->svc_monitor_map,
                                                &lsi->match, &lsi->actions,
-                                               NULL);
+                                               lb_dps->lflow_ref);
                     build_lswitch_flows_for_lb(lb_dps, lsi->lflows,
                                                lsi->meter_groups,
                                                lsi->ls_datapaths,
                                                lsi->features,
                                                lsi->svc_monitor_map,
                                                &lsi->match, &lsi->actions,
-                                               NULL);
+                                               lb_dps->lflow_ref);
                 }
             }
             for (bnum = control->id;
@@ -16298,20 +16300,20 @@  build_lswitch_and_lrouter_flows(const struct ovn_datapaths *ls_datapaths,
             build_lswitch_arp_nd_service_monitor(lb_dps->lb, lsi.ls_ports,
                                                  lsi.lflows, &lsi.actions,
                                                  &lsi.match,
-                                                 NULL);
+                                                 lb_dps->lflow_ref);
             build_lrouter_defrag_flows_for_lb(lb_dps, lsi.lflows,
                                               lsi.lr_datapaths, &lsi.match,
-                                              NULL);
+                                              lb_dps->lflow_ref);
             build_lrouter_flows_for_lb(lb_dps, lsi.lflows, lsi.meter_groups,
                                        lsi.lr_datapaths, lsi.lr_sful_table,
                                        lsi.features, lsi.svc_monitor_map,
                                        &lsi.match, &lsi.actions,
-                                       NULL);
+                                       lb_dps->lflow_ref);
             build_lswitch_flows_for_lb(lb_dps, lsi.lflows, lsi.meter_groups,
                                        lsi.ls_datapaths, lsi.features,
                                        lsi.svc_monitor_map,
                                        &lsi.match, &lsi.actions,
-                                       NULL);
+                                       lb_dps->lflow_ref);
         }
         stopwatch_stop(LFLOWS_LBS_STOPWATCH_NAME, time_msec());
 
@@ -16483,11 +16485,17 @@  void build_lflows(struct ovsdb_idl_txn *ovnsb_txn,
 void
 reset_lflow_refs_for_northd_resources(struct lflow_input *lflow_input)
 {
+    struct ovn_lb_datapaths *lb_dps;
     struct ovn_port *op;
+
     HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) {
         lflow_ref_reset(op->lflow_ref);
         lflow_ref_reset(op->stateful_lflow_ref);
     }
+
+    HMAP_FOR_EACH (lb_dps, hmap_node, lflow_input->lb_datapaths_map) {
+        lflow_ref_reset(lb_dps->lflow_ref);
+    }
 }
 
 bool
@@ -16626,6 +16634,77 @@  lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn,
     return true;
 }
 
+bool
+lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn,
+                               struct tracked_lbs *trk_lbs,
+                               struct lflow_input *lflow_input,
+                               struct lflow_table *lflows)
+{
+    struct ovn_lb_datapaths *lb_dps;
+    struct hmapx_node *hmapx_node;
+    HMAPX_FOR_EACH (hmapx_node, &trk_lbs->deleted) {
+        lb_dps = hmapx_node->data;
+
+        lflow_ref_clear_lflows_for_all_dps(lb_dps->lflow_ref,
+                                        ods_size(lflow_input->ls_datapaths),
+                                        ods_size(lflow_input->lr_datapaths));
+        lflow_ref_sync_lflows_to_sb(lb_dps->lflow_ref, lflows, ovnsb_txn,
+                             lflow_input->ls_datapaths,
+                             lflow_input->lr_datapaths,
+                             lflow_input->ovn_internal_version_changed,
+                             lflow_input->sbrec_logical_flow_table,
+                             lflow_input->sbrec_logical_dp_group_table);
+    }
+
+    HMAPX_FOR_EACH (hmapx_node, &trk_lbs->crupdated) {
+        lb_dps = hmapx_node->data;
+
+        /* unlink old lflows. */
+        lflow_ref_clear_lflows_for_all_dps(lb_dps->lflow_ref,
+                                        ods_size(lflow_input->ls_datapaths),
+                                        ods_size(lflow_input->lr_datapaths));
+
+        /* Generate new lflows. */
+        struct ds match = DS_EMPTY_INITIALIZER;
+        struct ds actions = DS_EMPTY_INITIALIZER;
+
+        build_lswitch_arp_nd_service_monitor(lb_dps->lb, lflow_input->ls_ports,
+                                             lflows, &actions,
+                                             &match, lb_dps->lflow_ref);
+        build_lrouter_defrag_flows_for_lb(lb_dps, lflows,
+                                          lflow_input->lr_datapaths, &match,
+                                          lb_dps->lflow_ref);
+        build_lrouter_flows_for_lb(lb_dps, lflows,
+                                   lflow_input->meter_groups,
+                                   lflow_input->lr_datapaths,
+                                   lflow_input->lr_sful_table,
+                                   lflow_input->features,
+                                   lflow_input->svc_monitor_map,
+                                   &match, &actions,
+                                   lb_dps->lflow_ref);
+        build_lswitch_flows_for_lb(lb_dps, lflows,
+                                   lflow_input->meter_groups,
+                                   lflow_input->ls_datapaths,
+                                   lflow_input->features,
+                                   lflow_input->svc_monitor_map,
+                                   &match, &actions,
+                                   lb_dps->lflow_ref);
+
+        ds_destroy(&match);
+        ds_destroy(&actions);
+
+        /* Sync the new flows to SB. */
+        lflow_ref_sync_lflows_to_sb(lb_dps->lflow_ref, lflows, ovnsb_txn,
+                             lflow_input->ls_datapaths,
+                             lflow_input->lr_datapaths,
+                             lflow_input->ovn_internal_version_changed,
+                             lflow_input->sbrec_logical_flow_table,
+                             lflow_input->sbrec_logical_dp_group_table);
+    }
+
+    return true;
+}
+
 static bool
 mirror_needs_update(const struct nbrec_mirror *nb_mirror,
                     const struct sbrec_mirror *sb_mirror)
diff --git a/northd/northd.h b/northd/northd.h
index e2df53e6f8..863bcce001 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -101,6 +101,30 @@  struct ovn_lb_datapaths {
 
     size_t n_nb_lr;
     unsigned long *nb_lr_map;
+
+    /* Reference of lflows generated for this load balancer.
+     *
+     * This data is initialized and destroyed by the en_northd node, but
+     * populated and used only by the en_lflow node. Ideally this data should
+     * be maintained as part of en_lflow's data (struct lflow_data): a hash
+     * index from ovn_port key to lflows.  However, it would be less efficient
+     * and more complex:
+     *
+     * 1. It would require an extra search (using the index) to find the
+     * lflows.
+     *
+     * 2. Building the index needs to be thread-safe, using either a global
+     * lock which is obviously less efficient, or hash-based lock array which
+     * is more complex.
+     *
+     * Maintaining the lflow_ref here is more straightforward. The drawback is
+     * that we need to keep in mind that this data belongs to en_lflow node,
+     * so never access it from any other nodes.
+     *
+     * 'lflow_ref' is used to reference logical flows generated for this
+     *  load balancer.
+     */
+    struct lflow_ref *lflow_ref;
 };
 
 struct ovn_lb_group_datapaths {
@@ -700,6 +724,10 @@  bool lflow_handle_northd_port_changes(struct ovsdb_idl_txn *ovnsb_txn,
                                       struct tracked_ovn_ports *,
                                       struct lflow_input *,
                                       struct lflow_table *lflows);
+bool lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn,
+                                    struct tracked_lbs *,
+                                    struct lflow_input *,
+                                    struct lflow_table *lflows);
 bool northd_handle_sb_port_binding_changes(
     const struct sbrec_port_binding_table *, struct hmap *ls_ports,
     struct hmap *lr_ports);
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 03d62695db..50d88fecd5 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -10507,7 +10507,7 @@  check ovn-nbctl --wait=sb lb-add lb1 10.0.0.10:80 10.0.0.3:80
 
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute nocompute
 
 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
@@ -10517,21 +10517,26 @@  check ovn-nbctl --wait=sb set load_balancer . ip_port_mappings:10.0.0.3=sw0-p1:1
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb set load_balancer . options:foo=bar
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 
 check ovn-nbctl --wait=sb -- lb-add lb2 20.0.0.10:80 20.0.0.20:80 -- lb-add lb3 30.0.0.10:80 30.0.0.20:80
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute nocompute
 
 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
@@ -10541,7 +10546,7 @@  check ovn-nbctl --wait=sb -- lb-del lb2 -- lb-del lb3
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute nocompute
 
 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
@@ -10753,8 +10758,9 @@  check ovn-nbctl --wait=sb add load_balancer_group . load_Balancer $lb1_uuid
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb clear load_balancer_group . load_Balancer
@@ -10769,7 +10775,7 @@  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb add load_balancer_group . load_Balancer $lb1_uuid
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10778,6 +10784,7 @@  check ovn-nbctl --wait=sb add logical_switch sw0 load_balancer_group $lbg1_uuid
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
+check_engine_stats ls_stateful norecompute compute
 check_engine_stats lflow recompute nocompute
 check_engine_stats sync_to_sb_lb recompute compute
 
@@ -10786,6 +10793,7 @@  check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb set load_balancer . options:bar=foo
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
+check_engine_stats ls_stateful norecompute compute
 check_engine_stats lflow recompute nocompute
 check_engine_stats sync_to_sb_lb recompute compute
 
@@ -10795,6 +10803,7 @@  check ovn-nbctl --wait=sb set load_balancer lb1 vips:'"10.0.0.10:80"'='"10.0.0.1
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
+check_engine_stats ls_stateful norecompute compute
 check_engine_stats lflow recompute nocompute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
@@ -10892,6 +10901,7 @@  check_engine_stats northd recompute nocompute
 check_engine_stats lr_stateful recompute nocompute
 check_engine_stats lflow recompute nocompute
 check_engine_stats sync_to_sb_lb recompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 # Add back lb group to logical switch and then delete it.
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
@@ -10901,6 +10911,7 @@  check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
 check_engine_stats lflow recompute nocompute
 check_engine_stats sync_to_sb_lb recompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb clear logical_switch sw0 load_balancer_group -- \
@@ -10934,14 +10945,17 @@  check_engine_stats northd norecompute compute
 check_engine_stats lr_stateful norecompute compute
 check_engine_stats lflow norecompute nocompute
 check_engine_stats sync_to_sb_lb norecompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb set load_balancer_group . load_balancer="$lb2_uuid,$lb3_uuid,$lb4_uuid"
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd norecompute compute
+check_engine_stats ls_stateful norecompute compute
 check_engine_stats lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute nocompute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
 check ovn-nbctl --wait=sb set logical_switch sw0 load_balancer_group=$lbg1_uuid