On Tue, Oct 6, 2026 at 1:51 PM Mark Michelson via dev <
[email protected]> wrote:
> 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.
>
router policy processing. The only change that
outport_key = out_port->nbrp->name at en-route-policies.c:296) and
│ actually makes a difference is if the
logical router ports change, and
a string copy (src_addr, en-route-policies.c:298). They never hold
│ there are "reroute" policies installed
on the log
>
> 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;
> + }
>
Is the "if (!od)" check required? new_policy entries are produced by
en_route_policies from od->sdp->index which is the same index used to put
datapaths in lr_datapaths
and the full-recompute loop does not check for datapath existence.
> + 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
>
Thanks,
Jacob Tanenbaum
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
