On Tue, Oct 6, 2026 at 1:50 PM Mark Michelson via dev <
[email protected]> wrote:

> ECMP group IDs were assigned while building ECMP policy logical flows.
> This commit moves the assignment into en-route-policies.
>
> This will assist with the upcoming commits that will add incremental
> processing for routing policies.
>
> Signed-off-by: Mark Michelson <[email protected]>
> ---
>  northd/en-route-policies.c |  6 ++++++
>  northd/en-route-policies.h |  4 ++++
>  northd/northd.c            | 18 ++++++------------
>  3 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/northd/en-route-policies.c b/northd/en-route-policies.c
> index 0cdfb511d..dc33edb53 100644
> --- a/northd/en-route-policies.c
> +++ b/northd/en-route-policies.c
> @@ -195,6 +195,7 @@ build_route_policies(struct ovn_datapath *od,
>      }
>
>      size_t hash = uuid_hash(&od->key);
> +    uint16_t ecmp_group_id = 1;
>      for (int i = 0; i < od->nbr->n_policies; i++) {
>          const struct nbrec_logical_router_policy *rule =
> od->nbr->policies[i];
>
> @@ -323,12 +324,17 @@ build_route_policies(struct ovn_datapath *od,
>              }
>          }
>
> +        uint16_t group = 0;
> +        if (vector_len(&valid_nexthops) > 1) {
>

missing upper bound check


> +            group = ecmp_group_id++;
> +        }
>          struct route_policy *new_rp = xmalloc(sizeof *new_rp);
>          *new_rp = (struct route_policy) {
>              .rule = rule,
>              .valid_nexthops = vector_steal(&valid_nexthops),
>              .chain_id = chain_id,
>              .jump_chain_id = jump_chain_id,
> +            .ecmp_group_id = group,
>          };
>          hmap_insert(route_policies, &new_rp->key_node, hash);
>      }
> diff --git a/northd/en-route-policies.h b/northd/en-route-policies.h
> index 490ba09e7..dba0fc2b6 100644
> --- a/northd/en-route-policies.h
> +++ b/northd/en-route-policies.h
> @@ -43,6 +43,10 @@ struct route_policy {
>      struct vector valid_nexthops; /* struct route_policy_nexthop */
>      uint32_t chain_id;
>      uint32_t jump_chain_id;
> +    /* If the policy is ECMP, then this is the group ID for the policy.
> +     * If the policy is not ECMP, then this is 0.
> +     */
> +    uint32_t ecmp_group_id;
>

The field here is uint32_t but the counter is assigned from is uint16_t, I
think this is supposed to correspond to REG_ECMP_GROUP_ID
which is a 16 bit register. Could you change it all to uin16_t?


>  };
>
>  /* Global route policy data exported by en-route-policies. */
> diff --git a/northd/northd.c b/northd/northd.c
> index d3f80f898..15921d525 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -12272,7 +12272,6 @@ static void
>  build_ecmp_routing_policy_flows(struct lflow_table *lflows,
>                                  struct ovn_datapath *od,
>                                  struct route_policy *rp,
> -                                uint16_t ecmp_group_id,
>                                  struct lflow_ref *lflow_ref)
>  {
>      const struct nbrec_logical_router_policy *rule = rp->rule;
> @@ -12317,9 +12316,9 @@ build_ecmp_routing_policy_flows(struct lflow_table
> *lflows,
>                        is_ipv4);
>
>          ds_clear(&match);
> -        ds_put_format(&match, REG_ECMP_GROUP_ID" == %"PRIu16" && "
> +        ds_put_format(&match, REG_ECMP_GROUP_ID" == %"PRIu32" && "
>                        REG_ECMP_MEMBER_ID" == %"PRIuSIZE,
> -                      ecmp_group_id, i + 1);
> +                      rp->ecmp_group_id, i + 1);
>          ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY_ECMP, 100,
>                        ds_cstr(&match), ds_cstr(&actions), lflow_ref,
>                        WITH_HINT(&rule->header_));
> @@ -12327,8 +12326,8 @@ build_ecmp_routing_policy_flows(struct lflow_table
> *lflows,
>      }
>
>      ds_clear(&actions);
> -    ds_put_format(&actions, "%s = %"PRIu16
> -                  "; %s = select(", REG_ECMP_GROUP_ID, ecmp_group_id,
> +    ds_put_format(&actions, "%s = %"PRIu32
> +                  "; %s = select(", REG_ECMP_GROUP_ID, rp->ecmp_group_id,
>                    REG_ECMP_MEMBER_ID);
>
>      i = 0;
> @@ -15686,19 +15685,14 @@ build_ingress_policy_flows_for_lrouter(
>                                 lflow_ref);
>
>      /* Convert routing policies to flows. */
> -    uint16_t ecmp_group_id = 1;
>      struct route_policy *rp;
>      HMAP_FOR_EACH_WITH_HASH (rp, key_node, uuid_hash(&od->key),
>                               route_policies) {
>          const struct nbrec_logical_router_policy *rule = rp->rule;
> -        bool is_ecmp_reroute =
> -            (!strcmp(rule->action, "reroute") &&
> -             vector_len(&rp->valid_nexthops) > 1);
> +        bool is_ecmp_reroute = rp->ecmp_group_id != 0;
>
>          if (is_ecmp_reroute) {
> -            build_ecmp_routing_policy_flows(lflows, od, rp, ecmp_group_id,
> -                                            lflow_ref);
> -            ecmp_group_id++;
> +            build_ecmp_routing_policy_flows(lflows, od, rp, lflow_ref);
>          } else {
>              build_routing_policy_flow(lflows, od, rp, &rule->header_,
>                                        lflow_ref);
> --
> 2.55.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

Reply via email to