On Tue, Dec 16, 2025 at 7:44 PM Mark Michelson via dev <
[email protected]> wrote:

> The enum version of ovn_stage was too tightly coupled with the
> particular stages used by ovn-northd for logical switches and logical
> routers. Using a struct allows for more flexibility for other logical
> datapaths.
>
> Signed-off-by: Mark Michelson <[email protected]>
> ---
>

Hi Mark,

thank you for the patch. I have a couple of small comments down below.


>  northd/lflow-mgr.c | 48 ++++++++++++++++-------
>  northd/lflow-mgr.h |  7 +++-
>  northd/northd.c    | 98 ++++++++++++++++++++++++++++------------------
>  northd/northd.h    | 65 +++++++++++++-----------------
>  4 files changed, 126 insertions(+), 92 deletions(-)
>
> diff --git a/northd/lflow-mgr.c b/northd/lflow-mgr.c
> index f1273db19..9cb14fb5b 100644
> --- a/northd/lflow-mgr.c
> +++ b/northd/lflow-mgr.c
> @@ -35,14 +35,14 @@ struct ovn_lflow;
>
>  static void ovn_lflow_init(struct ovn_lflow *,
>                             const struct ovn_synced_datapath *dp,
> -                           size_t dp_bitmap_len, enum ovn_stage stage,
> +                           size_t dp_bitmap_len, const struct ovn_stage
> *stage,
>                             uint16_t priority, char *match,
>                             char *actions, char *io_port,
>                             char *ctrl_meter, char *stage_hint,
>                             const char *where, const char *flow_desc,
>                             struct uuid sbuuid);
>  static struct ovn_lflow *ovn_lflow_find(const struct hmap *lflows,
> -                                        enum ovn_stage stage,
> +                                        const struct ovn_stage *stage,
>                                          uint16_t priority, const char
> *match,
>                                          const char *actions,
>                                          const char *ctrl_meter, uint32_t
> hash);
> @@ -52,7 +52,7 @@ static char *ovn_lflow_hint(const struct ovsdb_idl_row
> *row);
>
>  static struct ovn_lflow *do_ovn_lflow_add(
>      struct lflow_table *, size_t dp_bitmap_len, uint32_t hash,
> -    enum ovn_stage stage, uint16_t priority, const char *match,
> +    const struct ovn_stage *stage, uint16_t priority, const char *match,
>      const char *actions, const char *io_port,
>      const char *ctrl_meter,
>      const struct ovsdb_idl_row *stage_hint,
> @@ -174,7 +174,7 @@ struct ovn_lflow {
>
>      const struct ovn_synced_datapath *dp;
>      struct dynamic_bitmap dpg_bitmap;
> -    enum ovn_stage stage;
> +    const struct ovn_stage *stage;
>      uint16_t priority;
>      char *match;
>      char *actions;
> @@ -355,9 +355,15 @@ lflow_table_sync_to_sb(struct lflow_table
> *lflow_table,
>          enum ovn_datapath_type dp_type
>              = ovn_datapath_get_type(logical_datapath_od);
>
> +        /* Since we should have weeded out sb_lflows with invalid
> +         * datapaths, we should only get valid datapath types here.
> +         */
> +        ovs_assert(dp_type < DP_MAX);
> +        struct ovn_stage stage = ovn_stage_build(dp_type, pipeline,
> +                                                 sbflow->table_id);
> +
>          lflow = ovn_lflow_find(
> -            lflows,
> -            ovn_stage_build(dp_type, pipeline, sbflow->table_id),
> +            lflows, &stage,
>              sbflow->priority, sbflow->match, sbflow->actions,
>              sbflow->controller_meter, sbflow->hash);
>          if (lflow) {
> @@ -732,7 +738,7 @@ void
>  lflow_table_add_lflow(struct lflow_table *lflow_table,
>                        const struct ovn_datapath *od,
>                        const unsigned long *dp_bitmap, size_t
> dp_bitmap_len,
> -                      enum ovn_stage stage, uint16_t priority,
> +                      const struct ovn_stage *stage, uint16_t priority,
>                        const char *match, const char *actions,
>                        const char *io_port, const char *ctrl_meter,
>                        const struct ovsdb_idl_row *stage_hint,
> @@ -801,6 +807,18 @@ lflow_table_add_lflow(struct lflow_table *lflow_table,
>      lflow_hash_unlock(hash_lock);
>  }
>
> +void
> +lflow_table_add_lflow_default_drop(struct lflow_table *lflow_table,
> +                                   const struct ovn_datapath *od,
> +                                   const struct ovn_stage *stage,
> +                                   const char *where,
> +                                   struct lflow_ref *lflow_ref)
> +{
> +    lflow_table_add_lflow(lflow_table, od, NULL, 0, stage, 0, "1",
> +                          debug_drop_action(), NULL, NULL, NULL,
> +                          where, NULL, lflow_ref);
> +}
> +
>  struct ovn_dp_group *
>  ovn_dp_group_get(struct hmap *dp_groups,
>                   const struct dynamic_bitmap *desired_bitmap,
> @@ -913,9 +931,9 @@ lflow_hash_lock_destroy(void)
>  static void
>  ovn_lflow_init(struct ovn_lflow *lflow,
>                 const struct ovn_synced_datapath *dp,
> -               size_t dp_bitmap_len, enum ovn_stage stage, uint16_t
> priority,
> -               char *match, char *actions, char *io_port, char
> *ctrl_meter,
> -               char *stage_hint, const char *where,
> +               size_t dp_bitmap_len, const struct ovn_stage *stage,
> +               uint16_t priority, char *match, char *actions, char
> *io_port,
> +               char *ctrl_meter, char *stage_hint, const char *where,
>                 const char *flow_desc, struct uuid sbuuid)
>  {
>      dynamic_bitmap_alloc(&lflow->dpg_bitmap, dp_bitmap_len);
> @@ -962,11 +980,11 @@ lflow_hash_unlock(struct ovs_mutex *hash_lock)
>  }
>
>  static bool
> -ovn_lflow_equal(const struct ovn_lflow *a, enum ovn_stage stage,
> +ovn_lflow_equal(const struct ovn_lflow *a, const struct ovn_stage *stage,
>                  uint16_t priority, const char *match,
>                  const char *actions, const char *ctrl_meter)
>  {
> -    return (a->stage == stage
> +    return (ovn_stage_equal(a->stage, stage)
>              && a->priority == priority
>              && !strcmp(a->match, match)
>              && !strcmp(a->actions, actions)
> @@ -975,7 +993,7 @@ ovn_lflow_equal(const struct ovn_lflow *a, enum
> ovn_stage stage,
>
>  static struct ovn_lflow *
>  ovn_lflow_find(const struct hmap *lflows,
> -               enum ovn_stage stage, uint16_t priority,
> +               const struct ovn_stage *stage, uint16_t priority,
>                 const char *match, const char *actions,
>                 const char *ctrl_meter, uint32_t hash)
>  {
> @@ -1018,8 +1036,8 @@ ovn_lflow_destroy(struct lflow_table *lflow_table,
> struct ovn_lflow *lflow)
>
>  static struct ovn_lflow *
>  do_ovn_lflow_add(struct lflow_table *lflow_table, size_t dp_bitmap_len,
> -                 uint32_t hash, enum ovn_stage stage, uint16_t priority,
> -                 const char *match, const char *actions,
> +                 uint32_t hash, const struct ovn_stage *stage,
> +                 uint16_t priority, const char *match, const char
> *actions,
>                   const char *io_port, const char *ctrl_meter,
>                   const struct ovsdb_idl_row *stage_hint,
>                   const char *where, const char *flow_desc)
> diff --git a/northd/lflow-mgr.h b/northd/lflow-mgr.h
> index 9276faeee..d4d98390b 100644
> --- a/northd/lflow-mgr.h
> +++ b/northd/lflow-mgr.h
> @@ -79,13 +79,18 @@ bool lflow_ref_sync_lflows(struct lflow_ref *,
>
>  void lflow_table_add_lflow(struct lflow_table *, const struct
> ovn_datapath *,
>                             const unsigned long *dp_bitmap,
> -                           size_t dp_bitmap_len, enum ovn_stage stage,
> +                           size_t dp_bitmap_len, const struct ovn_stage
> *stage,
>                             uint16_t priority, const char *match,
>                             const char *actions, const char *io_port,
>                             const char *ctrl_meter,
>                             const struct ovsdb_idl_row *stage_hint,
>                             const char *where, const char *flow_desc,
>                             struct lflow_ref *);
> +void lflow_table_add_lflow_default_drop(struct lflow_table *,
> +                                        const struct ovn_datapath *,
> +                                        const struct ovn_stage *stage,
> +                                        const char *where,
> +                                        struct lflow_ref *);
>
>  /* Adds a row with the specified contents to the Logical_Flow table. */
>  #define ovn_lflow_add_with_hint__(LFLOW_TABLE, OD, STAGE, PRIORITY,
> MATCH, \
> diff --git a/northd/northd.c b/northd/northd.c
> index 686bb28f0..4f733c851 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -371,18 +371,33 @@ static const char *reg_ct_state[] = {
>  #define ROUTE_PRIO_OFFSET_STATIC 4
>  #define ROUTE_PRIO_OFFSET_CONNECTED 6
>
> +/* ovn_stages used by northd for logical switches and logical routers.
> + * The first three components are combined to form the constant stage's
> + * struct name, e.g. S_SWITCH_IN_PORT_SEC_L2, S_ROUTER_OUT_DELIVERY.
> + */
> +#define PIPELINE_STAGE(DP_TYPE, PIPELINE, STAGE, TABLE, NAME)   \
> +    static const struct ovn_stage T_##DP_TYPE##_##PIPELINE##_##STAGE = {
>  \
> +        DP_##DP_TYPE, P_##PIPELINE, TABLE, NAME                       \
> +    };                                                                \
> +    static const struct ovn_stage *S_##DP_TYPE##_##PIPELINE##_##STAGE = \
> +        &T_##DP_TYPE##_##PIPELINE##_##STAGE;
> +    PIPELINE_STAGES
> +#undef PIPELINE_STAGE
> +
>  /* Returns the type of the datapath to which a flow with the given
> 'stage' may
>   * be added. */
>  enum ovn_datapath_type
> -ovn_stage_to_datapath_type(enum ovn_stage stage)
> +ovn_stage_to_datapath_type(const struct ovn_stage *stage)
>  {
> -    switch (stage) {
> -#define PIPELINE_STAGE(DP_TYPE, PIPELINE, STAGE, TABLE, NAME)       \
> -        case S_##DP_TYPE##_##PIPELINE##_##STAGE: return DP_##DP_TYPE;
> -    PIPELINE_STAGES
> -#undef PIPELINE_STAGE
> -    default: OVS_NOT_REACHED();
> -    }
> +    return stage->dp_type;
> +}
> +
> +bool
> +ovn_stage_equal(const struct ovn_stage *a, const struct ovn_stage *b)
> +{
> +    return a->dp_type == b->dp_type &&
> +           a->pipeline == b->pipeline &&
> +           a->table == b->table;
>  }
>
>  static uint32_t
> @@ -5798,7 +5813,7 @@ build_mirror_lflow(struct ovn_port *op,
>  {
>      struct ds match = DS_EMPTY_INITIALIZER;
>      struct ds action = DS_EMPTY_INITIALIZER;
> -    enum ovn_stage stage;
> +    const struct ovn_stage *stage;
>      const char *dir;
>      uint32_t priority = OVN_LPORT_MIRROR_OFFSET + rule->priority;
>
> @@ -5830,7 +5845,7 @@ build_mirror_pass_lflow(struct ovn_port *op,
>  {
>      struct ds match = DS_EMPTY_INITIALIZER;
>      struct ds action = DS_EMPTY_INITIALIZER;
> -    enum ovn_stage stage;
> +    const struct ovn_stage *stage;
>      const char *dir;
>
>      if (egress) {
> @@ -6105,8 +6120,9 @@ build_lswitch_output_port_sec_od(struct ovn_datapath
> *od,
>
>  static void
>  skip_port_from_conntrack(const struct ovn_datapath *od, struct ovn_port
> *op,
> -                         bool has_stateful_acl, enum ovn_stage in_stage,
> -                         enum ovn_stage out_stage, uint16_t priority,
> +                         bool has_stateful_acl,
> +                         const struct ovn_stage *in_stage,
> +                         const struct ovn_stage *out_stage, uint16_t
> priority,
>                           struct lflow_table *lflows,
>                           struct lflow_ref *lflow_ref)
>  {
> @@ -6560,13 +6576,13 @@ build_acl_hints(const struct ls_stateful_record
> *ls_stateful_rec,
>       * corresponding to all potential matches are set.
>       */
>
> -    enum ovn_stage stages[] = {
> +    const struct ovn_stage *stages[] = {
>          S_SWITCH_IN_ACL_HINT,
>          S_SWITCH_OUT_ACL_HINT,
>      };
>
>      for (size_t i = 0; i < ARRAY_SIZE(stages); i++) {
> -        enum ovn_stage stage = stages[i];
> +        const struct ovn_stage *stage = stages[i];
>
>          /* In any case, advance to the next stage. */
>          if (!ls_stateful_rec->has_acls && !ls_stateful_rec->has_lb_vip) {
> @@ -6852,7 +6868,7 @@ build_acl_sample_label_match(struct ds *match, const
> struct nbrec_acl *acl,
>  static void
>  build_acl_sample_new_flows(const struct ovn_datapath *od,
>                             struct lflow_table *lflows,
> -                           enum ovn_stage stage,
> +                           const struct ovn_stage *stage,
>                             struct ds *match, struct ds *actions,
>                             const struct nbrec_acl *acl,
>                             uint8_t sample_domain_id, bool stateful,
> @@ -6890,7 +6906,7 @@ build_acl_sample_new_flows(const struct ovn_datapath
> *od,
>  static void
>  build_acl_sample_est_orig_stateful_flows(const struct ovn_datapath *od,
>                                           struct lflow_table *lflows,
> -                                         enum ovn_stage stage,
> +                                         const struct ovn_stage *stage,
>                                           struct ds *match, struct ds
> *actions,
>                                           const struct nbrec_acl *acl,
>                                           uint8_t sample_domain_id,
> @@ -6924,7 +6940,7 @@ build_acl_sample_est_orig_stateful_flows(const
> struct ovn_datapath *od,
>  static void
>  build_acl_sample_est_rpl_stateful_flows(const struct ovn_datapath *od,
>                                          struct lflow_table *lflows,
> -                                        enum ovn_stage rpl_stage,
> +                                        const struct ovn_stage *rpl_stage,
>                                          struct ds *match, struct ds
> *actions,
>                                          const struct nbrec_acl *acl,
>                                          uint8_t sample_domain_id,
> @@ -6953,7 +6969,7 @@ build_acl_sample_est_rpl_stateful_flows(const struct
> ovn_datapath *od,
>  static void
>  build_acl_sample_est_stateful_flows(const struct ovn_datapath *od,
>                                      struct lflow_table *lflows,
> -                                    enum ovn_stage stage,
> +                                    const struct ovn_stage *stage,
>                                      struct ds *match, struct ds *actions,
>                                      const struct nbrec_acl *acl,
>                                      uint8_t sample_domain_id,
> @@ -6967,9 +6983,9 @@ build_acl_sample_est_stateful_flows(const struct
> ovn_datapath *od,
>
>      /* Install flows in the "opposite" pipeline direction to handle reply
>       * traffic on established connections. */
> -    enum ovn_stage rpl_stage = (stage == S_SWITCH_OUT_ACL_SAMPLE
> -                                ? S_SWITCH_IN_ACL_SAMPLE
> -                                : S_SWITCH_OUT_ACL_SAMPLE);
> +    const struct ovn_stage *rpl_stage = (stage == S_SWITCH_OUT_ACL_SAMPLE
> +                                         ? S_SWITCH_IN_ACL_SAMPLE
> +                                         : S_SWITCH_OUT_ACL_SAMPLE);
>      build_acl_sample_est_rpl_stateful_flows(od, lflows, rpl_stage,
>                                              match, actions,
>                                              acl, sample_domain_id,
> lflow_ref);
> @@ -6984,7 +7000,7 @@ static void build_acl_reject_action(struct ds
> *actions, bool is_ingress);
>  static void
>  build_acl_sample_generic_new_flows(const struct ovn_datapath *od,
>                                     struct lflow_table *lflows,
> -                                   enum ovn_stage stage,
> +                                   const struct ovn_stage *stage,
>                                     enum acl_observation_stage obs_stage,
>                                     struct ds *match, struct ds *actions,
>                                     const struct nbrec_sample_collector
> *coll,
> @@ -7032,7 +7048,7 @@ build_acl_sample_generic_new_flows(const struct
> ovn_datapath *od,
>  static void
>  build_acl_sample_generic_est_flows(const struct ovn_datapath *od,
>                                     struct lflow_table *lflows,
> -                                   enum ovn_stage stage,
> +                                   const struct ovn_stage *stage,
>                                     enum acl_observation_stage obs_stage,
>                                     struct ds *match, struct ds *actions,
>                                     const struct nbrec_sample_collector
> *coll,
> @@ -7065,9 +7081,9 @@ build_acl_sample_generic_est_flows(const struct
> ovn_datapath *od,
>      ovn_lflow_add(lflows, od, stage, 1000, ds_cstr(match),
>                    ds_cstr(actions), lflow_ref);
>
> -    enum ovn_stage rpl_stage = (stage == S_SWITCH_OUT_ACL_SAMPLE
> -                                ? S_SWITCH_IN_ACL_SAMPLE
> -                                : S_SWITCH_OUT_ACL_SAMPLE);
> +    const struct ovn_stage *rpl_stage = (stage == S_SWITCH_OUT_ACL_SAMPLE
> +                                         ? S_SWITCH_IN_ACL_SAMPLE
> +                                         : S_SWITCH_OUT_ACL_SAMPLE);
>
>      ds_truncate(match, match_len);
>      ds_put_format(match, "ct.rpl && ct_mark.obs_collector_id == %"PRIu8,
> @@ -7120,7 +7136,7 @@ build_acl_sample_flows(const struct
> ls_stateful_record *ls_stateful_rec,
>      }
>
>      bool ingress = !strcmp(acl->direction, "from-lport") ? true : false;
> -    enum ovn_stage stage;
> +    const struct ovn_stage *stage;
>      enum acl_observation_stage obs_stage;
>
>      if (ingress && smap_get_bool(&acl->options, "apply-after-lb", false))
> {
> @@ -7193,7 +7209,7 @@ consider_acl(struct lflow_table *lflows, const
> struct ovn_datapath *od,
>               const struct sbrec_acl_id_table *sbrec_acl_id_table)
>  {
>      bool ingress = !strcmp(acl->direction, "from-lport") ? true :false;
> -    enum ovn_stage stage;
> +    const struct ovn_stage *stage;
>      enum acl_observation_stage obs_stage;
>
>      if (ingress && smap_get_bool(&acl->options, "apply-after-lb", false))
> {
> @@ -7507,13 +7523,13 @@ build_acl_action_lflows(const struct
> ls_stateful_record *ls_stateful_rec,
>                          struct ds *actions,
>                          struct lflow_ref *lflow_ref)
>  {
> -    enum ovn_stage stages [] = {
> +    const struct ovn_stage *stages [] = {
>          S_SWITCH_IN_ACL_ACTION,
>          S_SWITCH_IN_ACL_AFTER_LB_ACTION,
>          S_SWITCH_OUT_ACL_ACTION,
>      };
>
> -    enum ovn_stage eval_stages[] = {
> +    const struct ovn_stage *eval_stages[] = {
>          S_SWITCH_IN_ACL_EVAL,
>          S_SWITCH_IN_ACL_AFTER_LB_EVAL,
>          S_SWITCH_OUT_ACL_EVAL,
> @@ -7532,7 +7548,7 @@ build_acl_action_lflows(const struct
> ls_stateful_record *ls_stateful_rec,
>
>      size_t verdict_len = actions->length;
>      for (size_t i = 0; i < ARRAY_SIZE(stages); i++) {
> -        enum ovn_stage stage = stages[i];
> +        const struct ovn_stage *stage = stages[i];
>          if (max_acl_tiers[i]) {
>              ds_put_cstr(actions, REG_ACL_TIER " = 0; ");
>          }
> @@ -7623,7 +7639,7 @@ build_acl_log_related_flows(const struct
> ovn_datapath *od,
>      /* Related/reply flows need to be set on the opposite pipeline
>       * from where the ACL itself is set.
>       */
> -    enum ovn_stage log_related_stage = ingress ?
> +    const struct ovn_stage *log_related_stage = ingress ?
>          S_SWITCH_OUT_ACL_EVAL :
>          S_SWITCH_IN_ACL_EVAL;
>      ds_clear(match);
> @@ -7985,7 +8001,9 @@ build_qos(struct ovn_datapath *od, struct
> lflow_table *lflows,
>      for (size_t i = 0; i < od->nbs->n_qos_rules; i++) {
>          struct nbrec_qos *qos = od->nbs->qos_rules[i];
>          bool ingress = !strcmp(qos->direction, "from-lport") ? true
> :false;
> -        enum ovn_stage stage = ingress ? S_SWITCH_IN_QOS :
> S_SWITCH_OUT_QOS;
> +        const struct ovn_stage *stage = ingress
> +            ? S_SWITCH_IN_QOS
> +            : S_SWITCH_OUT_QOS;
>          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>          int64_t rate = 0;
>          int64_t burst = 0;
> @@ -13295,7 +13313,9 @@ lrouter_nat_add_ext_ip_match(const struct
> ovn_datapath *od,
>                        allowed_ext_ips->name);
>      } else if (exempted_ext_ips) {
>          struct ds match_exempt = DS_EMPTY_INITIALIZER;
> -        enum ovn_stage stage = is_src ? S_ROUTER_IN_DNAT :
> S_ROUTER_OUT_SNAT;
> +        const struct ovn_stage *stage = is_src
> +            ? S_ROUTER_IN_DNAT
> +            : S_ROUTER_OUT_SNAT;
>
>          /* Priority of logical flows corresponding to exempted_ext_ips is
>           * +2 of the corresponding regular NAT rule.
> @@ -13553,7 +13573,7 @@ build_lrouter_port_nat_arp_nd_flow(struct ovn_port
> *op,
>  static void
>  build_lrouter_drop_own_dest(struct ovn_port *op,
>                              const struct lr_stateful_record
> *lr_stateful_rec,
> -                            enum ovn_stage stage,
> +                            const struct ovn_stage *stage,
>                              uint16_t priority, bool drop_snat_ip,
>                              struct lflow_table *lflows,
>                              struct lflow_ref *lflow_ref)
> @@ -13970,7 +13990,7 @@ build_gateway_get_l2_hdr_size(struct ovn_port *op)
>   */
>  static void OVS_PRINTF_FORMAT(10, 11)
>  build_gateway_mtu_flow(struct lflow_table *lflows, struct ovn_port *op,
> -                       enum ovn_stage stage, uint16_t prio_low,
> +                       const struct ovn_stage *stage, uint16_t prio_low,
>                         uint16_t prio_high, struct ds *match,
>                         struct ds *actions, const struct ovsdb_idl_row
> *hint,
>                         struct lflow_ref *lflow_ref,
> @@ -15421,7 +15441,7 @@ create_icmp_need_frag_lflow(const struct ovn_port
> *op, int mtu,
>                              struct ds *actions, struct ds *match,
>                              const char *meter, struct lflow_table *lflows,
>                              struct lflow_ref *lflow_ref,
> -                            enum ovn_stage stage, uint16_t priority,
> +                            const struct ovn_stage *stage, uint16_t
> priority,
>                              bool is_ipv6, const char *extra_match,
>                              const char *extra_action)
>  {
> @@ -15459,7 +15479,7 @@ static void
>  build_icmperr_pkt_big_flows(struct ovn_port *op, int mtu,
>                              struct lflow_table *lflows,
>                              const struct shash *meter_groups, struct ds
> *match,
> -                            struct ds *actions, enum ovn_stage stage,
> +                            struct ds *actions, const struct ovn_stage
> *stage,
>                              struct ovn_port *outport,
>                              const char *ct_state_match,
>                              struct lflow_ref *lflow_ref)
> @@ -18491,7 +18511,7 @@ consider_network_function(struct lflow_table
> *lflows,
>          return;
>      }
>
> -    enum ovn_stage fwd_stage, rev_stage;
> +    const struct ovn_stage *fwd_stage, *rev_stage;
>      struct ovn_port *redirect_port = NULL;
>      struct ovn_port *reverse_redirect_port = NULL;
>      if (ingress) {
> diff --git a/northd/northd.h b/northd/northd.h
> index 94950b822..6e66ef57e 100644
> --- a/northd/northd.h
> +++ b/northd/northd.h
> @@ -511,21 +511,6 @@ ovn_datapath_is_stale(const struct ovn_datapath *od)
>  };
>
>  /* Pipeline stages. */
> -/* Returns an "enum ovn_stage" built from the arguments.
> - *
> - * (It's better to use ovn_stage_build() for type-safety reasons, but
> inline
> - * functions can't be used in enums or switch cases.) */
> -#define OVN_STAGE_BUILD(DP_TYPE, PIPELINE, TABLE) \
> -    (((DP_TYPE) << 9) | ((PIPELINE) << 8) | (TABLE))
> -
> -/* A stage within an OVN logical switch or router.
> - *
> - * An "enum ovn_stage" indicates whether the stage is part of a logical
> switch
> - * or router, whether the stage is part of the ingress or egress
> pipeline, and
> - * the table within that pipeline.  The first three components are
> combined to
> - * form the stage's full name, e.g. S_SWITCH_IN_PORT_SEC_L2,
> - * S_ROUTER_OUT_DELIVERY. */
> -enum ovn_stage {
>  #define PIPELINE_STAGES
>  \
>      /* Logical switch ingress stages. */
> \
>      PIPELINE_STAGE(SWITCH, IN,  CHECK_PORT_SEC, 0,
> "ls_in_check_port_sec")   \
> @@ -625,14 +610,23 @@ enum ovn_stage {
>      PIPELINE_STAGE(ROUTER, OUT, EGR_LOOP,           5,
> "lr_out_egr_loop")    \
>      PIPELINE_STAGE(ROUTER, OUT, DELIVERY,           6, "lr_out_delivery")
>
> -#define PIPELINE_STAGE(DP_TYPE, PIPELINE, STAGE, TABLE, NAME)   \
> -    S_##DP_TYPE##_##PIPELINE##_##STAGE                          \
> -        = OVN_STAGE_BUILD(DP_##DP_TYPE, P_##PIPELINE, TABLE),
> -    PIPELINE_STAGES
> -#undef PIPELINE_STAGE
> +/* A stage within an OVN logical switch or router.
> + *
> + * A "struct ovn_stage" indicates whether the stage is part of a logical
> switch
> + * or router, whether the stage is part of the ingress or egress
> pipeline, and
> + * the table within that pipeline.
> + */
> +struct ovn_stage {
> +    enum ovn_datapath_type dp_type;
> +    enum ovn_pipeline pipeline;
> +    int table;
>

Let's use uint8_t.


> +    const char *name;
>  };
>
> -enum ovn_datapath_type ovn_stage_to_datapath_type(enum ovn_stage stage);
> +bool ovn_stage_equal(const struct ovn_stage *a, const struct ovn_stage
> *b);
>

I suppose this could be static inline too.


> +
> +enum ovn_datapath_type ovn_stage_to_datapath_type(
> +    const struct ovn_stage *stage);
>
>
>  /* Returns 'od''s datapath type. */
> @@ -642,46 +636,43 @@ ovn_datapath_get_type(const struct ovn_datapath *od)
>      return od->nbs ? DP_SWITCH : DP_ROUTER;
>  }
>
> -/* Returns an "enum ovn_stage" built from the arguments. */
> -static inline enum ovn_stage
> +/* Returns a "struct ovn_stage" built from the arguments.
> + * The stage will not have a proper name, but so far, the only cases
> + * where stages are built from components, a name is not necessary.
> + */
> +static inline struct ovn_stage
>  ovn_stage_build(enum ovn_datapath_type dp_type, enum ovn_pipeline
> pipeline,
>                  uint8_t table)
>  {
> -    return OVN_STAGE_BUILD(dp_type, pipeline, table);
> +    return (struct ovn_stage) {dp_type, pipeline, table, "<unknown>"};
>  }
>
>  /* Returns the pipeline to which 'stage' belongs. */
>  static inline enum ovn_pipeline
> -ovn_stage_get_pipeline(enum ovn_stage stage)
> +ovn_stage_get_pipeline(const struct ovn_stage *stage)
>  {
> -    return (stage >> 8) & 1;
> +    return stage->pipeline;
>  }
>
>  /* Returns the pipeline name to which 'stage' belongs. */
>  static inline const char *
> -ovn_stage_get_pipeline_name(enum ovn_stage stage)
> +ovn_stage_get_pipeline_name(const struct ovn_stage *stage)
>  {
>      return ovn_stage_get_pipeline(stage) == P_IN ? "ingress" : "egress";
>  }
>
>  /* Returns the table to which 'stage' belongs. */
>  static inline uint8_t
> -ovn_stage_get_table(enum ovn_stage stage)
> +ovn_stage_get_table(const struct ovn_stage *stage)
>  {
> -    return stage & 0xff;
> +    return stage->table;
>  }
>
>  /* Returns a string name for 'stage'. */
>  static inline const char *
> -ovn_stage_to_str(enum ovn_stage stage)
> +ovn_stage_to_str(const struct ovn_stage *stage)
>  {
> -    switch (stage) {
> -#define PIPELINE_STAGE(DP_TYPE, PIPELINE, STAGE, TABLE, NAME)       \
> -        case S_##DP_TYPE##_##PIPELINE##_##STAGE: return NAME;
> -    PIPELINE_STAGES
> -#undef PIPELINE_STAGE
> -        default: return "<unknown>";
> -    }
> +    return stage->name;
>  }
>
>  /* A logical switch port or logical router port.
> --
> 2.51.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
With that addressed:

Acked-by: Ales Musil <[email protected]>

It also seems like CI had some infra issues, let's try it again:
Recheck-request: github-robot-_Build_and_Test
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to