Hi Ales, thanks for reviewing the series. I made the changes suggested on this patch and pushed the first 8 patches to main. I am going to review your suggested changes on patch 9 and then create a v2 of this series that just has patches 9-17. I will save patch 18 for a future change.
On Wed, Jan 7, 2026 at 6:01 AM Ales Musil <[email protected]> wrote: > > > > 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
