diff mbox series

[ovs-dev,v6,10/13] northd: Add ls_stateful handler for lflow engine node.

Message ID 20240130212317.1483388-1-numans@ovn.org
State Accepted
Headers show
Series northd lflow incremental processing | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_ovn-kubernetes success github build: passed
ovsrobot/github-robot-_Build_and_Test fail github build: failed
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 Jan. 30, 2024, 9:23 p.m. UTC
From: Numan Siddique <numans@ovn.org>

Acked-by: Han Zhou <hzhou@ovn.org>
Signed-off-by: Numan Siddique <numans@ovn.org>
---
 northd/en-lflow.c        | 26 +++++++++++++
 northd/en-lflow.h        |  1 +
 northd/en-ls-stateful.c  |  9 +++--
 northd/en-ls-stateful.h  | 26 +++++++++++++
 northd/inc-proc-northd.c |  2 +-
 northd/northd.c          | 61 +++++++++++++++++++++++++++---
 northd/northd.h          |  5 +++
 tests/ovn-northd.at      | 81 ++++++++++++++++++++++++++--------------
 8 files changed, 173 insertions(+), 38 deletions(-)

Comments

Numan Siddique Feb. 1, 2024, 3:13 p.m. UTC | #1
On Tue, Jan 30, 2024 at 4:24 PM <numans@ovn.org> wrote:
>
> From: Numan Siddique <numans@ovn.org>
>
> Acked-by: Han Zhou <hzhou@ovn.org>
> Signed-off-by: Numan Siddique <numans@ovn.org>

Recheck-request: github-robot-_Build_and_Test

