With this change, en-lflow will now incrementally process logical router policy changes.
The lflow_ref is scoped to the datapath on which logical router policies are attached. In the case that any logical router policies are added or deleted on an existing logical router all of the policies on that logical router will have their logical flows rebuilt and resynced. Signed-off-by: Mark Michelson <[email protected]> --- northd/en-lflow.c | 33 ++++++++++++++++++++++++++++++++ northd/en-lflow.h | 2 ++ northd/inc-proc-northd.c | 3 ++- northd/northd.c | 30 +++++++++++++++++++++++++++++ northd/northd.h | 6 ++++++ tests/ovn-inc-proc-graph-dump.at | 2 +- tests/ovn-northd.at | 20 +++++++++++-------- 7 files changed, 86 insertions(+), 10 deletions(-) diff --git a/northd/en-lflow.c b/northd/en-lflow.c index b5204a8f2..1bf8da227 100644 --- a/northd/en-lflow.c +++ b/northd/en-lflow.c @@ -311,6 +311,39 @@ lflow_ic_learned_svc_mons_handler(struct engine_node *node, return EN_HANDLED_UPDATED; } +enum engine_input_handler_result +lflow_route_policies_handler(struct engine_node *node, + void *data) +{ + struct route_policies_data *rp_data = + engine_get_input_data("route_policies", node); + + if (!rp_data->trk.has_tracked) { + return EN_UNHANDLED; + } + + if (!rp_data->trk.has_tracked_policies) { + /* en-route-policies computed incrementally, but there + * were no changes to any of the policies. Therefore, + * there is no need for en-lflow to take action. + */ + return EN_HANDLED_UNCHANGED; + } + + struct lflow_data *lflow_data = data; + + struct lflow_input lflow_input; + lflow_get_input_data(node, &lflow_input); + + if (!lflow_handle_route_policies_changes( + rp_data, &lflow_input, lflow_data->lflow_table, + &lflow_data->trk_data.dirty_lflow_refs)) { + return EN_UNHANDLED; + } + + return EN_HANDLED_UPDATED; +} + 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 fd3f2427f..26dd5c9e9 100644 --- a/northd/en-lflow.h +++ b/northd/en-lflow.h @@ -38,4 +38,6 @@ enum engine_input_handler_result lflow_group_ecmp_route_change_handler(struct engine_node *node, void *data); enum engine_input_handler_result lflow_ic_learned_svc_mons_handler(struct engine_node *node, void *data); +enum engine_input_handler_result +lflow_route_policies_handler(struct engine_node *node, void *data); #endif /* EN_LFLOW_H */ diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 0fde77032..603590697 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -400,7 +400,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_lflow, &en_sync_meters, NULL); engine_add_input(&en_lflow, &en_sb_multicast_group, NULL); engine_add_input(&en_lflow, &en_bfd_sync, NULL); - engine_add_input(&en_lflow, &en_route_policies, NULL); + engine_add_input(&en_lflow, &en_route_policies, + lflow_route_policies_handler); /* Route changes are propagated to en_lflow through the en_group_ecmp_route * input. Any change to en_routes also triggers en_group_ecmp_route (via * group_ecmp_route_routes_change_handler), which then triggers en_lflow. diff --git a/northd/northd.c b/northd/northd.c index 3fa84963d..996eb2747 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -21067,6 +21067,36 @@ lflow_handle_ls_stateful_changes(struct ls_stateful_tracked_data *trk_data, } } +bool +lflow_handle_route_policies_changes(struct route_policies_data *rp_data, + struct lflow_input *lflow_input, + struct lflow_table *lflows, + struct hmapx *dirty_lflow_refs) +{ + struct hmapx_node *hmapx_node; + struct datapath_route_policies *dp_rp; + HMAPX_FOR_EACH (hmapx_node, &rp_data->trk.deleted_policies) { + dp_rp = hmapx_node->data; + lflow_ref_unlink_lflows(dp_rp->lflow_ref, lflows); + hmapx_add(dirty_lflow_refs, dp_rp->lflow_ref); + } + + HMAPX_FOR_EACH (hmapx_node, &rp_data->trk.new_policies) { + dp_rp = hmapx_node->data; + struct ovn_datapath *od = + sparse_array_get(&lflow_input->lr_datapaths->dps, dp_rp->dp_index); + if (!od) { + return false; + } + lflow_ref_unlink_lflows(dp_rp->lflow_ref, lflows); + build_ingress_policy_flows_for_lrouter( + od, lflows, &dp_rp->route_policies, dp_rp->lflow_ref); + hmapx_add(dirty_lflow_refs, dp_rp->lflow_ref); + } + + 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 fb7d3e44e..a8985b603 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -1017,6 +1017,12 @@ bool northd_handle_lb_data_changes(struct tracked_lb_data *, void bfd_table_sync(struct ovsdb_idl_txn *, const struct hmap *, struct hmap *, struct sset *); +struct route_policies_data; +bool lflow_handle_route_policies_changes(struct route_policies_data *rp_data, + struct lflow_input *lflow_input, + struct lflow_table *lflows, + struct hmapx *dirty_lflow_refs); + void build_bfd_map(const struct nbrec_bfd_table *, const struct sbrec_bfd_table *, struct hmap *, const struct uuidset *); diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ovn-inc-proc-graph-dump.at index 5686e0208..8cd199b11 100644 --- a/tests/ovn-inc-proc-graph-dump.at +++ b/tests/ovn-inc-proc-graph-dump.at @@ -183,7 +183,7 @@ digraph "Incremental-Processing-Engine" { sync_meters -> lflow [[label=""]]; SB_multicast_group -> lflow [[label=""]]; bfd_sync -> lflow [[label=""]]; - route_policies -> lflow [[label=""]]; + route_policies -> lflow [[label="lflow_route_policies_handler"]]; routes -> lflow [[label="engine_noop_handler"]]; group_ecmp_route -> lflow [[label="lflow_group_ecmp_route_change_handler"]]; global_config -> lflow [[label="node_global_config_handler"]]; diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index e11ebc6d5..ffc387aa5 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -4690,7 +4690,11 @@ AT_CHECK([ovn-nbctl list logical_router_policy | grep -q $bfd_route_policy_uuid] check_engine_stats northd norecompute incremental check_engine_stats route_policies norecompute compute -check_engine_stats lflow recompute nocompute +# The BFD changes trigger a recompute of en-lflow, since en-routes cannot +# incrementally handle BFD changes. However, en-lflow is able to incrementally +# handle the added logical router policy. This is why we see both recompute +# and compute true for en-lflow. +check_engine_stats lflow recompute compute check_engine_stats northd_output norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats @@ -24656,7 +24660,7 @@ check_engine_stats sync_from_sb norecompute nocompute check_engine_stats sync_to_sb_lb norecompute compute check_engine_stats route_policies norecompute compute check_engine_stats bfd_sync recompute nocompute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE # Change router Policy to use explicit output port. @@ -24684,7 +24688,7 @@ check_engine_stats sync_from_sb norecompute nocompute check_engine_stats sync_to_sb_lb norecompute compute check_engine_stats route_policies norecompute compute check_engine_stats bfd_sync recompute nocompute -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 @@ -24710,7 +24714,7 @@ check_engine_stats sync_from_sb norecompute nocompute check_engine_stats sync_to_sb_lb norecompute compute check_engine_stats route_policies norecompute compute check_engine_stats bfd_sync recompute nocompute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE # From this point on, operations we perform may result in @@ -24729,7 +24733,7 @@ check ovn-nbctl --wait=sb sync check_engine_stats northd norecompute compute check_engine_stats route_policies norecompute compute check_engine_stats bfd_sync recompute nocompute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE # Invalidate the policy by changing it to a jump policy but not @@ -24741,7 +24745,7 @@ check ovn-nbctl set Logical_Router_Policy $policy_uuid action=jump check_engine_stats northd norecompute compute check_engine_stats route_policies norecompute compute check_engine_stats bfd_sync recompute nocompute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE # Un-invalidate the policy by changing it back to an allow policy. @@ -24751,7 +24755,7 @@ check ovn-nbctl set Logical_Router_Policy $policy_uuid action=allow check_engine_stats northd norecompute compute check_engine_stats route_policies norecompute compute check_engine_stats bfd_sync recompute nocompute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE # Add a new logical router with an invalid policy in the same transaction @@ -24777,7 +24781,7 @@ check ovn-nbctl --wait=sb lr-del lr1 check_engine_stats northd norecompute compute check_engine_stats route_policies norecompute compute check_engine_stats bfd_sync recompute nocompute -check_engine_stats lflow recompute nocompute +check_engine_stats lflow norecompute compute CHECK_NO_CHANGE_AFTER_RECOMPUTE OVN_CLEANUP_NORTHD -- 2.55.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
