On Thu, Oct 15, 2026 at 4:37 PM Jacob Tanenbaum <[email protected]> wrote:
>
>
>
> 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.
I think this is just me being overly defensive. I'll switch this to an
ovs_assert() instead.
>
>
>
>
>>
>> + 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
