Hi
Thanks for your review.

Em qua., 8 de jul. de 2026 às 09:02, Dumitru Ceara <[email protected]>
escreveu:

> On 7/1/26 6:49 PM, Lucas Vargas Dias wrote:
> > Create a handler for static routes. It handles update, create or
> > delete.
> > Full recompute is triggered when update bfd column for static route.
> >
> > Test with 2000 static routes created in the same logical router
> > and add a new one:
> > Without the incremental processing:
> > ovn-nbctl --print-wait-time --wait=sb lr-route-add lr1-2 10.0.0.1/32
> 192.168.20.2
> > Time spent on processing nb_cfg 4:
> >       ovn-northd delay before processing:     4ms
> >       ovn-northd completion:                  62ms
> >
> > With the incremental processing:
> > ovn-nbctl --print-wait-time --wait=sb lr-route-add lr1-2 10.0.0.1/32
> 192.168.20.2
> > Time spent on processing nb_cfg 6:
> >       ovn-northd delay before processing:     1ms
> >       ovn-northd completion:                  18ms
> >
> > Test with 2000 static routes created in the same logical router
> > and delete one:
> > Without the incremental processing:
> > ovn-nbctl --print-wait-time --wait=sb lr-route-del lr1-2 10.0.0.1/32
> 192.168.20.2
> > Time spent on processing nb_cfg 5:
> >       ovn-northd delay before processing:     3ms
> >       ovn-northd completion:                  62ms
> >
> > With the incremental processing:
> > ovn-nbctl --print-wait-time --wait=sb lr-route-del lr1-2 10.0.0.1/32
> 192.168.20.2
> > Time spent on processing nb_cfg 9:
> >       ovn-northd delay before processing:     10ms
> >       ovn-northd completion:                  28ms
> >
> > Signed-off-by: Lucas Vargas Dias <[email protected]>
> > ---
>
> Hi Lucas,
>
> Thanks for this new revision!
>
> >  northd/en-group-ecmp-route.c     |  52 ++++++++++
> >  northd/en-group-ecmp-route.h     |   4 +
> >  northd/en-lflow.c                |  12 +++
> >  northd/en-northd.c               | 161 +++++++++++++++++++++++++++++--
> >  northd/en-northd.h               |   7 ++
> >  northd/inc-proc-northd.c         |  21 ++--
> >  northd/northd.c                  | 113 +++++++++++++++++-----
> >  northd/northd.h                  |  32 +++++-
> >  tests/ovn-inc-proc-graph-dump.at |   8 +-
> >  tests/ovn-northd.at              | 135 ++++++++++++++++++++++++--
> >  10 files changed, 493 insertions(+), 52 deletions(-)
> >
> > diff --git a/northd/en-group-ecmp-route.c b/northd/en-group-ecmp-route.c
> > index c4c93fd84..3c5d898d1 100644
> > --- a/northd/en-group-ecmp-route.c
> > +++ b/northd/en-group-ecmp-route.c
> > @@ -519,3 +519,55 @@
> group_ecmp_route_learned_route_change_handler(struct engine_node *eng_node,
> >      }
> >      return EN_HANDLED_UNCHANGED;
> >  }
> > +
> > +enum engine_input_handler_result
> > +group_ecmp_route_routes_change_handler(struct engine_node *eng_node,
> > +                                       void *_data)
> > +{
> > +    struct routes_data *routes_data
> > +        = engine_get_input_data("routes", eng_node);
> > +    struct group_ecmp_route_data *data = _data;
> > +    if (!routes_data->tracked) {
> > +        data->tracked = false;
>
> There's actually no need to do this here, it's automatically handled
> because group_ecmp_route_clear_tracked() gets called on failure to
> incrementally process input changes.
>
> I know we already did it in
> group_ecmp_route_learned_route_change_handler() but we should probably
> remove it from there too.  It doesn't add much and potentially just
> confuses future readers.
>
>
I agree


> > +        return EN_UNHANDLED;
> > +    }
>
> Functionally there's no difference but I'd set data->tracked = true here
> instead of lower (in the end of the function).  And empty "set of
> tracked data" is still valid tracked data.
>
> It's also what group_ecmp_route_learned_route_change_handler() does so
> we'd be more consistent like that.
>
>
I agree


> > +
> > +    struct parsed_route *pr;
> > +    struct hmapx updated_routes = HMAPX_INITIALIZER(&updated_routes);
> > +
> > +    const struct hmapx_node *hmapx_node;
> > +    HMAPX_FOR_EACH (hmapx_node,
> > +                    &routes_data->trk_data.trk_deleted_parsed_route) {
> > +        pr = hmapx_node->data;
> > +        if (!handle_deleted_route(data, pr, &updated_routes)) {
> > +            hmapx_destroy(&updated_routes);
> > +            return EN_UNHANDLED;
> > +        }
> > +    }
> > +
> > +    HMAPX_FOR_EACH (hmapx_node,
> > +                    &routes_data->trk_data.trk_crupdated_parsed_route) {
> > +        pr = hmapx_node->data;
> > +        handle_added_route(data, pr, &updated_routes);
> > +    }
> > +
> > +    HMAPX_FOR_EACH (hmapx_node, &updated_routes) {
> > +        struct group_ecmp_datapath *node = hmapx_node->data;
> > +        if (hmap_is_empty(&node->unique_routes) &&
> > +                hmap_is_empty(&node->ecmp_groups)) {
> > +            hmapx_add(&data->trk_data.deleted_datapath_routes, node);
> > +            hmap_remove(&data->datapaths, &node->hmap_node);
> > +        } else {
> > +            hmapx_add(&data->trk_data.crupdated_datapath_routes, node);
> > +        }
> > +    }
> > +
> > +    hmapx_destroy(&updated_routes);
> > +
> > +    if (!hmapx_is_empty(&data->trk_data.crupdated_datapath_routes) ||
> > +        !hmapx_is_empty(&data->trk_data.deleted_datapath_routes)) {
> > +        data->tracked = true;
> > +        return EN_HANDLED_UPDATED;
> > +    }
> > +    return EN_HANDLED_UNCHANGED;
> > +}
> > diff --git a/northd/en-group-ecmp-route.h b/northd/en-group-ecmp-route.h
> > index d4a3248d0..aa6b26015 100644
> > --- a/northd/en-group-ecmp-route.h
> > +++ b/northd/en-group-ecmp-route.h
> > @@ -98,6 +98,10 @@ enum engine_input_handler_result
> >  group_ecmp_route_learned_route_change_handler(struct engine_node *,
> >                                                void *data);
> >
> > +enum engine_input_handler_result
> > +group_ecmp_route_routes_change_handler(struct engine_node *,
> > +                                       void *data);
> > +
> >  struct group_ecmp_datapath *group_ecmp_datapath_lookup(
> >      const struct group_ecmp_route_data *data,
> >      const struct ovn_datapath *od);
> > diff --git a/northd/en-lflow.c b/northd/en-lflow.c
> > index 8cb987777..5632135db 100644
> > --- a/northd/en-lflow.c
> > +++ b/northd/en-lflow.c
> > @@ -310,6 +310,8 @@ lflow_group_ecmp_route_change_handler(struct
> engine_node *node,
> >      /* Now we handle created or updated route nodes. */
> >      struct hmapx *crupdated_datapath_routes =
> >          &group_ecmp_route_data->trk_data.crupdated_datapath_routes;
> > +    struct ds actions = DS_EMPTY_INITIALIZER;
> > +    struct ds match = DS_EMPTY_INITIALIZER;
> >      HMAPX_FOR_EACH (hmapx_node, crupdated_datapath_routes) {
> >          route_node = hmapx_node->data;
> >          lflow_ref_unlink_lflows(route_node->lflow_ref);
> > @@ -317,6 +319,12 @@ lflow_group_ecmp_route_change_handler(struct
> engine_node *node,
> >              route_node->od, lflow_data->lflow_table,
> >              route_node, lflow_input.bfd_ports);
> >
> > +        build_arp_request_flows_for_lrouter(route_node->od,
> > +                                            lflow_data->lflow_table,
> > +                                            &match, &actions,
> > +                                            lflow_input.meter_groups,
> > +                                            route_node->lflow_ref);
>
> In the recompute path, in build_lswitch_and_lrouter_iterate_by_lr() we
> pass od's lflow_ref to build_arp_request_flows_for_lrouter().  But here,
> in the incremental path, we use the route_node->lflow_ref.
>
> Looking more closely at what that function does, it seems weird that it
> needs to do specific processing for static IPv6 routes.  I think we
> don't really need most of what that function is doing, I posted a patch
> to fix that here:
>
>
> https://patchwork.ozlabs.org/project/ovn/patch/[email protected]/
>
> It would be great if you could review it (I cc-ed Numan on it too).
> Once we apply that one, your changes would also be simplified - this
> whole chunk here can then be removed.
>
>
Sure, I can review.


> > +
> >          bool handled = lflow_ref_sync_lflows(
> >              route_node->lflow_ref, lflow_data->lflow_table,
> >              eng_ctx->ovnsb_idl_txn, lflow_input.dps,
> > @@ -324,10 +332,14 @@ lflow_group_ecmp_route_change_handler(struct
> engine_node *node,
> >              lflow_input.sbrec_logical_flow_table,
> >              lflow_input.sbrec_logical_dp_group_table);
> >          if (!handled) {
> > +            ds_destroy(&match);
> > +            ds_destroy(&actions);
> >              return EN_UNHANDLED;
> >          }
> >      }
> >
> > +    ds_destroy(&match);
> > +    ds_destroy(&actions);
> >      return EN_HANDLED_UPDATED;
> >  }
> >
> > diff --git a/northd/en-northd.c b/northd/en-northd.c
> > index c34818dba..f8e6ff147 100644
> > --- a/northd/en-northd.c
> > +++ b/northd/en-northd.c
> > @@ -207,7 +207,8 @@ northd_nb_logical_router_handler(struct engine_node
> *node,
> >      }
> >
> >      if (northd_has_lr_nats_in_tracked_data(&nd->trk_data) ||
> > -        northd_has_lrouters_in_tracked_data(&nd->trk_data)) {
> > +        northd_has_lrouters_in_tracked_data(&nd->trk_data) ||
> > +        northd_has_lr_route_in_tracked_data(&nd->trk_data)) {
> >          return EN_HANDLED_UPDATED;
> >      }
> >
> > @@ -329,7 +330,7 @@ en_route_policies_run(struct engine_node *node, void
> *data)
> >
> >  enum engine_input_handler_result
> >  routes_northd_change_handler(struct engine_node *node,
> > -                                    void *data OVS_UNUSED)
> > +                             void *data OVS_UNUSED)
> >  {
> >      struct northd_data *northd_data = engine_get_input_data("northd",
> node);
> >      if (!northd_has_tracked_data(&northd_data->trk_data)) {
> > @@ -347,15 +348,143 @@ routes_northd_change_handler(struct engine_node
> *node,
> >       *      Note: When we add I-P to the created/deleted logical
> routers or
> >       *      logical router ports, we need to revisit this handler.
> >       *
> > -     *      This node also accesses the static routes of the logical
> router.
> > -     *      When these static routes gets updated, en_northd engine
> recomputes
> > -     *      and so does this node.
> > -     *      Note: When we add I-P to handle static routes changes, we
> need
> > -     *      to revisit this handler.
> >       */
> >      return EN_HANDLED_UNCHANGED;
> >  }
> >
> > +static bool
> > +static_route_is_relevant_updated(
> > +    const struct nbrec_logical_router_static_route *sr)
> > +{
> > +    return nbrec_logical_router_static_route_is_updated(sr,
> > +                NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_NEXTHOP)
> > +        || nbrec_logical_router_static_route_is_updated(sr,
> > +                NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_IP_PREFIX)
> > +        || nbrec_logical_router_static_route_is_updated(sr,
> > +                NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_OUTPUT_PORT)
> > +        || nbrec_logical_router_static_route_is_updated(sr,
> > +                NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_POLICY)
> > +        || nbrec_logical_router_static_route_is_updated(sr,
> > +                NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_ROUTE_TABLE)
> > +        || nbrec_logical_router_static_route_is_updated(sr,
> > +                NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_SELECTION_FIELDS)
> > +        || nbrec_logical_router_static_route_is_updated(sr,
> > +                NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_OPTIONS);
>
> Nit: I'd list these in the same order as we have them defined in the
> schema.  I'd also add BFD here; I know we fall back to recompute early,
> before we call this function if BFD is changed but it feels more
> complete to include it here - essentially include everything except
> external_ids.
>

I agree


>
> > +}
> > +
> > +static struct parsed_route *
> > +static_route_lookup_parsed(struct routes_data *routes_data,
> > +                           const struct
> nbrec_logical_router_static_route *sr)
> > +{
> > +    struct hmap *routes = &routes_data->parsed_routes;
> > +    struct parsed_route *pr = parsed_route_lookup_by_source(
> > +        ROUTE_SOURCE_STATIC, &sr->header_, routes);
> > +    if (!pr) {
> > +        pr = parsed_route_lookup_by_source(
> > +            ROUTE_SOURCE_IC_DYNAMIC, &sr->header_, routes);
> > +    }
> > +    return pr;
> > +}
> > +
> > +enum engine_input_handler_result
> > +routes_static_route_change_handler(struct engine_node *node,
> > +                                   void *data)
> > +{
> > +    struct routes_data *routes_data = data;
> > +    const struct nbrec_logical_router_static_route_table *
> > +        nb_lr_static_route_table =
> > +        EN_OVSDB_GET(engine_get_input("NB_logical_router_static_route",
> node));
> > +    struct northd_data *northd_data = engine_get_input_data("northd",
> node);
> > +    struct bfd_data *bfd_data = engine_get_input_data("bfd", node);
> > +    struct parsed_route *pr;
> > +
> > +    /* Logical routers reference their static routes, so a route change
> also
> > +     * marks the router as updated.  Iterate the updated routers and
> their
> > +     * routes to handle created and updated routes, avoiding a reverse
> lookup
> > +     * from route to router. */
> > +    struct hmapx_node *hmapx_node;
> > +    HMAPX_FOR_EACH (hmapx_node, &northd_data->trk_data.trk_lrs_routes) {
> > +        struct ovn_datapath *od = hmapx_node->data;
> > +        for (size_t i = 0; i < od->nbr->n_static_routes; i++) {
> > +            const struct nbrec_logical_router_static_route *sr =
> > +                od->nbr->static_routes[i];
> > +
> > +            if (nbrec_logical_router_static_route_is_new(sr)) {
> > +                pr = parsed_routes_add_static(od,
> &northd_data->lr_ports, sr,
> > +                        &bfd_data->bfd_connections,
> > +                        &routes_data->parsed_routes,
> > +                        &routes_data->route_tables,
> > +                        &routes_data->bfd_active_connections);
> > +                if (!pr) {
> > +                    return EN_UNHANDLED;
> > +                }
> > +
> hmapx_add(&routes_data->trk_data.trk_crupdated_parsed_route,
> > +                          pr);
> > +                continue;
> > +            }
> > +
> > +            /* A BFD column change requires a full recompute. */
> > +            if (nbrec_logical_router_static_route_is_updated(sr,
> > +                    NBREC_LOGICAL_ROUTER_STATIC_ROUTE_COL_BFD)) {
> > +                return EN_UNHANDLED;
> > +            }
> > +
> > +            if (!static_route_is_relevant_updated(sr)) {
> > +                continue;
> > +            }
> > +
> > +            pr = static_route_lookup_parsed(routes_data, sr);
> > +            if (!pr || !pr->od) {
> > +                return EN_UNHANDLED;
> > +            }
> > +            hmapx_add(&routes_data->trk_data.trk_deleted_parsed_route,
> pr);
> > +            hmap_remove(&routes_data->parsed_routes, &pr->key_node);
> > +            pr = parsed_routes_add_static(od, &northd_data->lr_ports,
> sr,
> > +                    &bfd_data->bfd_connections,
> > +                    &routes_data->parsed_routes,
> > +                    &routes_data->route_tables,
> > +                    &routes_data->bfd_active_connections);
> > +            if (!pr) {
> > +                /* engine_recompute() will clear the tracked data and
> free the
> > +                 * route we just moved to trk_deleted_parsed_route. */
> > +                return EN_UNHANDLED;
> > +            }
> > +
> hmapx_add(&routes_data->trk_data.trk_crupdated_parsed_route, pr);
> > +        }
> > +    }
> > +
> > +    /* Deleted routes are no longer referenced by any router, so handle
> them
> > +     * by iterating the tracked static route table. */
> > +    const struct nbrec_logical_router_static_route
> *changed_static_route;
> > +    NBREC_LOGICAL_ROUTER_STATIC_ROUTE_TABLE_FOR_EACH_TRACKED (
> > +                            changed_static_route,
> nb_lr_static_route_table) {
> > +        if (!nbrec_logical_router_static_route_is_deleted(
> > +                changed_static_route)) {
> > +            continue;
> > +        }
> > +        /* A route created and deleted within the same transaction
> never got a
> > +         * parsed route. */
> > +        if
> (nbrec_logical_router_static_route_is_new(changed_static_route)) {
> > +            continue;
> > +        }
> > +
> > +        pr = static_route_lookup_parsed(routes_data,
> changed_static_route);
> > +        if (!pr) {
> > +            return EN_UNHANDLED;
> > +        }
> > +        hmapx_add(&routes_data->trk_data.trk_deleted_parsed_route, pr);
> > +        hmap_remove(&routes_data->parsed_routes, &pr->key_node);
> > +    }
> > +
> > +    if
> (!hmapx_is_empty(&routes_data->trk_data.trk_crupdated_parsed_route) ||
> > +
> !hmapx_is_empty(&routes_data->trk_data.trk_deleted_parsed_route)) {
> > +        routes_data->tracked = true;
> > +        return EN_HANDLED_UPDATED;
> > +    }
> > +
> > +    return EN_HANDLED_UNCHANGED;
> > +}
> > +
> >  enum engine_node_state
> >  en_routes_run(struct engine_node *node, void *data)
> >  {
> > @@ -421,6 +550,18 @@ bfd_sync_northd_change_handler(struct engine_node
> *node, void *data OVS_UNUSED)
> >      return EN_HANDLED_UNCHANGED;
> >  }
> >
> > +enum engine_input_handler_result
> > +bfd_sync_routes_change_handler(struct engine_node *node,
> > +                               void *data OVS_UNUSED)
> > +{
> > +    struct routes_data *routes_data
> > +        = engine_get_input_data("routes", node);
> > +    if (!routes_data->tracked) {
> > +        return EN_UNHANDLED;
> > +    }
> > +    return EN_HANDLED_UNCHANGED;
> > +}
> > +
> >  enum engine_node_state
> >  en_bfd_sync_run(struct engine_node *node, void *data)
> >  {
> > @@ -590,6 +731,12 @@ en_routes_cleanup(void *data)
> >      routes_destroy(data);
> >  }
> >
> > +void
> > +en_routes_clear_tracked_data(void *data)
> > +{
> > +    routes_clear_tracked(data);
> > +}
> > +
> >  void
> >  en_bfd_cleanup(void *data)
> >  {
> > diff --git a/northd/en-northd.h b/northd/en-northd.h
> > index 7794739b9..c62631008 100644
> > --- a/northd/en-northd.h
> > +++ b/northd/en-northd.h
> > @@ -39,9 +39,12 @@ enum engine_node_state en_route_policies_run(struct
> engine_node *node,
> >                                               void *data);
> >  void *en_route_policies_init(struct engine_node *node OVS_UNUSED,
> >                               struct engine_arg *arg OVS_UNUSED);
> > +void en_routes_clear_tracked_data(void *data);
> >  void en_routes_cleanup(void *data);
> >  enum engine_input_handler_result
> >  routes_northd_change_handler(struct engine_node *node, void *data
> OVS_UNUSED);
> > +enum engine_input_handler_result
> > +routes_static_route_change_handler(struct engine_node *node, void
> *data);
> >  enum engine_node_state en_routes_run(struct engine_node *node, void
> *data);
> >  void *en_bfd_init(struct engine_node *node OVS_UNUSED,
> >                    struct engine_arg *arg OVS_UNUSED);
> > @@ -52,6 +55,10 @@ void *en_bfd_sync_init(struct engine_node *node
> OVS_UNUSED,
> >  enum engine_input_handler_result
> >  bfd_sync_northd_change_handler(struct engine_node *node,
> >                                 void *data OVS_UNUSED);
> > +enum engine_input_handler_result
> > +bfd_sync_routes_change_handler(struct engine_node *node,
> > +                               void *data OVS_UNUSED);
> > +
> >  enum engine_node_state en_bfd_sync_run(struct engine_node *node, void
> *data);
> >  void en_bfd_sync_cleanup(void *data OVS_UNUSED);
> >  void *en_ic_learned_svc_monitors_init(struct engine_node *node
> OVS_UNUSED,
> > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c
> > index a2b464411..c59b89ec0 100644
> > --- a/northd/inc-proc-northd.c
> > +++ b/northd/inc-proc-northd.c
> > @@ -75,7 +75,8 @@ static unixctl_cb_func chassis_features_list;
> >      NB_NODE(sampling_app) \
> >      NB_NODE(network_function) \
> >      NB_NODE(network_function_group) \
> > -    NB_NODE(logical_switch_port_health_check)
> > +    NB_NODE(logical_switch_port_health_check) \
> > +    NB_NODE(logical_router_static_route)
> >
> >      enum nb_engine_node {
> >  #define NB_NODE(NAME) NB_##NAME,
> > @@ -177,7 +178,7 @@ static ENGINE_NODE(lr_nat, CLEAR_TRACKED_DATA);
> >  static ENGINE_NODE(lr_stateful, CLEAR_TRACKED_DATA);
> >  static ENGINE_NODE(ls_stateful, CLEAR_TRACKED_DATA);
> >  static ENGINE_NODE(route_policies);
> > -static ENGINE_NODE(routes);
> > +static ENGINE_NODE(routes, CLEAR_TRACKED_DATA);
> >  static ENGINE_NODE(bfd);
> >  static ENGINE_NODE(bfd_sync, SB_WRITE);
> >  static ENGINE_NODE(ecmp_nexthop, SB_WRITE);
> > @@ -336,10 +337,12 @@ void inc_proc_northd_init(struct ovsdb_idl_loop
> *nb,
> >      engine_add_input(&en_routes, &en_bfd, NULL);
> >      engine_add_input(&en_routes, &en_northd,
> >                       routes_northd_change_handler);
> > +    engine_add_input(&en_routes, &en_nb_logical_router_static_route,
> > +                     routes_static_route_change_handler);
> >
> >      engine_add_input(&en_bfd_sync, &en_bfd, NULL);
> >      engine_add_input(&en_bfd_sync, &en_nb_bfd, NULL);
> > -    engine_add_input(&en_bfd_sync, &en_routes, NULL);
> > +    engine_add_input(&en_bfd_sync, &en_routes,
> bfd_sync_routes_change_handler);
> >      engine_add_input(&en_bfd_sync, &en_route_policies, NULL);
> >      engine_add_input(&en_bfd_sync, &en_northd,
> bfd_sync_northd_change_handler);
> >
> > @@ -375,7 +378,8 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> >      engine_add_input(&en_learned_route_sync, &en_northd,
> >                       learned_route_sync_northd_change_handler);
> >
> > -    engine_add_input(&en_group_ecmp_route, &en_routes, NULL);
> > +    engine_add_input(&en_group_ecmp_route, &en_routes,
> > +                     group_ecmp_route_routes_change_handler);
> >      engine_add_input(&en_group_ecmp_route, &en_learned_route_sync,
> >                       group_ecmp_route_learned_route_change_handler);
> >
> > @@ -394,9 +398,12 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb,
> >      engine_add_input(&en_lflow, &en_sb_logical_dp_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_routes, NULL);
> > -    /* XXX: The incremental processing only supports changes to learned
> routes.
> > -     * All other changes trigger a full recompute. */
> > +    /* 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.
> > +     * If en_routes falls back to a recompute, en_group_ecmp_route
> recomputes
> > +     * as well and so does en_lflow.  Hence a noop handler is enough
> here. */
> > +    engine_add_input(&en_lflow, &en_routes, engine_noop_handler);
> >      engine_add_input(&en_lflow, &en_group_ecmp_route,
> >                       lflow_group_ecmp_route_change_handler);
> >      engine_add_input(&en_lflow, &en_global_config,
> > diff --git a/northd/northd.c b/northd/northd.c
> > index f5aa5cca3..d3cb7f0b5 100644
> > --- a/northd/northd.c
> > +++ b/northd/northd.c
> > @@ -4522,6 +4522,7 @@ destroy_northd_data_tracked_changes(struct
> northd_data *nd)
> >      destroy_tracked_ovn_ports(&trk_changes->trk_lsps);
> >      destroy_tracked_lbs(&trk_changes->trk_lbs);
> >      hmapx_clear(&trk_changes->trk_nat_lrs);
> > +    hmapx_clear(&trk_changes->trk_lrs_routes);
> >      hmapx_clear(&trk_changes->ls_with_changed_lbs);
> >      hmapx_clear(&trk_changes->ls_with_changed_acls);
> >      hmapx_clear(&trk_changes->ls_with_changed_ipam);
> > @@ -4545,6 +4546,7 @@ init_northd_tracked_data(struct northd_data *nd)
> >      hmapx_init(&trk_data->trk_lbs.crupdated);
> >      hmapx_init(&trk_data->trk_lbs.deleted);
> >      hmapx_init(&trk_data->trk_nat_lrs);
> > +    hmapx_init(&trk_data->trk_lrs_routes);
> >      hmapx_init(&trk_data->ls_with_changed_lbs);
> >      hmapx_init(&trk_data->ls_with_changed_acls);
> >      hmapx_init(&trk_data->ls_with_changed_ipam);
> > @@ -4563,6 +4565,7 @@ destroy_northd_tracked_data(struct northd_data *nd)
> >      hmapx_destroy(&trk_data->trk_lbs.crupdated);
> >      hmapx_destroy(&trk_data->trk_lbs.deleted);
> >      hmapx_destroy(&trk_data->trk_nat_lrs);
> > +    hmapx_destroy(&trk_data->trk_lrs_routes);
> >      hmapx_destroy(&trk_data->ls_with_changed_lbs);
> >      hmapx_destroy(&trk_data->ls_with_changed_acls);
> >      hmapx_destroy(&trk_data->ls_with_changed_ipam);
> > @@ -5387,7 +5390,8 @@ lr_changes_can_be_handled(const struct
> nbrec_logical_router *lr)
> >          if (nbrec_logical_router_is_updated(lr, col)) {
> >              if (col == NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER
> >                  || col == NBREC_LOGICAL_ROUTER_COL_LOAD_BALANCER_GROUP
> > -                || col == NBREC_LOGICAL_ROUTER_COL_NAT) {
> > +                || col == NBREC_LOGICAL_ROUTER_COL_NAT
> > +                || col == NBREC_LOGICAL_ROUTER_COL_STATIC_ROUTES) {
> >                  continue;
> >              }
> >              return false;
> > @@ -5412,12 +5416,7 @@ lr_changes_can_be_handled(const struct
> nbrec_logical_router *lr)
> >              return false;
> >          }
> >      }
> > -    for (size_t i = 0; i < lr->n_static_routes; i++) {
> > -        if (nbrec_logical_router_static_route_row_get_seqno(
> > -            lr->static_routes[i], OVSDB_IDL_CHANGE_MODIFY) > 0) {
> > -            return false;
> > -        }
> > -    }
> > +
> >      return true;
> >  }
> >
> > @@ -5443,6 +5442,27 @@ is_lr_nats_changed(const struct
> nbrec_logical_router *nbr) {
> >              || is_lr_nats_seqno_changed(nbr));
> >  }
> >
> > +static bool
> > +is_lr_static_routes_seqno_changed(const struct nbrec_logical_router
> *nbr)
> > +{
> > +    for (size_t i = 0; i < nbr->n_static_routes; i++) {
> > +        if (nbrec_logical_router_static_route_row_get_seqno(
> > +                        nbr->static_routes[i], OVSDB_IDL_CHANGE_MODIFY)
> > 0) {
> > +            return true;
> > +        }
> > +    }
> > +
> > +    return false;
> > +}
> > +
> > +static bool
> > +is_lr_static_routes_changed(const struct nbrec_logical_router *nbr) {
> > +    return nbrec_logical_router_is_updated(nbr,
> > +
>  NBREC_LOGICAL_ROUTER_COL_STATIC_ROUTES)
> > +           || is_lr_static_routes_seqno_changed(nbr);
> > +}
> > +
> > +
>
> Nit: no need for the second newline.
>
> I agree


> >  /* Return true if changes are handled incrementally, false otherwise.
> >   *
> >   * Note: Changes to load balancer and load balancer groups associated
> with
> > @@ -5514,6 +5534,22 @@ northd_handle_lr_changes(const struct
> northd_input *ni,
> >
> >              hmapx_add(&nd->trk_data.trk_nat_lrs, od);
> >          }
> > +
> > +        /* Static Route was added or deleted. */
> > +        if (is_lr_static_routes_changed(changed_lr)) {
> > +            struct ovn_datapath *od = ovn_datapath_find_(
> > +                                    &nd->lr_datapaths.datapaths,
> > +                                    &changed_lr->header_.uuid);
> > +
> > +            if (!od) {
> > +                static struct vlog_rate_limit rl =
> VLOG_RATE_LIMIT_INIT(1, 1);
> > +                VLOG_WARN_RL(&rl, "Internal error: a tracked updated LR
> "
> > +                            "doesn't exist in lr_datapaths: "UUID_FMT,
> > +                            UUID_ARGS(&changed_lr->header_.uuid));
> > +                goto fail;
> > +            }
> > +            hmapx_add(&nd->trk_data.trk_lrs_routes, od);
> > +        }
> >      }
> >
> >      HMAPX_FOR_EACH (node, &ni->synced_lrs->deleted) {
> > @@ -5554,6 +5590,9 @@ northd_handle_lr_changes(const struct northd_input
> *ni,
> >      if (!hmapx_is_empty(&nd->trk_data.trk_nat_lrs)) {
> >          nd->trk_data.type |= NORTHD_TRACKED_LR_NATS;
> >      }
> > +    if (!hmapx_is_empty(&nd->trk_data.trk_lrs_routes)) {
> > +        nd->trk_data.type |= NORTHD_TRACKED_LR_ROUTES;
> > +    }
> >      if (!hmapx_is_empty(&nd->trk_data.trk_routers.crupdated) ||
> >          !hmapx_is_empty(&nd->trk_data.trk_routers.deleted)) {
> >          nd->trk_data.type |= NORTHD_TRACKED_ROUTERS;
> > @@ -12337,7 +12376,7 @@ parsed_route_add(const struct ovn_datapath *od,
> >      }
> >  }
> >
> > -static void
> > +struct parsed_route *
> >  parsed_routes_add_static(const struct ovn_datapath *od,
> >                           const struct hmap *lr_ports,
> >                           const struct nbrec_logical_router_static_route
> *route,
> > @@ -12358,8 +12397,9 @@ parsed_routes_add_static(const struct
> ovn_datapath *od,
> >                           UUID_FMT, route->nexthop,
> >                           UUID_ARGS(&route->header_.uuid));
> >              free(nexthop);
> > -            return;
> > +            return NULL;
> >          }
> > +
> >          if ((IN6_IS_ADDR_V4MAPPED(nexthop) && plen != 32) ||
> >              (!IN6_IS_ADDR_V4MAPPED(nexthop) && plen != 128)) {
> >              static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5,
> 1);
> > @@ -12367,7 +12407,7 @@ parsed_routes_add_static(const struct
> ovn_datapath *od,
> >                           UUID_FMT, route->nexthop,
> >                           UUID_ARGS(&route->header_.uuid));
> >              free(nexthop);
> > -            return;
> > +            return NULL;
> >          }
> >      }
> >
> > @@ -12379,7 +12419,7 @@ parsed_routes_add_static(const struct
> ovn_datapath *od,
> >                       UUID_FMT, route->ip_prefix,
> >                       UUID_ARGS(&route->header_.uuid));
> >          free(nexthop);
> > -        return;
> > +        return NULL;
> >      }
> >
> >      /* Verify that ip_prefix and nexthop are on the same network. */
> > @@ -12391,7 +12431,7 @@ parsed_routes_add_static(const struct
> ovn_datapath *od,
> >                                     : IN6_IS_ADDR_V4MAPPED(&prefix),
> >                                     &lrp_addr_s, &out_port)) {
> >          free(nexthop);
> > -        return;
> > +        return NULL;
> >      }
> >
> >      const struct nbrec_bfd *nb_bt = route->bfd;
> > @@ -12401,7 +12441,7 @@ parsed_routes_add_static(const struct
> ovn_datapath *od,
> >                                                    nb_bt->dst_ip);
> >          if (!bfd_e) {
> >              free(nexthop);
> > -            return;
> > +            return NULL;
> >          }
> >
> >          /* This static route is linked to an active bfd session. */
> > @@ -12418,10 +12458,9 @@ parsed_routes_add_static(const struct
> ovn_datapath *od,
> >              bfd_set_status(bfd_sr, "down");
> >          }
> >
> > -
> >          if (!strcmp(bfd_sr->status, "down")) {
> >              free(nexthop);
> > -            return;
> > +            return NULL;
> >          }
> >      }
> >
> > @@ -12465,12 +12504,17 @@ parsed_routes_add_static(const struct
> ovn_datapath *od,
> >      bool dynamic_routing_advertise = smap_get_bool(&route->options,
> >
>  "dynamic-routing-advertise",
> >                                                     true);
> > -    parsed_route_add(od, nexthop, &prefix, plen, is_discard_route,
> lrp_addr_s,
> > -                     out_port, route_table_id, is_src_route,
> > -                     ecmp_symmetric_reply, override_connected,
> > -                     &ecmp_selection_fields, source,
> dynamic_routing_advertise,
> > -                     &route->header_, NULL, routes);
> > +    struct parsed_route *pr = parsed_route_add(od, nexthop, &prefix,
> plen,
> > +                                               is_discard_route,
> lrp_addr_s,
> > +                                               out_port, route_table_id,
> > +                                               is_src_route,
> > +                                               ecmp_symmetric_reply,
> > +                                               override_connected,
> > +                                               &ecmp_selection_fields,
> source,
> > +
>  dynamic_routing_advertise,
> > +                                               &route->header_, NULL,
> routes);
> >      sset_destroy(&ecmp_selection_fields);
> > +    return pr;
> >  }
> >
> >  static void
> > @@ -16374,12 +16418,12 @@ build_lr_gateway_redirect_flows_for_nats(
> >   * In the common case where the Ethernet destination has been resolved,
> >   * this table outputs the packet (priority 0).  Otherwise, it composes
> >   * and sends an ARP/IPv6 NA request (priority 100). */
> > -static void
> > +void
> >  build_arp_request_flows_for_lrouter(
> > -        struct ovn_datapath *od, struct lflow_table *lflows,
> > -        struct ds *match, struct ds *actions,
> > -        const struct shash *meter_groups,
> > -        struct lflow_ref *lflow_ref)
> > +    const struct ovn_datapath *od, struct lflow_table *lflows,
> > +    struct ds *match, struct ds *actions,
> > +    const struct shash *meter_groups,
> > +    struct lflow_ref *lflow_ref)
> >  {
> >      ovs_assert(od->nbr);
> >      for (int i = 0; i < od->nbr->n_static_routes; i++) {
> > @@ -21076,6 +21120,9 @@ routes_init(struct routes_data *data)
> >      hmap_init(&data->parsed_routes);
> >      simap_init(&data->route_tables);
> >      hmap_init(&data->bfd_active_connections);
> > +    data->tracked = false;
> > +    hmapx_init(&data->trk_data.trk_deleted_parsed_route);
> > +    hmapx_init(&data->trk_data.trk_crupdated_parsed_route);
> >  }
> >
> >  void
> > @@ -21206,6 +21253,22 @@ routes_destroy(struct routes_data *data)
> >
> >      simap_destroy(&data->route_tables);
> >      __bfd_destroy(&data->bfd_active_connections);
> > +    data->tracked = false;
> > +    hmapx_destroy(&data->trk_data.trk_crupdated_parsed_route);
> > +    hmapx_destroy(&data->trk_data.trk_deleted_parsed_route);
> > +}
> > +
> > +void
> > +routes_clear_tracked(struct routes_data *data)
> > +{
> > +    data->tracked = false;
> > +    hmapx_clear(&data->trk_data.trk_crupdated_parsed_route);
> > +    struct hmapx_node *hmapx_node;
> > +    HMAPX_FOR_EACH_SAFE (hmapx_node,
> > +                         &data->trk_data.trk_deleted_parsed_route) {
> > +        parsed_route_free(hmapx_node->data);
> > +        hmapx_delete(&data->trk_data.trk_deleted_parsed_route,
> hmapx_node);
> > +    }
> >  }
> >
> >  void
> > diff --git a/northd/northd.h b/northd/northd.h
> > index 726a416e4..396deb660 100644
> > --- a/northd/northd.h
> > +++ b/northd/northd.h
> > @@ -159,6 +159,7 @@ enum northd_tracked_data_type {
> >      NORTHD_TRACKED_LS_ACLS  = (1 << 4),
> >      NORTHD_TRACKED_SWITCHES = (1 << 5),
> >      NORTHD_TRACKED_ROUTERS  = (1 << 6),
> > +    NORTHD_TRACKED_LR_ROUTES  = (1 << 7),
>
> Nit: we can't align this definition with the ones above anyway, let's
> not use the double space before "=".
>
> I agree

>  };
> >
> >  /* Track what's changed in the northd engine node.
> > @@ -176,6 +177,10 @@ struct northd_tracked_data {
> >       * hmapx node is 'struct ovn_datapath *'. */
> >      struct hmapx trk_nat_lrs;
> >
> > +    /* Tracked logical routers whose static routes have changed.
> > +     * hmapx node is 'struct ovn_datapath *'. */
> > +    struct hmapx trk_lrs_routes;
> > +
> >      /* Tracked logical switches whose load balancers have changed.
> >       * hmapx node is 'struct ovn_datapath *'. */
> >      struct hmapx ls_with_changed_lbs;
> > @@ -216,10 +221,17 @@ struct route_policy {
> >      uint32_t jump_chain_id;
> >  };
> >
> > +struct route_tracked_data {
> > +    struct hmapx trk_crupdated_parsed_route;
> > +    struct hmapx trk_deleted_parsed_route;
> > +};
> > +
> >  struct routes_data {
> >      struct hmap parsed_routes; /* Stores struct parsed_route. */
> >      struct simap route_tables;
> >      struct hmap bfd_active_connections;
> > +    bool tracked;
>
> Nit: I'd move this field at the end of the struct, it kind of reads better.
>
> I agree


> > +    struct route_tracked_data trk_data;
> >  };
> >
> >  struct route_policies_data {
> > @@ -886,6 +898,14 @@ struct parsed_route *parsed_route_add(
> >      const struct ovn_port *tracked_port,
> >      struct hmap *routes);
> >
> > +struct parsed_route *parsed_routes_add_static(
> > +    const struct ovn_datapath *od,
> > +    const struct hmap *lr_ports,
> > +    const struct nbrec_logical_router_static_route *route,
> > +    const struct hmap *bfd_connections,
> > +    struct hmap *routes, struct simap *route_tables,
> > +    struct hmap *bfd_active_connections);
> > +
> >  struct svc_monitors_map_data {
> >      const struct hmap *local_svc_monitors_map;
> >      const struct hmap *ic_learned_svc_monitors_map;
> > @@ -930,7 +950,7 @@ void build_parsed_routes(const struct ovn_datapath
> *, const struct hmap *,
> >  uint32_t get_route_table_id(struct simap *, const char *);
> >  void routes_init(struct routes_data *);
> >  void routes_destroy(struct routes_data *);
> > -
> > +void routes_clear_tracked(struct routes_data *);
>
> Missing empty line for readability.
>
> I agree


> >  void bfd_init(struct bfd_data *);
> >  void bfd_destroy(struct bfd_data *);
> >
> > @@ -956,6 +976,11 @@ void build_route_data_flows_for_lrouter(
> >      const struct ovn_datapath *od, struct lflow_table *lflows,
> >      const struct group_ecmp_datapath *route_node,
> >      const struct sset *bfd_ports);
> > +void build_arp_request_flows_for_lrouter(
> > +    const struct ovn_datapath *od, struct lflow_table *lflows,
> > +    struct ds *match, struct ds *actions,
> > +    const struct shash *meter_groups,
> > +    struct lflow_ref *lflow_ref);
> >
> >  bool lflow_handle_northd_lr_changes(struct ovsdb_idl_txn *ovnsh_txn,
> >                                       struct tracked_dps *,
> > @@ -1042,6 +1067,11 @@ northd_has_lr_nats_in_tracked_data(struct
> northd_tracked_data *trk_nd_changes)
> >  {
> >      return trk_nd_changes->type & NORTHD_TRACKED_LR_NATS;
> >  }
> > +static inline bool
> > +northd_has_lr_route_in_tracked_data(struct northd_tracked_data
> *trk_nd_changes)
> > +{
> > +    return trk_nd_changes->type & NORTHD_TRACKED_LR_ROUTES;
> > +}
> >
> >  static inline bool
> >  northd_has_ls_lbs_in_tracked_data(struct northd_tracked_data
> *trk_nd_changes)
> > diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/
> ovn-inc-proc-graph-dump.at
> > index 3750339d0..0278e4a57 100644
> > --- a/tests/ovn-inc-proc-graph-dump.at
> > +++ b/tests/ovn-inc-proc-graph-dump.at
> > @@ -151,16 +151,18 @@ digraph "Incremental-Processing-Engine" {
> >       bfd [[style=filled, shape=box, fillcolor=white, label="bfd"]];
> >       NB_bfd -> bfd [[label=""]];
> >       SB_bfd -> bfd [[label=""]];
> > +     NB_logical_router_static_route [[style=filled, shape=box,
> fillcolor=white, label="NB_logical_router_static_route"]];
> >       routes [[style=filled, shape=box, fillcolor=white,
> label="routes"]];
> >       bfd -> routes [[label=""]];
> >       northd -> routes [[label="routes_northd_change_handler"]];
> > +     NB_logical_router_static_route -> routes
> [[label="routes_static_route_change_handler"]];
> >       route_policies [[style=filled, shape=box, fillcolor=white,
> label="route_policies"]];
> >       bfd -> route_policies [[label=""]];
> >       northd -> route_policies
> [[label="route_policies_northd_change_handler"]];
> >       bfd_sync [[style=filled, shape=box, fillcolor=white,
> label="bfd_sync"]];
> >       bfd -> bfd_sync [[label=""]];
> >       NB_bfd -> bfd_sync [[label=""]];
> > -     routes -> bfd_sync [[label=""]];
> > +     routes -> bfd_sync [[label="bfd_sync_routes_change_handler"]];
> >       route_policies -> bfd_sync [[label=""]];
> >       northd -> bfd_sync [[label="bfd_sync_northd_change_handler"]];
> >       SB_learned_route [[style=filled, shape=box, fillcolor=white,
> label="SB_learned_route"]];
> > @@ -168,7 +170,7 @@ digraph "Incremental-Processing-Engine" {
> >       SB_learned_route -> learned_route_sync
> [[label="learned_route_sync_sb_learned_route_change_handler"]];
> >       northd -> learned_route_sync
> [[label="learned_route_sync_northd_change_handler"]];
> >       group_ecmp_route [[style=filled, shape=box, fillcolor=white,
> label="group_ecmp_route"]];
> > -     routes -> group_ecmp_route [[label=""]];
> > +     routes -> group_ecmp_route
> [[label="group_ecmp_route_routes_change_handler"]];
> >       learned_route_sync -> group_ecmp_route
> [[label="group_ecmp_route_learned_route_change_handler"]];
> >       ls_stateful [[style=filled, shape=box, fillcolor=white,
> label="ls_stateful"]];
> >       northd -> ls_stateful [[label="ls_stateful_northd_handler"]];
> > @@ -186,7 +188,7 @@ digraph "Incremental-Processing-Engine" {
> >       SB_logical_dp_group -> lflow [[label=""]];
> >       bfd_sync -> lflow [[label=""]];
> >       route_policies -> lflow [[label=""]];
> > -     routes -> lflow [[label=""]];
> > +     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"]];
> >       sampling_app -> lflow [[label=""]];
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 7f4a88d4e..376107fac 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -4383,7 +4383,7 @@ check ovn-nbctl --bfd=$uuid lr-route-add r0
> 100.0.0.0/8 192.168.1.2
> >  wait_column down bfd status logical_port=r0-sw1
> >  AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.1.2 | grep -q bfd],
> [0], [], [ignore])
> >
> > -check_engine_stats northd recompute nocompute
> > +check_engine_stats northd norecompute compute
> >  check_engine_stats bfd recompute nocompute
> >  check_engine_stats routes recompute nocompute
> >  check_engine_stats lflow recompute nocompute
> > @@ -4399,7 +4399,7 @@ check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8
> 192.168.5.2 r0-sw5
> >  wait_column down bfd status logical_port=r0-sw5
> >  AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q bfd],
> [0], [], [ignore])
> >
> > -check_engine_stats northd recompute nocompute
> > +check_engine_stats northd norecompute compute
> >  check_engine_stats bfd recompute nocompute
> >  check_engine_stats routes recompute nocompute
> >  check_engine_stats lflow recompute nocompute
> > @@ -4411,7 +4411,7 @@ check ovn-nbctl --bfd --policy=src-ip lr-route-add
> r0 192.168.6.1/32 192.168.10.
> >  wait_column down bfd status logical_port=r0-sw6
> >  AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.6.1 | grep -q bfd],
> [0], [], [ignore])
> >
> > -check_engine_stats northd recompute nocompute
> > +check_engine_stats northd norecompute compute
> >  check_engine_stats bfd recompute nocompute
> >  check_engine_stats route_policies recompute nocompute
> >  check_engine_stats lflow recompute nocompute
> > @@ -4446,7 +4446,7 @@ wait_column down bfd status logical_port=r0-sw8
> >  bfd_route_policy_uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw8)
> >  AT_CHECK([ovn-nbctl list logical_router_policy | grep -q
> $bfd_route_policy_uuid])
> >
> > -check_engine_stats northd recompute nocompute
> > +check_engine_stats northd recompute incremental
> >  check_engine_stats bfd recompute nocompute
> >  check_engine_stats routes recompute nocompute
> >  check_engine_stats lflow recompute nocompute
> > @@ -16847,12 +16847,12 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >
> >  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> >  check ovn-nbctl --wait=sb lr-route-add lr0 192.168.0.0/24 10.0.0.10
> > -check_engine_compute northd recompute
> > -check_engine_compute routes recompute
> > +check_engine_compute northd incremental
> > +check_engine_compute routes incremental
> >  check_engine_compute advertised_route_sync recompute
> > -check_engine_compute learned_route_sync recompute
> > -check_engine_compute group_ecmp_route recompute
> > -check_engine_compute lflow recompute
> > +check_engine_compute learned_route_sync incremental
> > +check_engine_compute group_ecmp_route incremental
> > +check_engine_compute lflow incremental
> >  CHECK_NO_CHANGE_AFTER_RECOMPUTE
> >
> >  check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > @@ -20962,3 +20962,120 @@ check_column "$global_svc_mon_mac"
> sb:Service_Monitor src_mac port=2
> >  OVN_CLEANUP_NORTHD
> >  AT_CLEANUP
> >  ])
> > +
> > +OVN_FOR_EACH_NORTHD_NO_HV([
> > +AT_SETUP([Static Route incremental processing])
> > +ovn_start
> > +
> > +check ovn-nbctl lr-add r0
> > +
> > +check ovn-nbctl --wait=sb lrp-add r0 r0-lrp1 00:00:00:00:00:01
> 192.168.1.1/24 fe80::1/64
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb lr-route-add r0 10.0.0.0/24 192.168.1.2
> > +
> > +check_engine_compute northd incremental
> > +check_engine_compute routes incremental
> > +check_engine_compute bfd_sync incremental
> > +check_engine_compute group_ecmp_route incremental
> > +check_engine_compute lflow incremental
> > +
> > +static_route_uuid=`ovn-nbctl --bare --columns _uuid find
> Logical_Router_Static_Route nexthop=192.168.1.2`
> > +AT_CHECK([test "$static_route_uuid" != ""])
> > +
> > +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 &&
> ip4.dst == 10.0.0.0/24"')
> > +AT_CHECK([test "$sr_lflow_uuid" != ""])
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb set logical_router_static_route
> $static_route_uuid nexthop=192.168.1.3
> > +check_engine_compute northd incremental
> > +check_engine_compute routes incremental
> > +check_engine_compute bfd_sync incremental
> > +check_engine_compute group_ecmp_route incremental
> > +check_engine_compute lflow incremental
> > +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 &&
> ip4.dst == 10.0.0.0/24"')
> > +AT_CHECK([test "$sr_lflow_uuid" != ""])
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb set logical_router_static_route
> $static_route_uuid ip_prefix=10.0.1.0/24
> > +check_engine_compute northd incremental
> > +check_engine_compute routes incremental
> > +check_engine_compute bfd_sync incremental
> > +check_engine_compute group_ecmp_route incremental
> > +check_engine_compute lflow incremental
> > +sr_lflow_uuid_old=$(fetch_column Logical_flow _uuid match='"reg7 == 0
> && ip4.dst == 10.0.0.0/24"')
> > +AT_CHECK([test "$sr_lflow_uuid_old" == ""])
> > +
> > +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 &&
> ip4.dst == 10.0.1.0/24"')
> > +AT_CHECK([test "$sr_lflow_uuid" != ""])
> > +
> > +check ovn-nbctl --wait=sb lrp-add r0 r0-lrp2 00:00:00:00:00:02
> 192.168.1.10/24
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb set logical_router_static_route
> $static_route_uuid output_port=r0-lrp2
> > +check_engine_compute northd incremental
> > +check_engine_compute routes incremental
> > +check_engine_compute bfd_sync incremental
> > +check_engine_compute group_ecmp_route incremental
> > +check_engine_compute lflow incremental
> > +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 &&
> ip4.dst == 10.0.1.0/24"')
> > +AT_CHECK([test "$sr_lflow_uuid" != ""])
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb set logical_router_static_route
> $static_route_uuid policy=src-ip
> > +check_engine_compute northd incremental
> > +check_engine_compute routes incremental
> > +check_engine_compute bfd_sync incremental
> > +check_engine_compute group_ecmp_route incremental
> > +check_engine_compute lflow incremental
> > +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 &&
> ip4.src == 10.0.1.0/24"')
> > +AT_CHECK([test "$sr_lflow_uuid" != ""])
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb set logical_router_static_route
> $static_route_uuid policy=dst-ip
> > +check ovn-nbctl --wait=sb set logical_router_static_route
> $static_route_uuid selection_fields="ip_proto,ip_src,ip_dst"
> > +check_engine_compute northd incremental
> > +check_engine_compute routes incremental
> > +check_engine_compute bfd_sync incremental
> > +check_engine_compute group_ecmp_route incremental
> > +check_engine_compute lflow incremental
> > +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 &&
> ip4.dst == 10.0.1.0/24"')
> > +AT_CHECK([test "$sr_lflow_uuid" != ""])
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb set logical_router_static_route
> $static_route_uuid options:ecmp_symmetric_reply=true
> > +check_engine_compute northd incremental
> > +check_engine_compute routes incremental
> > +check_engine_compute bfd_sync incremental
> > +check_engine_compute group_ecmp_route incremental
> > +check_engine_compute lflow incremental
> > +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 &&
> ip4.dst == 10.0.1.0/24"')
> > +AT_CHECK([test "$sr_lflow_uuid" != ""])
> > +
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl --wait=sb set logical_router_static_route
> $static_route_uuid nexthop=\"fe80::2\"
> > +check_engine_compute northd incremental
> > +check_engine_compute routes incremental
> > +check_engine_compute bfd_sync incremental
> > +check_engine_compute group_ecmp_route incremental
> > +check_engine_compute lflow incremental
> > +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 &&
> ip4.dst == 10.0.1.0/24"')
> > +AT_CHECK([test "$sr_lflow_uuid" != ""])
> > +sr_lflow_uuid_nexthop_ipv6=$(fetch_column Logical_flow _uuid
> match='"eth.dst == 00:00:00:00:00:00 && reg9[[9]] == 0 && xxreg0 ==
> fe80::2"')
> > +AT_CHECK([test "$sr_lflow_uuid_nexthop_ipv6" != ""])
> > +
> > +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
> > +check ovn-nbctl remove logical_router r0 static_routes
> $static_route_uuid
> > +check_engine_compute northd incremental
> > +check_engine_compute routes incremental
> > +check_engine_compute bfd_sync incremental
> > +check_engine_compute group_ecmp_route incremental
> > +check_engine_compute lflow incremental
> > +sr_lflow_uuid=$(fetch_column Logical_flow _uuid match='"reg7 == 0 &&
> ip4.src == 10.0.1.0/24"')
> > +AT_CHECK([test "$sr_lflow_uuid" == ""])
> > +
> > +CHECK_NO_CHANGE_AFTER_RECOMPUTE
> > +
> > +OVN_CLEANUP_NORTHD
> > +AT_CLEANUP
> > +])
>
> Aside from the relatively small issues above the rest looks good to me.
>
> Once we merge the patch I posted (to cleanup the nd_ns flows) could you
> please post a new version rebased on main?
>
> Yes, I can post  a rebased version.


> Thanks again for working on this!  It's something that we also really
> care about and it should improve OVN's performance significantly in some
> of our deployments.
>
>
You're welcome. In my scenario, it's really a performance issue with the
static routes.

Regards,
> Dumitru
>
>

Regards,
Lucas

-- 




_‘Esta mensagem é direcionada apenas para os endereços constantes no 
cabeçalho inicial. Se você não está listado nos endereços constantes no 
cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa 
mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão 
imediatamente anuladas e proibidas’._


* **‘Apesar do Magazine Luiza tomar 
todas as precauções razoáveis para assegurar que nenhum vírus esteja 
presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por 
quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.*



_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to