On 2/10/26 2:58 PM, Ilya Maximets wrote: > On 2/10/26 2:35 AM, MJ Ponsonby wrote: >> This both bumps the OVS submodule and fixes various breaking changes >> introduced in OVS version 3.7.0. >> >> - OVS commit dc14e92 updates route_table_handle_msg_callback to have a >> 3rd parameter 'uint32_t table', which allows us to remove table_id from >> the route_msg_handle_data struct provided we update handle_route_msg >> accordingly. >> - OVS commit 3900653 changes FLOW_N_REGS from 16 to 32, to avoid version >> incompatibility errors we set OVN_FLOW_N_REGS_SUPPORTED which is 16, >> and is checked to be within FLOW_N_REGS. We then check that the >> various MFF_LOG declarations fall within this still. >> >> This should allow us to sync up with OVS again in preparation for 26.03. >> >> Signed-off-by: MJ Ponsonby <[email protected]> >> ---
Hi MJ, Ilya, Thanks for the patch! I took care of the small nits and pushed the patch to main. Regards, Dumitru >> controller/pinctrl.c | 2 +- >> controller/route-exchange-netlink.c | 9 +++---- >> include/ovn/logical-fields.h | 42 +++++++++++++++++++++++++++++ >> lib/actions.c | 5 +--- >> ovs | 2 +- >> utilities/ovn-trace.c | 2 +- >> 6 files changed, 50 insertions(+), 12 deletions(-) > > Thanks for the update! > > The change looks good to me. There is a couple nits below that can > probably be addressed on commit. Otherwise: > > Acked-by: Ilya Maximets <[email protected]> > >> >> diff --git a/controller/pinctrl.c b/controller/pinctrl.c >> index 7baf6b488..08940cd8b 100644 >> --- a/controller/pinctrl.c >> +++ b/controller/pinctrl.c >> @@ -6368,7 +6368,7 @@ static void >> reload_metadata(struct ofpbuf *ofpacts, const struct match *md) >> { >> enum mf_field_id md_fields[] = { >> -#if FLOW_N_REGS == 16 >> +#if OVN_FLOW_N_REGS_SUPPORTED == 16 >> MFF_REG0, >> MFF_REG1, >> MFF_REG2, >> diff --git a/controller/route-exchange-netlink.c >> b/controller/route-exchange-netlink.c >> index 7ab0690f2..1fb41008c 100644 >> --- a/controller/route-exchange-netlink.c >> +++ b/controller/route-exchange-netlink.c >> @@ -205,17 +205,18 @@ struct route_msg_handle_data { >> struct vector *learned_routes; >> struct vector *stale_routes; >> const struct hmap *routes; >> - uint32_t table_id; /* requested table id. */ >> }; >> >> static void >> -handle_route_msg(const struct route_table_msg *msg, void *data) >> +handle_route_msg(const struct route_table_msg *msg, >> + void *data, >> + uint32_t table_id) >> { >> struct route_msg_handle_data *handle_data = data; >> const struct route_data *rd = &msg->rd; >> struct advertise_route_entry *ar; >> >> - if (handle_data->table_id != rd->rta_table_id) { >> + if (table_id != rd->rta_table_id) { >> /* We do not have the NLM_F_DUMP_FILTERED info here, so check if the >> * reported table_id matches the requested one. >> */ >> @@ -342,7 +343,6 @@ re_nl_sync_routes(uint32_t table_id, const struct hmap >> *routes, >> .learned_routes = learned_routes, >> .stale_routes = &stale_routes, >> .db = db, >> - .table_id = table_id, >> }; >> route_table_dump_one_table(table_id, handle_route_msg, &data); >> >> @@ -388,7 +388,6 @@ re_nl_cleanup_routes(uint32_t table_id) >> .routes_to_advertise = NULL, >> .learned_routes = NULL, >> .stale_routes = &stale_routes, >> - .table_id = table_id, >> }; >> route_table_dump_one_table(table_id, handle_route_msg, &data); >> >> diff --git a/include/ovn/logical-fields.h b/include/ovn/logical-fields.h >> index f0d34196a..00e6f1bd7 100644 >> --- a/include/ovn/logical-fields.h >> +++ b/include/ovn/logical-fields.h >> @@ -26,53 +26,95 @@ enum ovn_controller_event { >> OVN_EVENT_MAX, >> }; >> >> +/* OVS versions 2.6 to 3.6 support up to 16 32-bit registers. These can be >> + * used unconditionally. Registers above MFF_REG15, MFF_XREG7 and >> MFF_XXREG3 >> + * are only supported in newer versions and must not be used without prior >> + * support detection. */ >> +#define OVN_FLOW_N_REGS_SUPPORTED 16 >> +BUILD_ASSERT_DECL(FLOW_N_REGS == 32); >> +BUILD_ASSERT_DECL(FLOW_N_REGS >= OVN_FLOW_N_REGS_SUPPORTED); >> + >> +#define CHECK_REG(NAME) \ >> + BUILD_ASSERT_DECL( \ >> + MFF_LOG_##NAME < MFF_REG0 + OVN_FLOW_N_REGS_SUPPORTED) >> + >> +#define CHECK_XXREG(NAME) \ >> + BUILD_ASSERT_DECL( \ >> + MFF_LOG_##NAME < MFF_XXREG0 + OVN_FLOW_N_REGS_SUPPORTED / 4) >> + >> + >> /* Logical fields. >> * >> * These values are documented in ovn-architecture(7), please update the >> * documentation if you change any of them. */ >> #define MFF_LOG_DATAPATH MFF_METADATA /* Logical datapath (64 bits). */ >> #define MFF_LOG_FLAGS MFF_REG10 /* One of MLF_* (32 bits). */ >> +CHECK_REG(FLAGS); >> #define MFF_LOG_DNAT_ZONE MFF_REG11 /* conntrack dnat zone for gateway >> router >> * (32 bits). */ >> +CHECK_REG(DNAT_ZONE); >> #define MFF_LOG_SNAT_ZONE MFF_REG12 /* conntrack snat zone for gateway >> router >> * (32 bits). */ >> +CHECK_REG(SNAT_ZONE); >> #define MFF_LOG_CT_ZONE MFF_REG13 /* Logical conntrack zone for lports >> * (0..15 of the 32 bits). */ >> +CHECK_REG(CT_ZONE); >> #define MFF_LOG_ENCAP_ID MFF_REG13 /* Encap ID for lports >> * (16..31 of the 32 bits). */ >> +CHECK_REG(ENCAP_ID); >> #define MFF_LOG_INPORT MFF_REG14 /* Logical input port (32 bits). */ >> +CHECK_REG(INPORT); >> #define MFF_LOG_OUTPORT MFF_REG15 /* Logical output port (32 bits). */ >> +CHECK_REG(OUTPORT); >> #define MFF_LOG_TUN_OFPORT MFF_REG5 /* 16..31 of the 32 bits */ >> +CHECK_REG(TUN_OFPORT); >> >> /* Logical registers. >> * >> * Make sure these don't overlap with the logical fields! */ >> #define MFF_LOG_REG0 MFF_REG0 >> +CHECK_REG(REG0); >> #define MFF_LOG_LB_ORIG_DIP_IPV4 MFF_REG4 >> +CHECK_REG(LB_ORIG_DIP_IPV4); >> #define MFF_LOG_LB_ORIG_TP_DPORT MFF_REG2 >> +CHECK_REG(LB_ORIG_TP_DPORT); >> >> #define MFF_LOG_XXREG0 MFF_XXREG0 >> +CHECK_XXREG(XXREG0); >> #define MFF_LOG_LB_ORIG_DIP_IPV6 MFF_XXREG1 >> +CHECK_XXREG(LB_ORIG_DIP_IPV6); >> >> #define MFF_N_LOG_REGS 10 >> >> +BUILD_ASSERT_DECL(MFF_LOG_REG0 + MFF_N_LOG_REGS \ > > nit: We're not in a macro definition here, no need for the line continuation. > >> + <= MFF_REG0 + OVN_FLOW_N_REGS_SUPPORTED); >> + >> #define MFF_LOG_LB_AFF_MATCH_IP4_ADDR MFF_REG4 >> +CHECK_REG(LB_AFF_MATCH_IP4_ADDR); >> #define MFF_LOG_LB_AFF_MATCH_IP6_ADDR MFF_XXREG1 >> +CHECK_XXREG(LB_AFF_MATCH_IP6_ADDR); >> #define MFF_LOG_LB_AFF_MATCH_PORT MFF_REG2 >> +CHECK_REG(LB_AFF_MATCH_PORT); >> >> #define MFF_LOG_RESULT_REG MFF_XXREG1 >> +CHECK_XXREG(RESULT_REG); >> >> #define MFF_LOG_CT_SAVED_STATE MFF_REG4 >> +CHECK_REG(CT_SAVED_STATE); >> >> #define MFF_LOG_REMOTE_OUTPORT MFF_REG1 /* Logical remote output >> * port (32 bits). */ >> +CHECK_REG(REMOTE_OUTPORT); >> >> /* Logical registers that are needed for backwards >> * compatibility with older northd versions. >> * XXX: All of them can be removed in 26.09. */ >> #define MFF_LOG_LB_ORIG_DIP_IPV4_OLD MFF_REG1 >> +CHECK_REG(LB_ORIG_DIP_IPV4_OLD); >> #define MFF_LOG_LB_AFF_MATCH_PORT_OLD MFF_REG8 >> +CHECK_REG(LB_AFF_MATCH_PORT_OLD); >> #define MFF_LOG_LB_AFF_MATCH_LS_IP6_ADDR_OLD MFF_XXREG0 >> +CHECK_XXREG(LB_AFF_MATCH_LS_IP6_ADDR_OLD); >> >> /* Maximum number of networks supported by 4-bit flags.network_id. */ >> #define OVN_MAX_NETWORK_ID \ >> diff --git a/lib/actions.c b/lib/actions.c >> index 6d1230e1e..2d6df5f81 100644 >> --- a/lib/actions.c >> +++ b/lib/actions.c >> @@ -1486,10 +1486,7 @@ encode_ct_lb(const struct ovnact_ct_lb *cl, >> ds_put_format(&ds, ",fields(%s)", cl->hash_fields); >> } >> >> - BUILD_ASSERT(MFF_LOG_CT_ZONE >= MFF_REG0); >> - BUILD_ASSERT(MFF_LOG_CT_ZONE < MFF_REG0 + FLOW_N_REGS); >> - BUILD_ASSERT(MFF_LOG_DNAT_ZONE >= MFF_REG0); >> - BUILD_ASSERT(MFF_LOG_DNAT_ZONE < MFF_REG0 + FLOW_N_REGS); > > nit: I'd still add a comment here, e.g.: > > /* Make sure that all the used registers are within the NXM_NX class. */ > >> + BUILD_ASSERT(OVN_FLOW_N_REGS_SUPPORTED == 16); >> >> size_t n_active_backends = 0; >> for (size_t bucket_id = 0; bucket_id < cl->n_dsts; bucket_id++) { >> diff --git a/ovs b/ovs >> index 5be77f1ec..1d57509ef 160000 >> --- a/ovs >> +++ b/ovs >> @@ -1 +1 @@ >> -Subproject commit 5be77f1ecc521174af188ce9a9372c89db0d22e8 >> +Subproject commit 1d57509ef1cc9ff7fbd6e450a5bb82e91480959f >> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c >> index 8708a50c3..46cc65fa0 100644 >> --- a/utilities/ovn-trace.c >> +++ b/utilities/ovn-trace.c >> @@ -1696,7 +1696,7 @@ execute_output(const struct ovntrace_datapath *dp, >> struct flow *uflow, >> } >> >> struct flow egress_uflow = *uflow; >> - for (int i = 0; i < FLOW_N_REGS; i++) { >> + for (int i = 0; i < OVN_FLOW_N_REGS_SUPPORTED; i++) { >> if (i != MFF_LOG_INPORT - MFF_REG0 && >> i != MFF_LOG_OUTPORT - MFF_REG0) { >> egress_uflow.regs[i] = 0; > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
