| Message ID | 20231128023738.569962-1-numans@ovn.org |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | northd lflow incremental processing | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | success | github build: passed |
On 11/28/23 03:37, numans@ovn.org wrote: > From: Numan Siddique <numans@ovn.org> > > Signed-off-by: Numan Siddique <numans@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; > + } > + > + Nit: too many empty lines. > + engine_set_node_state(node, EN_UPDATED); > + I'd skip this one too, I think. > + 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); I mentioned this one on the previous patch. It was a typo there I guess. > 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; These two variables are unused, they can be removed. > + > + 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)) Why the double parenthesis? Typo? > > # 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 Regards, Dumitru
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