> ---
>  northd/en-lflow.c        | 26 +++++++++++++
>  northd/en-lflow.h        |  1 +
>  northd/en-ls-stateful.c  |  9 +++--
>  northd/en-ls-stateful.h  | 26 +++++++++++++
>  northd/inc-proc-northd.c |  2 +-
>  northd/northd.c          | 61 +++++++++++++++++++++++++++---
>  northd/northd.h          |  5 +++
>  tests/ovn-northd.at      | 81 ++++++++++++++++++++++++++--------------
>  8 files changed, 173 insertions(+), 38 deletions(-)
>
> diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> index 41b1779539..525453054b 100644
> --- a/northd/en-lflow.c
> +++ b/northd/en-lflow.c
> @@ -190,6 +190,32 @@ lflow_lr_stateful_handler(struct engine_node *node, void *data)
>      return true;
>  }
>
> +bool
> +lflow_ls_stateful_handler(struct engine_node *node, void *data)
> +{
> +    struct ed_type_ls_stateful *ls_sful_data =
> +        engine_get_input_data("ls_stateful", node);
> +
> +    if (!ls_stateful_has_tracked_data(&ls_sful_data->trk_data)) {
> +        return false;
> +    }
> +
> +    const struct engine_context *eng_ctx = engine_get_context();
> +    struct lflow_data *lflow_data = data;
> +    struct lflow_input lflow_input;
> +
> +    lflow_get_input_data(node, &lflow_input);
> +    if (!lflow_handle_ls_stateful_changes(eng_ctx->ovnsb_idl_txn,
> +                                          &ls_sful_data->trk_data,
> +                                          &lflow_input,
> +                                          lflow_data->lflow_table)) {
> +        return false;
> +    }
> +
> +    engine_set_node_state(node, EN_UPDATED);
> +    return true;
> +}
> +
>  void *en_lflow_init(struct engine_node *node OVS_UNUSED,
>                       struct engine_arg *arg OVS_UNUSED)
>  {
> diff --git a/northd/en-lflow.h b/northd/en-lflow.h
> index 1d813a2a29..32cae61763 100644
> --- a/northd/en-lflow.h
> +++ b/northd/en-lflow.h
> @@ -21,5 +21,6 @@ void en_lflow_cleanup(void *data);
>  bool lflow_northd_handler(struct engine_node *, void *data);
>  bool lflow_port_group_handler(struct engine_node *, void *data);
>  bool lflow_lr_stateful_handler(struct engine_node *, void *data);
> +bool lflow_ls_stateful_handler(struct engine_node *node, void *data);
>
>  #endif /* EN_LFLOW_H */
> diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c
> index fae71c4c8f..44f74ea08e 100644
> --- a/northd/en-ls-stateful.c
> +++ b/northd/en-ls-stateful.c
> @@ -39,6 +39,7 @@
>  #include "lib/ovn-sb-idl.h"
>  #include "lib/ovn-util.h"
>  #include "lib/stopwatch-names.h"
> +#include "lflow-mgr.h"
>  #include "northd.h"
>
>  VLOG_DEFINE_THIS_MODULE(en_ls_stateful);
> @@ -289,6 +290,7 @@ ls_stateful_record_create(struct ls_stateful_table *table,
>      ls_stateful_rec->ls_index = od->index;
>      ls_stateful_rec->nbs_uuid = od->nbs->header_.uuid;
>      ls_stateful_record_init(ls_stateful_rec, od, NULL, ls_pgs);
> +    ls_stateful_rec->lflow_ref = lflow_ref_create();
>
>      hmap_insert(&table->entries, &ls_stateful_rec->key_node,
>                  uuid_hash(&od->nbs->header_.uuid));
> @@ -299,14 +301,15 @@ ls_stateful_record_create(struct ls_stateful_table *table,
>  static void
>  ls_stateful_record_destroy(struct ls_stateful_record *ls_stateful_rec)
>  {
> +    lflow_ref_destroy(ls_stateful_rec->lflow_ref);
>      free(ls_stateful_rec);
>  }
>
>  static void
>  ls_stateful_record_init(struct ls_stateful_record *ls_stateful_rec,
> -                        const struct ovn_datapath *od,
> -                        const struct ls_port_group *ls_pg,
> -                        const struct ls_port_group_table *ls_pgs)
> +                      const struct ovn_datapath *od,
> +                      const struct ls_port_group *ls_pg,
> +                      const struct ls_port_group_table *ls_pgs)
>  {
>      ls_stateful_rec->has_lb_vip = ls_has_lb_vip(od);
>      ls_stateful_record_set_acl_flags(ls_stateful_rec, od, ls_pg, ls_pgs);
> diff --git a/northd/en-ls-stateful.h b/northd/en-ls-stateful.h
> index a31b1d154d..eae4b08e22 100644
> --- a/northd/en-ls-stateful.h
> +++ b/northd/en-ls-stateful.h
> @@ -31,6 +31,8 @@
>  #include "lib/ovn-util.h"
>  #include "lib/stopwatch-names.h"
>
> +struct lflow_ref;
> +
>  struct ls_stateful_record {
>      struct hmap_node key_node;
>
> @@ -45,6 +47,30 @@ struct ls_stateful_record {
>      bool has_lb_vip;
>      bool has_acls;
>      uint64_t max_acl_tier;
> +
> +    /* 'lflow_ref' is used to reference logical flows generated for
> +     * this ls_stateful record.
> +     *
> +     * This data is initialized and destroyed by the en_ls_stateful node,
> +     * but populated and used only by the en_lflow node. Ideally this data
> +     * should be maintained as part of en_lflow's data.  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.
> +     *
> +     * Adding 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.
> +     *
> +     * Note: lflow_ref is not thread safe.  Only one thread should
> +     * access ls_stateful_record->lflow_ref at any given time.
> +     */
> +    struct lflow_ref *lflow_ref;
>  };
>
>  struct ls_stateful_table {
> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> index dcce79510a..f7c3d2bcf5 100644
> --- a/northd/inc-proc-northd.c
> +++ b/northd/inc-proc-northd.c
> @@ -228,11 +228,11 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
>      engine_add_input(&en_lflow, &en_sb_logical_flow, NULL);
>      engine_add_input(&en_lflow, &en_sb_multicast_group, NULL);
>      engine_add_input(&en_lflow, &en_sb_igmp_group, NULL);
> -    engine_add_input(&en_lflow, &en_ls_stateful, NULL);
>      engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL);
>      engine_add_input(&en_lflow, &en_northd, lflow_northd_handler);
>      engine_add_input(&en_lflow, &en_port_group, lflow_port_group_handler);
>      engine_add_input(&en_lflow, &en_lr_stateful, lflow_lr_stateful_handler);
> +    engine_add_input(&en_lflow, &en_ls_stateful, lflow_ls_stateful_handler);
>
>      engine_add_input(&en_sync_to_sb_addr_set, &en_nb_address_set,
>                       sync_to_sb_addr_set_nb_address_set_handler);
> diff --git a/northd/northd.c b/northd/northd.c
> index e3448be29f..2864f78b0b 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -15740,12 +15740,15 @@ build_ls_stateful_flows(const struct ls_stateful_record *ls_stateful_rec,
>                          const struct shash *meter_groups,
>                          struct lflow_table *lflows)
>  {
> -    build_ls_stateful_rec_pre_acls(ls_stateful_rec, od, ls_pgs, lflows, NULL);
> -    build_ls_stateful_rec_pre_lb(ls_stateful_rec, od, lflows, NULL);
> -    build_acl_hints(ls_stateful_rec, od, features, lflows, NULL);
> +    build_ls_stateful_rec_pre_acls(ls_stateful_rec, od, ls_pgs, lflows,
> +                                   ls_stateful_rec->lflow_ref);
> +    build_ls_stateful_rec_pre_lb(ls_stateful_rec, od, lflows,
> +                                 ls_stateful_rec->lflow_ref);
> +    build_acl_hints(ls_stateful_rec, od, features, lflows,
> +                    ls_stateful_rec->lflow_ref);
>      build_acls(ls_stateful_rec, od, features, lflows, ls_pgs,
> -               meter_groups, NULL);
> -    build_lb_hairpin(ls_stateful_rec, od, lflows, NULL);
> +               meter_groups, ls_stateful_rec->lflow_ref);
> +    build_lb_hairpin(ls_stateful_rec, od, lflows, ls_stateful_rec->lflow_ref);
>  }
>
>  struct lswitch_flow_build_info {
> @@ -15916,6 +15919,7 @@ build_lflows_thread(void *arg)
>       *    - op->lflow_ref
>       *    - lb_dps->lflow_ref
>       *    - lr_stateful_rec->lflow_ref
> +     *    - ls_stateful_rec->lflow_ref
>       * are not accessed by multiple threads at the same time. */
>      while (!stop_parallel_processing()) {
>          wait_for_work(control);
> @@ -16423,6 +16427,7 @@ void
>  lflow_reset_northd_refs(struct lflow_input *lflow_input)
>  {
>      const struct lr_stateful_record *lr_stateful_rec;
> +    struct ls_stateful_record *ls_stateful_rec;
>      struct ovn_lb_datapaths *lb_dps;
>      struct ovn_port *op;
>
> @@ -16431,6 +16436,11 @@ lflow_reset_northd_refs(struct lflow_input *lflow_input)
>          lflow_ref_clear(lr_stateful_rec->lflow_ref);
>      }
>
> +    LS_STATEFUL_TABLE_FOR_EACH (ls_stateful_rec,
> +                                lflow_input->ls_stateful_table) {
> +        lflow_ref_clear(ls_stateful_rec->lflow_ref);
> +    }
> +
>      HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) {
>          lflow_ref_clear(op->lflow_ref);
>          lflow_ref_clear(op->stateful_lflow_ref);
> @@ -16750,6 +16760,47 @@ exit:
>      return handled;
>  }
>
> +bool
> +lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *ovnsb_txn,
> +                                struct ls_stateful_tracked_data *trk_data,
> +                                struct lflow_input *lflow_input,
> +                                struct lflow_table *lflows)
> +{
> +    struct hmapx_node *hmapx_node;
> +
> +    HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) {
> +        struct ls_stateful_record *ls_stateful_rec = hmapx_node->data;
> +        const struct ovn_datapath *od =
> +            ovn_datapaths_find_by_index(lflow_input->ls_datapaths,
> +                                        ls_stateful_rec->ls_index);
> +        ovs_assert(od->nbs && uuid_equals(&od->nbs->header_.uuid,
> +                                          &ls_stateful_rec->nbs_uuid));
> +
> +        lflow_ref_unlink_lflows(ls_stateful_rec->lflow_ref);
> +
> +        /* Generate new lflows. */
> +        build_ls_stateful_flows(ls_stateful_rec, od,
> +                                lflow_input->ls_port_groups,
> +                                lflow_input->features,
> +                                lflow_input->meter_groups,
> +                                lflows);
> +
> +        /* Sync the new flows to SB. */
> +        bool handled = lflow_ref_sync_lflows(
> +            ls_stateful_rec->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);
> +        if (!handled) {
> +            return false;
> +        }
> +    }
> +
> +    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 224b5d72ba..970b25f210 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -678,6 +678,7 @@ void northd_indices_create(struct northd_data *data,
>
>  struct lflow_table;
>  struct lr_stateful_tracked_data;
> +struct ls_stateful_tracked_data;
>
>  void build_lflows(struct ovsdb_idl_txn *ovnsb_txn,
>                    struct lflow_input *input_data,
> @@ -696,6 +697,10 @@ bool lflow_handle_lr_stateful_changes(struct ovsdb_idl_txn *,
>                                        struct lr_stateful_tracked_data *,
>                                        struct lflow_input *,
>                                        struct lflow_table *lflows);
> +bool lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *,
> +                                      struct ls_stateful_tracked_data *,
> +                                      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 8a20643989..29eb0953ee 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -10700,12 +10700,13 @@ check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
>  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
>  # A LB applied to a switch/router triggers:
>  # - a recompute in the first iteration (handling northd change)
>  # - a compute in the second iteration (handling SB update)
>  check_engine_stats sync_to_sb_lb recompute compute
> -CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> +check_engine_stats ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE((1))
>
>  # Modify the backend of the lb1 vip
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> @@ -10713,7 +10714,8 @@ 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 lflow recompute nocompute
> +check_engine_stats ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
>
> @@ -10723,7 +10725,8 @@ check ovn-nbctl --wait=sb clear load_Balancer lb1 vips
>  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 ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
>
> @@ -10733,7 +10736,8 @@ 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 lr_stateful norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
>
> @@ -10743,7 +10747,8 @@ check ovn-nbctl --wait=sb lb-add lb1 10.0.0.20:80 10.0.0.30:8080
>  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 ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
>
> @@ -10753,6 +10758,7 @@ check ovn-nbctl --wait=sb ls-lb-del sw0 lb1
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd recompute nocompute
>  check_engine_stats lr_stateful recompute nocompute
> +check_engine_stats ls_stateful recompute nocompute
>  check_engine_stats lflow recompute nocompute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
> @@ -10764,7 +10770,8 @@ check ovn-nbctl --wait=sb ls-lb-add sw0 lb1 -- lsp-add sw0 sw0p1
>  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 ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
> @@ -10883,7 +10890,7 @@ 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 lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>
>  # Update lb and this should not result in northd recompute
> @@ -10891,8 +10898,9 @@ check as northd ovn-appctl -t ovn-northd 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 lr_stateful norecompute compute
>  check_engine_stats ls_stateful norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>
>  # Modify the backend of the lb1 vip
> @@ -10902,7 +10910,7 @@ 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 lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -10912,7 +10920,8 @@ check ovn-nbctl --wait=sb clear load_Balancer lb1 vips
>  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 ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -10922,7 +10931,8 @@ 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 lr_stateful norecompute compute
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -10932,7 +10942,8 @@ check ovn-nbctl --wait=sb lb-add lb1 10.0.0.20:80 10.0.0.30:8080
>  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 ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -11007,7 +11018,8 @@ 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 lflow recompute nocompute
> +check_engine_stats ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -11060,7 +11072,8 @@ check ovn-nbctl --wait=sb set 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 lflow recompute nocompute
> +check_engine_stats ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -11078,7 +11091,8 @@ check ovn-nbctl --wait=sb ls-lb-add sw0 lb2
>  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 ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -11087,7 +11101,8 @@ check ovn-nbctl --wait=sb ls-lb-add sw0 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 ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -11115,6 +11130,7 @@ check ovn-nbctl --wait=sb lr-lb-del lr1 lb2
>  check_engine_stats lb_data norecompute compute
>  check_engine_stats northd recompute nocompute
>  check_engine_stats lr_stateful recompute nocompute
> +check_engine_stats ls_stateful recompute nocompute
>  check_engine_stats lflow recompute nocompute
>  check_engine_stats sync_to_sb_lb recompute nocompute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
> @@ -11126,7 +11142,8 @@ check ovn-nbctl --wait=sb lb-del lb4
>  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 ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -11137,7 +11154,8 @@ check ovn-nbctl --wait=sb lb-del lb2
>  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 ls_stateful norecompute compute
> +check_engine_stats lflow norecompute compute
>  check_engine_stats sync_to_sb_lb recompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
> @@ -11347,7 +11365,7 @@ check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>  check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
>  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_NO_CHANGE_AFTER_RECOMPUTE
>
>  # Clear the VIPs of lb1
> @@ -11355,7 +11373,7 @@ check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>  check ovn-nbctl --wait=sb clear load_balancer . vips
>  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_NO_CHANGE_AFTER_RECOMPUTE
>
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> @@ -11379,7 +11397,7 @@ check ovn-nbctl --wait=sb lb-add lb2 10.0.0.10:80 10.0.0.3:80
>
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>  check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  lb_lflow_uuid=$(fetch_column Logical_flow _uuid match='"ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80"')
> @@ -11393,7 +11411,7 @@ AT_CHECK([test "$lb_lflow_dpgrp" = ""])
>
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>  check ovn-nbctl --wait=sb ls-lb-add sw1 lb2
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  lb_lflow_dp=$(ovn-sbctl --bare --columns logical_datapath list logical_flow $lb_lflow_uuid)
> @@ -11402,6 +11420,11 @@ AT_CHECK([test "$lb_lflow_dp" = ""])
>  lb_lflow_dpgrp=$(ovn-sbctl --bare --columns logical_dp_group list logical_flow $lb_lflow_uuid)
>  AT_CHECK([test "$lb_lflow_dpgrp" != ""])
>
> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> +check ovn-nbctl --wait=sb set load_balancer lb1 options:bar=foo
> +check_engine_stats lflow norecompute compute
> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> +
>  # Clear the SB:Logical_Flow.logical_dp_groups column of all the
>  # logical flows and then modify the NB:Load_balancer.  ovn-northd
>  # should resync the logical flows.
> @@ -11419,7 +11442,7 @@ lb_lflow_uuid=$(fetch_column Logical_flow _uuid match='"ct.new && ip4.dst == 10.
>
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>  check ovn-nbctl --wait=sb clear load_balancer lb2 vips
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  lb_lflow_dp=$(ovn-sbctl --bare --columns logical_datapath list logical_flow $lb_lflow_uuid)
> @@ -11445,7 +11468,7 @@ check ovn-nbctl ls-lb-add sw4 lb2
>
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>  check ovn-nbctl --wait=sb ls-lb-add sw5 lb2
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  lb_lflow_dp=$(ovn-sbctl --bare --columns logical_datapath list logical_flow $lb_lflow_uuid)
> @@ -11476,7 +11499,7 @@ echo "dpgrp_dps - $dpgrp_dps"
>
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>  check ovn-nbctl --wait=sb clear load_balancer lb2 vips
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  lb_lflow_dpgrp=$(ovn-sbctl --bare --columns logical_dp_group list logical_flow $lb_lflow_uuid)
> @@ -11510,7 +11533,7 @@ check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>
>  check ovn-nbctl ls-lb-add sw0 lb3
>  check ovn-nbctl --wait=sb ls-lb-add sw1 lb3
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  lb_lflow_uuid=$(fetch_column Logical_flow _uuid match='"ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80"')
> @@ -11535,7 +11558,7 @@ AT_CHECK([echo $dpgrp_dps | grep $sw5_uuid], [0], [ignore])
>  # should have reference to sw0 and sw1, but not to sw2.
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>  check ovn-nbctl --wait=sb clear load_balancer lb1 vips
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  lb_lflow_dp=$(ovn-sbctl --bare --columns logical_datapath list logical_flow $lb_lflow_uuid)
> @@ -11561,7 +11584,7 @@ AT_CHECK([echo $dpgrp_dps | grep $sw5_uuid], [0], [ignore])
>
>  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>  check ovn-nbctl --wait=sb clear load_balancer lb3 vips
> -check_engine_stats lflow recompute nocompute
> +check_engine_stats lflow norecompute compute
>  CHECK_NO_CHANGE_AFTER_RECOMPUTE
>
>  lb_lflow_dp=$(ovn-sbctl --bare --columns logical_datapath list logical_flow $lb_lflow_uuid)
> --
> 2.43.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Dumitru Ceara Feb. 2, 2024, 12:14 p.m. UTC | #2
On 1/30/24 22:23, numans@ovn.org wrote:
> From: Numan Siddique <numans@ovn.org>
> 
> Acked-by: Han Zhou <hzhou@ovn.org>
> Signed-off-by: Numan Siddique <numans@ovn.org>
> ---

Acked-by: Dumitru Ceara <dceara@redhat.com>
diff mbox series

Patch

diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 41b1779539..525453054b 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -190,6 +190,32 @@  lflow_lr_stateful_handler(struct engine_node *node, void *data)
     return true;
 }
 
+bool
+lflow_ls_stateful_handler(struct engine_node *node, void *data)
+{
+    struct ed_type_ls_stateful *ls_sful_data =
+        engine_get_input_data("ls_stateful", node);
+
+    if (!ls_stateful_has_tracked_data(&ls_sful_data->trk_data)) {
+        return false;
+    }
+
+    const struct engine_context *eng_ctx = engine_get_context();
+    struct lflow_data *lflow_data = data;
+    struct lflow_input lflow_input;
+
+    lflow_get_input_data(node, &lflow_input);
+    if (!lflow_handle_ls_stateful_changes(eng_ctx->ovnsb_idl_txn,
+                                          &ls_sful_data->trk_data,
+                                          &lflow_input,
+                                          lflow_data->lflow_table)) {
+        return false;
+    }
+
+    engine_set_node_state(node, EN_UPDATED);
+    return true;
+}
+
 void *en_lflow_init(struct engine_node *node OVS_UNUSED,
                      struct engine_arg *arg OVS_UNUSED)
 {
diff --git a/northd/en-lflow.h b/northd/en-lflow.h
index 1d813a2a29..32cae61763 100644
--- a/northd/en-lflow.h
+++ b/northd/en-lflow.h
@@ -21,5 +21,6 @@  void en_lflow_cleanup(void *data);
 bool lflow_northd_handler(struct engine_node *, void *data);
 bool lflow_port_group_handler(struct engine_node *, void *data);
 bool lflow_lr_stateful_handler(struct engine_node *, void *data);
+bool lflow_ls_stateful_handler(struct engine_node *node, void *data);
 
 #endif /* EN_LFLOW_H */
diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c
index fae71c4c8f..44f74ea08e 100644
--- a/northd/en-ls-stateful.c
+++ b/northd/en-ls-stateful.c
@@ -39,6 +39,7 @@ 
 #include "lib/ovn-sb-idl.h"
 #include "lib/ovn-util.h"
 #include "lib/stopwatch-names.h"
+#include "lflow-mgr.h"
 #include "northd.h"
 
 VLOG_DEFINE_THIS_MODULE(en_ls_stateful);
@@ -289,6 +290,7 @@  ls_stateful_record_create(struct ls_stateful_table *table,
     ls_stateful_rec->ls_index = od->index;
     ls_stateful_rec->nbs_uuid = od->nbs->header_.uuid;
     ls_stateful_record_init(ls_stateful_rec, od, NULL, ls_pgs);
+    ls_stateful_rec->lflow_ref = lflow_ref_create();
 
     hmap_insert(&table->entries, &ls_stateful_rec->key_node,
                 uuid_hash(&od->nbs->header_.uuid));
@@ -299,14 +301,15 @@  ls_stateful_record_create(struct ls_stateful_table *table,
 static void
 ls_stateful_record_destroy(struct ls_stateful_record *ls_stateful_rec)
 {
+    lflow_ref_destroy(ls_stateful_rec->lflow_ref);
     free(ls_stateful_rec);
 }
 
 static void
 ls_stateful_record_init(struct ls_stateful_record *ls_stateful_rec,
-                        const struct ovn_datapath *od,
-                        const struct ls_port_group *ls_pg,
-                        const struct ls_port_group_table *ls_pgs)
+                      const struct ovn_datapath *od,
+                      const struct ls_port_group *ls_pg,
+                      const struct ls_port_group_table *ls_pgs)
 {
     ls_stateful_rec->has_lb_vip = ls_has_lb_vip(od);
     ls_stateful_record_set_acl_flags(ls_stateful_rec, od, ls_pg, ls_pgs);
diff --git a/northd/en-ls-stateful.h b/northd/en-ls-stateful.h
index a31b1d154d..eae4b08e22 100644
--- a/northd/en-ls-stateful.h
+++ b/northd/en-ls-stateful.h
@@ -31,6 +31,8 @@ 
 #include "lib/ovn-util.h"
 #include "lib/stopwatch-names.h"
 
+struct lflow_ref;
+
 struct ls_stateful_record {
     struct hmap_node key_node;
 
@@ -45,6 +47,30 @@  struct ls_stateful_record {
     bool has_lb_vip;
     bool has_acls;
     uint64_t max_acl_tier;
+
+    /* 'lflow_ref' is used to reference logical flows generated for
+     * this ls_stateful record.
+     *
+     * This data is initialized and destroyed by the en_ls_stateful node,
+     * but populated and used only by the en_lflow node. Ideally this data
+     * should be maintained as part of en_lflow's data.  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.
+     *
+     * Adding 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.
+     *
+     * Note: lflow_ref is not thread safe.  Only one thread should
+     * access ls_stateful_record->lflow_ref at any given time.
+     */
+    struct lflow_ref *lflow_ref;
 };
 
 struct ls_stateful_table {
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index dcce79510a..f7c3d2bcf5 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -228,11 +228,11 @@  void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
     engine_add_input(&en_lflow, &en_sb_logical_flow, NULL);
     engine_add_input(&en_lflow, &en_sb_multicast_group, NULL);
     engine_add_input(&en_lflow, &en_sb_igmp_group, NULL);
-    engine_add_input(&en_lflow, &en_ls_stateful, NULL);
     engine_add_input(&en_lflow, &en_sb_logical_dp_group, NULL);
     engine_add_input(&en_lflow, &en_northd, lflow_northd_handler);
     engine_add_input(&en_lflow, &en_port_group, lflow_port_group_handler);
     engine_add_input(&en_lflow, &en_lr_stateful, lflow_lr_stateful_handler);
+    engine_add_input(&en_lflow, &en_ls_stateful, lflow_ls_stateful_handler);
 
     engine_add_input(&en_sync_to_sb_addr_set, &en_nb_address_set,
                      sync_to_sb_addr_set_nb_address_set_handler);
diff --git a/northd/northd.c b/northd/northd.c
index e3448be29f..2864f78b0b 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -15740,12 +15740,15 @@  build_ls_stateful_flows(const struct ls_stateful_record *ls_stateful_rec,
                         const struct shash *meter_groups,
                         struct lflow_table *lflows)
 {
-    build_ls_stateful_rec_pre_acls(ls_stateful_rec, od, ls_pgs, lflows, NULL);
-    build_ls_stateful_rec_pre_lb(ls_stateful_rec, od, lflows, NULL);
-    build_acl_hints(ls_stateful_rec, od, features, lflows, NULL);
+    build_ls_stateful_rec_pre_acls(ls_stateful_rec, od, ls_pgs, lflows,
+                                   ls_stateful_rec->lflow_ref);
+    build_ls_stateful_rec_pre_lb(ls_stateful_rec, od, lflows,
+                                 ls_stateful_rec->lflow_ref);
+    build_acl_hints(ls_stateful_rec, od, features, lflows,
+                    ls_stateful_rec->lflow_ref);
     build_acls(ls_stateful_rec, od, features, lflows, ls_pgs,
-               meter_groups, NULL);
-    build_lb_hairpin(ls_stateful_rec, od, lflows, NULL);
+               meter_groups, ls_stateful_rec->lflow_ref);
+    build_lb_hairpin(ls_stateful_rec, od, lflows, ls_stateful_rec->lflow_ref);
 }
 
 struct lswitch_flow_build_info {
@@ -15916,6 +15919,7 @@  build_lflows_thread(void *arg)
      *    - op->lflow_ref
      *    - lb_dps->lflow_ref
      *    - lr_stateful_rec->lflow_ref
+     *    - ls_stateful_rec->lflow_ref
      * are not accessed by multiple threads at the same time. */
     while (!stop_parallel_processing()) {
         wait_for_work(control);
@@ -16423,6 +16427,7 @@  void
 lflow_reset_northd_refs(struct lflow_input *lflow_input)
 {
     const struct lr_stateful_record *lr_stateful_rec;
+    struct ls_stateful_record *ls_stateful_rec;
     struct ovn_lb_datapaths *lb_dps;
     struct ovn_port *op;
 
@@ -16431,6 +16436,11 @@  lflow_reset_northd_refs(struct lflow_input *lflow_input)
         lflow_ref_clear(lr_stateful_rec->lflow_ref);
     }
 
+    LS_STATEFUL_TABLE_FOR_EACH (ls_stateful_rec,
+                                lflow_input->ls_stateful_table) {
+        lflow_ref_clear(ls_stateful_rec->lflow_ref);
+    }
+
     HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) {
         lflow_ref_clear(op->lflow_ref);
         lflow_ref_clear(op->stateful_lflow_ref);
@@ -16750,6 +16760,47 @@  exit:
     return handled;
 }
 
+bool
+lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *ovnsb_txn,
+                                struct ls_stateful_tracked_data *trk_data,
+                                struct lflow_input *lflow_input,
+                                struct lflow_table *lflows)
+{
+    struct hmapx_node *hmapx_node;
+
+    HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) {
+        struct ls_stateful_record *ls_stateful_rec = hmapx_node->data;
+        const struct ovn_datapath *od =
+            ovn_datapaths_find_by_index(lflow_input->ls_datapaths,
+                                        ls_stateful_rec->ls_index);
+        ovs_assert(od->nbs && uuid_equals(&od->nbs->header_.uuid,
+                                          &ls_stateful_rec->nbs_uuid));
+
+        lflow_ref_unlink_lflows(ls_stateful_rec->lflow_ref);
+
+        /* Generate new lflows. */
+        build_ls_stateful_flows(ls_stateful_rec, od,
+                                lflow_input->ls_port_groups,
+                                lflow_input->features,
+                                lflow_input->meter_groups,
+                                lflows);
+
+        /* Sync the new flows to SB. */
+        bool handled = lflow_ref_sync_lflows(
+            ls_stateful_rec->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);
+        if (!handled) {
+            return false;
+        }
+    }
+
+    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 224b5d72ba..970b25f210 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -678,6 +678,7 @@  void northd_indices_create(struct northd_data *data,
 
 struct lflow_table;
 struct lr_stateful_tracked_data;
+struct ls_stateful_tracked_data;
 
 void build_lflows(struct ovsdb_idl_txn *ovnsb_txn,
                   struct lflow_input *input_data,
@@ -696,6 +697,10 @@  bool lflow_handle_lr_stateful_changes(struct ovsdb_idl_txn *,
                                       struct lr_stateful_tracked_data *,
                                       struct lflow_input *,
                                       struct lflow_table *lflows);
+bool lflow_handle_ls_stateful_changes(struct ovsdb_idl_txn *,
+                                      struct ls_stateful_tracked_data *,
+                                      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 8a20643989..29eb0953ee 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -10700,12 +10700,13 @@  check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
 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
 # A LB applied to a switch/router triggers:
 # - a recompute in the first iteration (handling northd change)
 # - a compute in the second iteration (handling SB update)
 check_engine_stats sync_to_sb_lb recompute compute
-CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
+check_engine_stats ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE((1))
 
 # Modify the backend of the lb1 vip
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
@@ -10713,7 +10714,8 @@  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 lflow recompute nocompute
+check_engine_stats ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
 
@@ -10723,7 +10725,8 @@  check ovn-nbctl --wait=sb clear load_Balancer lb1 vips
 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 ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
 
@@ -10733,7 +10736,8 @@  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 lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
 
@@ -10743,7 +10747,8 @@  check ovn-nbctl --wait=sb lb-add lb1 10.0.0.20:80 10.0.0.30:8080
 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 ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
 
@@ -10753,6 +10758,7 @@  check ovn-nbctl --wait=sb ls-lb-del sw0 lb1
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd recompute nocompute
 check_engine_stats lr_stateful recompute nocompute
+check_engine_stats ls_stateful recompute nocompute
 check_engine_stats lflow recompute nocompute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE(1)
@@ -10764,7 +10770,8 @@  check ovn-nbctl --wait=sb ls-lb-add sw0 lb1 -- lsp-add sw0 sw0p1
 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 ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
@@ -10883,7 +10890,7 @@  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 lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 
 # Update lb and this should not result in northd recompute
@@ -10891,8 +10898,9 @@  check as northd ovn-appctl -t ovn-northd 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 lr_stateful norecompute compute
 check_engine_stats ls_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 
 # Modify the backend of the lb1 vip
@@ -10902,7 +10910,7 @@  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 lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10912,7 +10920,8 @@  check ovn-nbctl --wait=sb clear load_Balancer lb1 vips
 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 ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10922,7 +10931,8 @@  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 lr_stateful norecompute compute
-check_engine_stats lflow recompute nocompute
+check_engine_stats ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -10932,7 +10942,8 @@  check ovn-nbctl --wait=sb lb-add lb1 10.0.0.20:80 10.0.0.30:8080
 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 ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11007,7 +11018,8 @@  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 lflow recompute nocompute
+check_engine_stats ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11060,7 +11072,8 @@  check ovn-nbctl --wait=sb set 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 lflow recompute nocompute
+check_engine_stats ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11078,7 +11091,8 @@  check ovn-nbctl --wait=sb ls-lb-add sw0 lb2
 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 ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11087,7 +11101,8 @@  check ovn-nbctl --wait=sb ls-lb-add sw0 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 ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11115,6 +11130,7 @@  check ovn-nbctl --wait=sb lr-lb-del lr1 lb2
 check_engine_stats lb_data norecompute compute
 check_engine_stats northd recompute nocompute
 check_engine_stats lr_stateful recompute nocompute
+check_engine_stats ls_stateful recompute nocompute
 check_engine_stats lflow recompute nocompute
 check_engine_stats sync_to_sb_lb recompute nocompute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
@@ -11126,7 +11142,8 @@  check ovn-nbctl --wait=sb lb-del lb4
 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 ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11137,7 +11154,8 @@  check ovn-nbctl --wait=sb lb-del lb2
 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 ls_stateful norecompute compute
+check_engine_stats lflow norecompute compute
 check_engine_stats sync_to_sb_lb recompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
@@ -11347,7 +11365,7 @@  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
 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_NO_CHANGE_AFTER_RECOMPUTE
 
 # Clear the VIPs of lb1
@@ -11355,7 +11373,7 @@  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb clear load_balancer . vips
 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_NO_CHANGE_AFTER_RECOMPUTE
 
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
@@ -11379,7 +11397,7 @@  check ovn-nbctl --wait=sb lb-add lb2 10.0.0.10:80 10.0.0.3:80
 
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb ls-lb-add sw0 lb1
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 lb_lflow_uuid=$(fetch_column Logical_flow _uuid match='"ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80"')
@@ -11393,7 +11411,7 @@  AT_CHECK([test "$lb_lflow_dpgrp" = ""])
 
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb ls-lb-add sw1 lb2
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 lb_lflow_dp=$(ovn-sbctl --bare --columns logical_datapath list logical_flow $lb_lflow_uuid)
@@ -11402,6 +11420,11 @@  AT_CHECK([test "$lb_lflow_dp" = ""])
 lb_lflow_dpgrp=$(ovn-sbctl --bare --columns logical_dp_group list logical_flow $lb_lflow_uuid)
 AT_CHECK([test "$lb_lflow_dpgrp" != ""])
 
+check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
+check ovn-nbctl --wait=sb set load_balancer lb1 options:bar=foo
+check_engine_stats lflow norecompute compute
+CHECK_NO_CHANGE_AFTER_RECOMPUTE
+
 # Clear the SB:Logical_Flow.logical_dp_groups column of all the
 # logical flows and then modify the NB:Load_balancer.  ovn-northd
 # should resync the logical flows.
@@ -11419,7 +11442,7 @@  lb_lflow_uuid=$(fetch_column Logical_flow _uuid match='"ct.new && ip4.dst == 10.
 
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb clear load_balancer lb2 vips
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 lb_lflow_dp=$(ovn-sbctl --bare --columns logical_datapath list logical_flow $lb_lflow_uuid)
@@ -11445,7 +11468,7 @@  check ovn-nbctl ls-lb-add sw4 lb2
 
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb ls-lb-add sw5 lb2
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 lb_lflow_dp=$(ovn-sbctl --bare --columns logical_datapath list logical_flow $lb_lflow_uuid)
@@ -11476,7 +11499,7 @@  echo "dpgrp_dps - $dpgrp_dps"
 
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb clear load_balancer lb2 vips
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 lb_lflow_dpgrp=$(ovn-sbctl --bare --columns logical_dp_group list logical_flow $lb_lflow_uuid)
@@ -11510,7 +11533,7 @@  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 
 check ovn-nbctl ls-lb-add sw0 lb3
 check ovn-nbctl --wait=sb ls-lb-add sw1 lb3
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 lb_lflow_uuid=$(fetch_column Logical_flow _uuid match='"ct.new && ip4.dst == 10.0.0.10 && tcp.dst == 80"')
@@ -11535,7 +11558,7 @@  AT_CHECK([echo $dpgrp_dps | grep $sw5_uuid], [0], [ignore])
 # should have reference to sw0 and sw1, but not to sw2.
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb clear load_balancer lb1 vips
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 lb_lflow_dp=$(ovn-sbctl --bare --columns logical_datapath list logical_flow $lb_lflow_uuid)
@@ -11561,7 +11584,7 @@  AT_CHECK([echo $dpgrp_dps | grep $sw5_uuid], [0], [ignore])
 
 check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
 check ovn-nbctl --wait=sb clear load_balancer lb3 vips
-check_engine_stats lflow recompute nocompute
+check_engine_stats lflow norecompute compute
 CHECK_NO_CHANGE_AFTER_RECOMPUTE
 
 lb_lflow_dp=$(ovn-sbctl --bare --columns logical_datapath list logical_flow $lb_lflow_uuid)