From: Numan Siddique <num...@ovn.org>

Signed-off-by: Numan Siddique <num...@ovn.org>
---
 northd/en-lflow.c        | 29 +++++++++++++++++++
 northd/en-lflow.h        |  1 +
 northd/en-ls-stateful.c  |  3 ++
 northd/en-ls-stateful.h  | 23 +++++++++++++++
 northd/inc-proc-northd.c |  3 +-
 northd/northd.c          | 56 +++++++++++++++++++++++++++++++++----
 northd/northd.h          |  5 ++++
 tests/ovn-northd.at      | 60 ++++++++++++++++++++++++++--------------
 8 files changed, 152 insertions(+), 28 deletions(-)

diff --git a/northd/en-lflow.c b/northd/en-lflow.c
index 09748f570b..784c3c2efb 100644
--- a/northd/en-lflow.c
+++ b/northd/en-lflow.c
@@ -193,6 +193,35 @@ 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 117def531b..8b695e04fa 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);
@@ -298,6 +299,7 @@ ls_stateful_record_create(struct ls_stateful_table *table,
     struct ls_stateful_record *ls_sful_rec = xzalloc(sizeof *ls_sful_rec);
     ls_sful_rec->od = od;
     ls_stateful_record_init(ls_sful_rec, od, NULL, ls_pgs);
+    ls_sful_rec->lflow_ref = lflow_ref_alloc(od->nbs->name);
 
     hmap_insert(&table->entries, &ls_sful_rec->key_node,
                 uuid_hash(&ls_sful_rec->od->nbs->header_.uuid));
@@ -308,6 +310,7 @@ ls_stateful_record_create(struct ls_stateful_table *table,
 static void
 ls_stateful_record_destroy(struct ls_stateful_record *ls_sful_rec)
 {
+    lflow_ref_destroy(ls_sful_rec->lflow_ref);
     free(ls_sful_rec);
 }
 
diff --git a/northd/en-ls-stateful.h b/northd/en-ls-stateful.h
index e409bb0141..5d7df849b9 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;
 
@@ -39,6 +41,27 @@ 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.
+     */
+    struct lflow_ref *lflow_ref;
 };
 
 struct ls_stateful_table {
diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
index 8be413200e..f7c3d2bcf5 100644
--- a/northd/inc-proc-northd.c
+++ b/northd/inc-proc-northd.c
@@ -228,12 +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_ls_stateful, 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 235b9e100f..59b30c5dc8 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -15754,12 +15754,14 @@ build_ls_stateful_flows(const struct 
ls_stateful_record *ls_stateful_rec,
     ovs_assert(ls_stateful_rec->od);
 
     build_ls_stateful_rec_pre_acls(ls_stateful_rec, ls_pgs, lflows,
-                                   NULL);
-    build_ls_stateful_rec_pre_lb(ls_stateful_rec, lflows, NULL);
-    build_acl_hints(ls_stateful_rec, features, lflows, NULL);
+                                   ls_stateful_rec->lflow_ref);
+    build_ls_stateful_rec_pre_lb(ls_stateful_rec, lflows,
+                                 ls_stateful_rec->lflow_ref);
+    build_acl_hints(ls_stateful_rec, features, lflows,
+                    ls_stateful_rec->lflow_ref);
     build_acls(ls_stateful_rec, features, lflows, ls_pgs, meter_groups,
-               NULL);
-    build_lb_hairpin(ls_stateful_rec, lflows, NULL);
+               ls_stateful_rec->lflow_ref);
+    build_lb_hairpin(ls_stateful_rec, lflows, ls_stateful_rec->lflow_ref);
 }
 
 struct lswitch_flow_build_info {
@@ -16433,6 +16435,7 @@ void
 reset_lflow_refs_for_northd_resources(struct lflow_input *lflow_input)
 {
     const struct lr_stateful_record *lr_sful_rec;
+    struct ls_stateful_record *ls_sful_rec;
     struct ovn_lb_datapaths *lb_dps;
     struct ovn_port *op;
 
@@ -16440,6 +16443,10 @@ reset_lflow_refs_for_northd_resources(struct 
lflow_input *lflow_input)
         lflow_ref_reset(lr_sful_rec->lflow_ref);
     }
 
+    LS_STATEFUL_TABLE_FOR_EACH (ls_sful_rec, lflow_input->ls_sful_table) {
+        lflow_ref_reset(ls_sful_rec->lflow_ref);
+    }
+
     HMAP_FOR_EACH (op, key_node, lflow_input->ls_ports) {
         lflow_ref_reset(op->lflow_ref);
         lflow_ref_reset(op->stateful_lflow_ref);
@@ -16749,6 +16756,45 @@ lflow_handle_lr_stateful_changes(struct ovsdb_idl_txn 
*ovnsb_txn,
     return true;
 }
 
+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 ls_stateful_record *ls_sful_rec;
+    struct hmapx_node *hmapx_node;
+
+    HMAPX_FOR_EACH (hmapx_node, &trk_data->crupdated) {
+        ls_sful_rec = hmapx_node->data;
+
+        lflow_ref_clear_lflows(ls_sful_rec->lflow_ref);
+
+        /* Generate new lflows. */
+        struct ds match = DS_EMPTY_INITIALIZER;
+        struct ds actions = DS_EMPTY_INITIALIZER;
+
+        build_ls_stateful_flows(ls_sful_rec, lflow_input->ls_port_groups,
+                                lflow_input->features,
+                                lflow_input->meter_groups,
+                                lflows);
+
+        ds_destroy(&match);
+        ds_destroy(&actions);
+
+        /* Sync the new flows to SB. */
+        lflow_ref_sync_lflows_to_sb(ls_sful_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);
+    }
+
+    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 6adf06c26f..c665092a09 100644
--- a/northd/northd.h
+++ b/northd/northd.h
@@ -720,6 +720,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,
@@ -738,6 +739,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 f517057534..9b2886493c 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -10603,12 +10603,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 NORTHD_TYPE inc-engine/clear-stats
@@ -10616,7 +10617,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)
 
@@ -10626,7 +10628,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)
 
@@ -10636,7 +10639,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)
 
@@ -10646,7 +10650,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)
 
@@ -10656,6 +10661,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)
@@ -10667,7 +10673,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
@@ -10787,7 +10794,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
@@ -10795,8 +10802,9 @@ 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 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
@@ -10806,7 +10814,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
 
@@ -10816,7 +10824,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
 
@@ -10826,7 +10835,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
 
@@ -10836,7 +10846,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
 
@@ -10911,7 +10922,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
 
@@ -10964,7 +10976,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
 
@@ -10982,7 +10995,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
 
@@ -10991,7 +11005,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
 
@@ -11019,6 +11034,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
@@ -11030,7 +11046,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
 
@@ -11041,7 +11058,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
 
@@ -11075,7 +11093,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 
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
@@ -11083,7 +11101,7 @@ check as northd ovn-appctl -t NORTHD_TYPE 
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 NORTHD_TYPE inc-engine/clear-stats
-- 
2.41.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to