On 12/19/25 2:19 PM, Dumitru Ceara wrote:
> OVN relies heavily in its OpenFlow rules on the registers that are
> provided by OVS. With the OVN feature set increasing we are slowly but
> surely running out of register space.
>
> The suggestion to increase the number of OVS registers again (last time
> this happened was with 847b8b027af4 ("Increase number of registers to
> 16.")), keeps being brought up on the mailing list, most recently in [0]
> and [1]. The alternative would be to work around the register space
> issue in OVN by using approaches like pushing/popping registers to the
> stack. But that would complicate the OVN pipeline even further.
>
> Instead this patch increases the number of general purpose 32-bit
> registers to 32. That implies that the number of 64-bit registers
> (xreg) also increases to 16 and the number of 128-bit registers (xxreg)
> increases to 8.
>
> There's currently not enough space in the NXM_NX class for the new 16
> registers so we use the NXOXM_ET class instead. According to commit
> 508a933809f8 ("nx-match: Add support for experimenter OXM.") this is the
> right class to use for field extensions.
>
> NOTE: Commit 85be6684f8af ("meta-flow: Fix the NXM_NX_* names of xxreg2
> and xxreg3.") had added placeholders for future expansion for four more
> xxregs (xxreg4-7) in the NXM_NX class. However, since now the
> new reg16-31 registers are added to the NXOXM_ET class, we also add
> the corresponding xxreg4-7 to it.
>
> [0] https://mail.openvswitch.org/pipermail/ovs-dev/2025-October/426703.html
> [1] https://mail.openvswitch.org/pipermail/ovs-dev/2025-December/428768.html
>
> Suggested-by: Ilya Maximets <[email protected]>
> Reported-at: https://issues.redhat.com/browse/FDP-2843
> Signed-off-by: Dumitru Ceara <[email protected]>
Hi, Dumitru. Thanks for working on this! I agree with the change in
general, it's much needed for OVN if we want to keep adding new features.
The patch looks good for the most part, with a few missing bits and nits
mentioned below.
nit: Maybe the subject line should specify 'openflow:' or 'meta-flow:'
area. All the other stuff in the patch follows the primary openflow
change.
> ---
> NEWS | 1 +
> include/openflow/nicira-ext.h | 2 +-
> include/openvswitch/flow.h | 8 +-
> include/openvswitch/meta-flow.h | 125 ++++++++++++++----
> lib/dpif-netdev-extract-avx512.c | 2 +-
> lib/flow.c | 20 +--
> lib/flow.h | 2 +-
> lib/match.c | 2 +-
> lib/meta-flow.xml | 39 +++++-
> lib/nx-match.c | 2 +-
> lib/odp-util.c | 6 +-
> lib/odp-util.h | 2 +-
> lib/ofp-match.c | 2 +-
> ofproto/ofproto-dpif-rid.h | 2 +-
> ofproto/ofproto-dpif-xlate.c | 2 +-
> .../ovs_build_helpers/extract_ofp_fields.py | 2 +-
> tests/ofproto.at | 4 +-
> tests/ovs-ofctl.at | 56 ++++++++
> 18 files changed, 221 insertions(+), 58 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index ca7e66ab7b45..42e59626cb8e 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -30,6 +30,7 @@ Post-v3.6.0
> * The OOT kernel module: was deprecated in v2.15; was last present in
> the v2.17 release; and is no longer present in any supported release
> since v2.17 went EOL when v3.5 was released.
> + - Increase number of registers to 32.
nit: Might be better to write NEWS entries in past tense, like what was
done, not what should be done. I know, we follow this pattern for some
things, but it's not the best, IMO.
>
>
> v3.6.0 - 18 Aug 2025
> diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
> index 959845ce6d74..56d4bf603b01 100644
> --- a/include/openflow/nicira-ext.h
> +++ b/include/openflow/nicira-ext.h
> @@ -537,7 +537,7 @@ OFP_ASSERT(sizeof(struct nx_async_config) == 24);
> */
>
> /* Number of registers allocated NXM field IDs. */
> -#define NXM_NX_MAX_REGS 16
> +#define NXM_NX_MAX_REGS 32
>
> /* Bits in the value of NXM_NX_IP_FRAG. */
> #define NX_IP_FRAG_ANY (1 << 0) /* Is this a fragment? */
> diff --git a/include/openvswitch/flow.h b/include/openvswitch/flow.h
> index df10cf579e2f..86edbb85fcc0 100644
> --- a/include/openvswitch/flow.h
> +++ b/include/openvswitch/flow.h
> @@ -27,10 +27,10 @@ extern "C" {
> /* This sequence number should be incremented whenever anything involving
> flows
> * or the wildcarding of flows changes. This will cause build assertion
> * failures in places which likely need to be updated. */
> -#define FLOW_WC_SEQ 42
> +#define FLOW_WC_SEQ 43
>
> /* Number of Open vSwitch extension 32-bit registers. */
> -#define FLOW_N_REGS 16
> +#define FLOW_N_REGS 32
> BUILD_ASSERT_DECL(FLOW_N_REGS <= NXM_NX_MAX_REGS);
> BUILD_ASSERT_DECL(FLOW_N_REGS % 4 == 0); /* Handle xxregs. */
>
> @@ -166,8 +166,8 @@ BUILD_ASSERT_DECL(sizeof(struct ovs_key_nsh) %
> sizeof(uint64_t) == 0);
>
> /* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
> BUILD_ASSERT_DECL(offsetof(struct flow, igmp_group_ip4) + sizeof(uint32_t)
> - == sizeof(struct flow_tnl) + sizeof(struct ovs_key_nsh) +
> 300
> - && FLOW_WC_SEQ == 42);
> + == sizeof(struct flow_tnl) + sizeof(struct ovs_key_nsh) +
> 364
> + && FLOW_WC_SEQ == 43);
>
> /* Incremental points at which flow classification may be performed in
> * segments.
> diff --git a/include/openvswitch/meta-flow.h b/include/openvswitch/meta-flow.h
> index 875f122c5f55..5fc60b6be0e6 100644
> --- a/include/openvswitch/meta-flow.h
> +++ b/include/openvswitch/meta-flow.h
> @@ -211,7 +211,7 @@ struct ofputil_tlv_table_mod;
> * field.
> *
> * Finally, a few "register" fields have very similar names and purposes,
> - * e.g. MFF_REG0 through MFF_REG15. For these, the comments may be merged
> + * e.g. MFF_REG0 through MFF_REG31. For these, the comments may be merged
> * together using <N> as a metasyntactic variable for the numeric suffix.
> * Lines in the comment that are specific to one of the particular fields by
> * writing, e.g. <1>, to consider that line only for e.g. MFF_REG1.
> @@ -982,7 +982,7 @@ enum OVS_PACKED_ENUM mf_field_id {
> */
> MFF_CT_TP_DST,
>
> -#if FLOW_N_REGS == 16
> +#if FLOW_N_REGS == 32
> /* "reg<N>".
> *
> * Nicira extension scratch pad register with initial value 0.
> @@ -1026,11 +1026,55 @@ enum OVS_PACKED_ENUM mf_field_id {
> MFF_REG13,
> MFF_REG14,
> MFF_REG15,
> +
> + /* "reg<N>".
> + *
> + * Nicira extension scratch pad register with initial value 0.
> + *
> + * Type: be32.
> + * Maskable: bitwise.
> + * Formatting: hexadecimal.
> + * Prerequisites: none.
> + * Access: read/write.
> + * NXM: none.
> + * OXM: NXOXM_ET_REG16(17) since v3.7. <16>
> + * OXM: NXOXM_ET_REG17(18) since v3.7. <17>
> + * OXM: NXOXM_ET_REG18(19) since v3.7. <18>
> + * OXM: NXOXM_ET_REG19(20) since v3.7. <19>
> + * OXM: NXOXM_ET_REG20(21) since v3.7. <20>
> + * OXM: NXOXM_ET_REG21(22) since v3.7. <21>
> + * OXM: NXOXM_ET_REG22(23) since v3.7. <22>
> + * OXM: NXOXM_ET_REG23(24) since v3.7. <23>
> + * OXM: NXOXM_ET_REG24(25) since v3.7. <24>
> + * OXM: NXOXM_ET_REG25(26) since v3.7. <25>
> + * OXM: NXOXM_ET_REG26(27) since v3.7. <26>
> + * OXM: NXOXM_ET_REG27(28) since v3.7. <27>
> + * OXM: NXOXM_ET_REG28(29) since v3.7. <28>
> + * OXM: NXOXM_ET_REG29(30) since v3.7. <29>
> + * OXM: NXOXM_ET_REG30(31) since v3.7. <30>
> + * OXM: NXOXM_ET_REG31(32) since v3.7. <31>
> + */
> + MFF_REG16,
> + MFF_REG17,
> + MFF_REG18,
> + MFF_REG19,
> + MFF_REG20,
> + MFF_REG21,
> + MFF_REG22,
> + MFF_REG23,
> + MFF_REG24,
> + MFF_REG25,
> + MFF_REG26,
> + MFF_REG27,
> + MFF_REG28,
> + MFF_REG29,
> + MFF_REG30,
> + MFF_REG31,
> #else
> #error "Need to update MFF_REG* to match FLOW_N_REGS"
> #endif
>
> -#if FLOW_N_XREGS == 8
> +#if FLOW_N_XREGS == 16
> /* "xreg<N>".
> *
> * OpenFlow 1.5 ``extended register".
> @@ -1051,11 +1095,19 @@ enum OVS_PACKED_ENUM mf_field_id {
> MFF_XREG5,
> MFF_XREG6,
> MFF_XREG7,
> + MFF_XREG8,
> + MFF_XREG9,
> + MFF_XREG10,
> + MFF_XREG11,
> + MFF_XREG12,
> + MFF_XREG13,
> + MFF_XREG14,
> + MFF_XREG15,
The comment here needs an update for the OVS version, otherwise they
get documented as supported in v2.4+.
> #else
> #error "Need to update MFF_REG* to match FLOW_N_XREGS"
> #endif
>
> -#if FLOW_N_XXREGS == 4
> +#if FLOW_N_XXREGS == 8
> /* "xxreg<N>".
> *
> * ``extended-extended register".
> @@ -1065,20 +1117,36 @@ enum OVS_PACKED_ENUM mf_field_id {
> * Formatting: hexadecimal.
> * Prerequisites: none.
> * Access: read/write.
> - * NXM: NXM_NX_XXREG0(111) since v2.6. <0>
> - * NXM: NXM_NX_XXREG1(112) since v2.6. <1>
> - * NXM: NXM_NX_XXREG2(113) since v2.6. <2>
> - * NXM: NXM_NX_XXREG3(114) since v2.6. <3>
> - * NXM: NXM_NX_XXREG4(115) since vX.Y. <4>
> - * NXM: NXM_NX_XXREG5(116) since vX.Y. <5>
> - * NXM: NXM_NX_XXREG6(117) since vX.Y. <6>
> - * NXM: NXM_NX_XXREG7(118) since vX.Y. <7>
> + * NXM: NXM_NX_XXREG0(111) since v2.6. <0>
> + * NXM: NXM_NX_XXREG1(112) since v2.6. <1>
> + * NXM: NXM_NX_XXREG2(113) since v2.6. <2>
> + * NXM: NXM_NX_XXREG3(114) since v2.6. <3>
> * OXM: none.
> */
> MFF_XXREG0,
> MFF_XXREG1,
> MFF_XXREG2,
> MFF_XXREG3,
> +
> + /* "xxreg<N>".
> + *
> + * ``extended-extended register".
> + *
> + * Type: be128.
> + * Maskable: bitwise.
> + * Formatting: hexadecimal.
> + * Prerequisites: none.
> + * Access: read/write.
> + * NXM: none.
> + * OXM: NXOXM_ET_XXREG4(33) since v3.7. <4>
> + * OXM: NXOXM_ET_XXREG5(34) since v3.7. <5>
> + * OXM: NXOXM_ET_XXREG6(35) since v3.7. <6>
> + * OXM: NXOXM_ET_XXREG7(36) since v3.7. <7>
> + */
> + MFF_XXREG4,
> + MFF_XXREG5,
> + MFF_XXREG6,
> + MFF_XXREG7,
> #else
> #error "Need to update MFF_REG* to match FLOW_N_XXREGS"
> #endif
> @@ -1977,31 +2045,38 @@ struct mf_bitmap mf_bitmap_not(struct mf_bitmap);
>
> /* Use this macro as CASE_MFF_REGS: in a switch statement to choose all of
> the
> * MFF_REGn cases. */
> -#if FLOW_N_REGS ==16
> -#define CASE_MFF_REGS \
> - case MFF_REG0: case MFF_REG1: case MFF_REG2: case MFF_REG3: \
> - case MFF_REG4: case MFF_REG5: case MFF_REG6: case MFF_REG7: \
> - case MFF_REG8: case MFF_REG9: case MFF_REG10: case MFF_REG11: \
> - case MFF_REG12: case MFF_REG13: case MFF_REG14: case MFF_REG15
> +#if FLOW_N_REGS == 32
> +#define CASE_MFF_REGS \
> + case MFF_REG0: case MFF_REG1: case MFF_REG2: case MFF_REG3: \
> + case MFF_REG4: case MFF_REG5: case MFF_REG6: case MFF_REG7: \
> + case MFF_REG8: case MFF_REG9: case MFF_REG10: case MFF_REG11: \
> + case MFF_REG12: case MFF_REG13: case MFF_REG14: case MFF_REG15: \
> + case MFF_REG16: case MFF_REG17: case MFF_REG18: case MFF_REG19: \
> + case MFF_REG20: case MFF_REG21: case MFF_REG22: case MFF_REG23: \
> + case MFF_REG24: case MFF_REG25: case MFF_REG26: case MFF_REG27: \
> + case MFF_REG28: case MFF_REG29: case MFF_REG30: case MFF_REG31
> #else
> #error "Need to update CASE_MFF_REGS to match FLOW_N_REGS"
> #endif
>
> /* Use this macro as CASE_MFF_XREGS: in a switch statement to choose all of
> the
> * MFF_REGn cases. */
> -#if FLOW_N_XREGS == 8
> -#define CASE_MFF_XREGS \
> - case MFF_XREG0: case MFF_XREG1: case MFF_XREG2: case MFF_XREG3: \
> - case MFF_XREG4: case MFF_XREG5: case MFF_XREG6: case MFF_XREG7
> +#if FLOW_N_XREGS == 16
> +#define CASE_MFF_XREGS \
> + case MFF_XREG0: case MFF_XREG1: case MFF_XREG2: case MFF_XREG3: \
> + case MFF_XREG4: case MFF_XREG5: case MFF_XREG6: case MFF_XREG7: \
> + case MFF_XREG8: case MFF_XREG9: case MFF_XREG10: case MFF_XREG11: \
> + case MFF_XREG12: case MFF_XREG13: case MFF_XREG14: case MFF_XREG15
> #else
> #error "Need to update CASE_MFF_XREGS to match FLOW_N_XREGS"
> #endif
>
> /* Use this macro as CASE_MFF_XXREGS: in a switch statement to choose
> * all of the MFF_REGn cases. */
> -#if FLOW_N_XXREGS == 4
> -#define CASE_MFF_XXREGS \
> - case MFF_XXREG0: case MFF_XXREG1: case MFF_XXREG2: case MFF_XXREG3
> +#if FLOW_N_XXREGS == 8
> +#define CASE_MFF_XXREGS \
> + case MFF_XXREG0: case MFF_XXREG1: case MFF_XXREG2: case MFF_XXREG3: \
> + case MFF_XXREG4: case MFF_XXREG5: case MFF_XXREG6: case MFF_XXREG7
> #else
> #error "Need to update CASE_MFF_XXREGS to match FLOW_N_XXREGS"
> #endif
> diff --git a/lib/dpif-netdev-extract-avx512.c
> b/lib/dpif-netdev-extract-avx512.c
> index 90bf53a7afb7..8741bbf296c9 100644
> --- a/lib/dpif-netdev-extract-avx512.c
> +++ b/lib/dpif-netdev-extract-avx512.c
> @@ -421,7 +421,7 @@ BUILD_ASSERT_DECL((OFFSETOFEND(struct dp_packet, l4_ofs)
> BUILD_ASSERT_DECL(FLOWMAP_UNITS == 2);
>
> /* Ensure the miniflow-struct ABI is the expected version. */
> -BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> +BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> /* If the above build assert happens, this means that you might need to make
> * some modifications to the AVX512 miniflow extractor code. In general, the
> diff --git a/lib/flow.c b/lib/flow.c
> index b522f7f116ca..a59a25c468a1 100644
> --- a/lib/flow.c
> +++ b/lib/flow.c
> @@ -136,7 +136,7 @@ struct mf_ctx {
> * away. Some GCC versions gave warnings on ALWAYS_INLINE, so these are
> * defined as macros. */
>
> -#if (FLOW_WC_SEQ != 42)
> +#if (FLOW_WC_SEQ != 43)
> #define MINIFLOW_ASSERT(X) ovs_assert(X)
> BUILD_MESSAGE("FLOW_WC_SEQ changed: miniflow_extract() will have runtime "
> "assertions enabled. Consider updating FLOW_WC_SEQ after "
> @@ -796,7 +796,7 @@ void
> miniflow_extract(struct dp_packet *packet, struct miniflow *dst)
> {
> /* Add code to this function (or its callees) to extract new fields. */
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> const struct pkt_metadata *md = &packet->md;
> const void *data = dp_packet_data(packet);
> @@ -1313,7 +1313,7 @@ flow_get_metadata(const struct flow *flow, struct match
> *flow_metadata)
> {
> int i;
>
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> match_init_catchall(flow_metadata);
> if (flow->tunnel.tun_id != htonll(0)) {
> @@ -1899,7 +1899,7 @@ flow_wildcards_init_for_packet(struct flow_wildcards
> *wc,
> memset(&wc->masks, 0x0, sizeof wc->masks);
>
> /* Update this function whenever struct flow changes. */
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> if (flow_tnl_dst_is_set(&flow->tunnel)) {
> if (flow->tunnel.flags & FLOW_TNL_F_KEY) {
> @@ -2052,7 +2052,7 @@ void
> flow_wc_map(const struct flow *flow, struct flowmap *map)
> {
> /* Update this function whenever struct flow changes. */
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> flowmap_init(map);
>
> @@ -2155,7 +2155,7 @@ void
> flow_wildcards_clear_non_packet_fields(struct flow_wildcards *wc)
> {
> /* Update this function whenever struct flow changes. */
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> memset(&wc->masks.metadata, 0, sizeof wc->masks.metadata);
> memset(&wc->masks.regs, 0, sizeof wc->masks.regs);
> @@ -2299,7 +2299,7 @@ flow_wildcards_set_xxreg_mask(struct flow_wildcards
> *wc, int idx,
> uint32_t
> miniflow_hash_5tuple(const struct miniflow *flow, uint32_t basis)
> {
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> uint32_t hash = basis;
>
> if (flow) {
> @@ -2346,7 +2346,7 @@ ASSERT_SEQUENTIAL(ipv6_src, ipv6_dst);
> uint32_t
> flow_hash_5tuple(const struct flow *flow, uint32_t basis)
> {
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
> uint32_t hash = basis;
>
> if (flow) {
> @@ -3024,7 +3024,7 @@ flow_push_mpls(struct flow *flow, int n, ovs_be16
> mpls_eth_type,
>
> if (clear_flow_L3) {
> /* Clear all L3 and L4 fields and dp_hash. */
> - BUILD_ASSERT(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT(FLOW_WC_SEQ == 43);
> memset((char *) flow + FLOW_SEGMENT_2_ENDS_AT, 0,
> sizeof(struct flow) - FLOW_SEGMENT_2_ENDS_AT);
> flow->dp_hash = 0;
> @@ -3324,7 +3324,7 @@ flow_compose(struct dp_packet *p, const struct flow
> *flow,
> /* Add code to this function (or its callees) for emitting new fields or
> * protocols. (This isn't essential, so it can be skipped for initial
> * testing.) */
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> uint32_t pseudo_hdr_csum;
> size_t l4_len;
> diff --git a/lib/flow.h b/lib/flow.h
> index 60ec4b0d7809..422cd6d48a14 100644
> --- a/lib/flow.h
> +++ b/lib/flow.h
> @@ -975,7 +975,7 @@ static inline void
> pkt_metadata_from_flow(struct pkt_metadata *md, const struct flow *flow)
> {
> /* Update this function whenever struct flow changes. */
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> md->recirc_id = flow->recirc_id;
> md->dp_hash = flow->dp_hash;
> diff --git a/lib/match.c b/lib/match.c
> index 9b7e06e0c7f8..4f8b9e903681 100644
> --- a/lib/match.c
> +++ b/lib/match.c
> @@ -1462,7 +1462,7 @@ match_format(const struct match *match,
> bool is_megaflow = false;
> int i;
>
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> if (priority != OFP_DEFAULT_PRIORITY) {
> ds_put_format(s, "%spriority=%s%d,",
> diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml
> index 5c57ab08ff18..c996cddad341 100644
> --- a/lib/meta-flow.xml
> +++ b/lib/meta-flow.xml
> @@ -2905,7 +2905,8 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
> This is the first of several Open vSwitch registers, all of which have
> the same properties. Open vSwitch 1.1 introduced registers 0, 1, 2,
> and
> 3, version 1.3 added register 4, version 1.7 added registers 5, 6, and
> 7,
> - and version 2.6 added registers 8 through 15.
> + version 2.6 added registers 8 through 15 and version 3.7 added
> registers
> + 16 through 31.
> </field>
> <!-- XXX series -->
> <field id="MFF_REG1" title="Register 1" hidden="yes"/>
> @@ -2923,6 +2924,22 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
> <field id="MFF_REG13" title="Register 13" hidden="yes"/>
> <field id="MFF_REG14" title="Register 14" hidden="yes"/>
> <field id="MFF_REG15" title="Register 15" hidden="yes"/>
> + <field id="MFF_REG16" title="Register 16" hidden="yes"/>
> + <field id="MFF_REG17" title="Register 17" hidden="yes"/>
> + <field id="MFF_REG18" title="Register 18" hidden="yes"/>
> + <field id="MFF_REG19" title="Register 19" hidden="yes"/>
> + <field id="MFF_REG20" title="Register 20" hidden="yes"/>
> + <field id="MFF_REG21" title="Register 21" hidden="yes"/>
> + <field id="MFF_REG22" title="Register 22" hidden="yes"/>
> + <field id="MFF_REG23" title="Register 23" hidden="yes"/>
> + <field id="MFF_REG24" title="Register 24" hidden="yes"/>
> + <field id="MFF_REG25" title="Register 25" hidden="yes"/>
> + <field id="MFF_REG26" title="Register 26" hidden="yes"/>
> + <field id="MFF_REG27" title="Register 27" hidden="yes"/>
> + <field id="MFF_REG28" title="Register 28" hidden="yes"/>
> + <field id="MFF_REG29" title="Register 29" hidden="yes"/>
> + <field id="MFF_REG30" title="Register 30" hidden="yes"/>
> + <field id="MFF_REG31" title="Register 31" hidden="yes"/>
The ovs-fields man page shows the REG0 as an exmaple, we probably need to
mention that new registers have a different class somehow, either by showing
another example, or at least just stating what is the other class and for
which registers it is getting used. Same for the xxregs.
>
> <field id="MFF_XREG0" title="Extended Register 0">
> <p>
> @@ -2931,8 +2948,8 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
> vSwitch already had 32-bit registers by that name, so Open vSwitch
> uses
> the name ``extended registers'' in an attempt to reduce confusion.
> The
> standard allows for up to 128 registers, each 64 bits wide, but Open
> - vSwitch only implements 4 (in versions 2.4 and 2.5) or 8 (in version
> - 2.6 and later).
> + vSwitch only implements 4 (in versions 2.4 and 2.5), 8 (in version
> + 2.6 and later) or 16 (in version 3.7 and later).
> </p>
>
> <p>
> @@ -2961,6 +2978,14 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
> <field id="MFF_XREG5" title="Extended Register 5" hidden="yes"/>
> <field id="MFF_XREG6" title="Extended Register 6" hidden="yes"/>
> <field id="MFF_XREG7" title="Extended Register 7" hidden="yes"/>
> + <field id="MFF_XREG8" title="Extended Register 8" hidden="yes"/>
> + <field id="MFF_XREG9" title="Extended Register 9" hidden="yes"/>
> + <field id="MFF_XREG10" title="Extended Register 10" hidden="yes"/>
> + <field id="MFF_XREG11" title="Extended Register 11" hidden="yes"/>
> + <field id="MFF_XREG12" title="Extended Register 12" hidden="yes"/>
> + <field id="MFF_XREG13" title="Extended Register 13" hidden="yes"/>
> + <field id="MFF_XREG14" title="Extended Register 14" hidden="yes"/>
> + <field id="MFF_XREG15" title="Extended Register 15" hidden="yes"/>
>
> <field id="MFF_XXREG0" title="Double-Extended Register 0">
> <p>
> @@ -2970,7 +2995,9 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
> through <code>reg3</code>, with <code>reg0</code> supplying the
> most-significant bits of <code>xxreg0</code> and <code>reg3</code>
> the
> least-significant. <code>xxreg1</code> similarly overlays
> - <code>reg4</code> through <code>reg7</code>, and so on.
> + <code>reg4</code> through <code>reg7</code>, and so on. Open
> + vSwitch 4 128-bit registers (in versions 2.6 and later) and 8
> + (in version 3.7 and later).
> </p>
> </field>
>
> @@ -2978,6 +3005,10 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
> <field id="MFF_XXREG1" title="Double-Extended Register 1" hidden="yes"/>
> <field id="MFF_XXREG2" title="Double-Extended Register 2" hidden="yes"/>
> <field id="MFF_XXREG3" title="Double-Extended Register 3" hidden="yes"/>
> + <field id="MFF_XXREG4" title="Double-Extended Register 4" hidden="yes"/>
> + <field id="MFF_XXREG5" title="Double-Extended Register 5" hidden="yes"/>
> + <field id="MFF_XXREG6" title="Double-Extended Register 6" hidden="yes"/>
> + <field id="MFF_XXREG7" title="Double-Extended Register 7" hidden="yes"/>
> </group>
>
> <group title="Layer 2 (Ethernet)">
> diff --git a/lib/nx-match.c b/lib/nx-match.c
> index cb49185f198b..0a27d6260009 100644
> --- a/lib/nx-match.c
> +++ b/lib/nx-match.c
> @@ -1054,7 +1054,7 @@ nx_put_raw(struct ofpbuf *b, enum ofp_version oxm,
> const struct match *match,
> ovs_be32 spi_mask;
> int match_len;
>
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> struct nxm_put_ctx ctx = { .output = b, .implied_ethernet = false };
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index ee186820200b..8e8a2c7021ee 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -6355,7 +6355,7 @@ odp_flow_key_from_flow__(const struct
> odp_flow_key_parms *parms,
> /* New "struct flow" fields that are visible to the datapath (including
> all
> * data fields) should be translated into equivalent datapath flow fields
> * here (you will have to add a OVS_KEY_ATTR_* for them). */
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> struct ovs_key_ethernet *eth_key;
> size_t encap[FLOW_MAX_VLAN_HEADERS] = {0};
> @@ -7464,7 +7464,7 @@ odp_flow_key_to_flow__(const struct nlattr *key, size_t
> key_len,
> /* New "struct flow" fields that are visible to the datapath (including
> all
> * data fields) should be translated from equivalent datapath flow fields
> * here (you will have to add a OVS_KEY_ATTR_* for them). */
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> enum odp_key_fitness fitness = ODP_FIT_ERROR;
> if (errorp) {
> @@ -8921,7 +8921,7 @@ commit_odp_actions(const struct flow *flow, struct flow
> *base,
> /* If you add a field that OpenFlow actions can change, and that is
> visible
> * to the datapath (including all data fields), then you should also add
> * code here to commit changes to the field. */
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> enum slow_path_reason slow1, slow2;
> bool mpls_done = false;
> diff --git a/lib/odp-util.h b/lib/odp-util.h
> index 339ffc14f248..85386d55290c 100644
> --- a/lib/odp-util.h
> +++ b/lib/odp-util.h
> @@ -147,7 +147,7 @@ void odp_portno_name_format(const struct hmap
> *portno_names,
> * add another field and forget to adjust this value.
> */
> #define ODPUTIL_FLOW_KEY_BYTES 640
> -BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> +BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> /* A buffer with sufficient size and alignment to hold an nlattr-formatted
> flow
> * key. An array of "struct nlattr" might not, in theory, be sufficiently
> diff --git a/lib/ofp-match.c b/lib/ofp-match.c
> index 86a082dde141..6525922ef8e7 100644
> --- a/lib/ofp-match.c
> +++ b/lib/ofp-match.c
> @@ -65,7 +65,7 @@ ofputil_netmask_to_wcbits(ovs_be32 netmask)
> void
> ofputil_wildcard_from_ofpfw10(uint32_t ofpfw, struct flow_wildcards *wc)
> {
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> /* Initialize most of wc. */
> flow_wildcards_init_catchall(wc);
> diff --git a/ofproto/ofproto-dpif-rid.h b/ofproto/ofproto-dpif-rid.h
> index 4df630c62bd3..21f32b93574b 100644
> --- a/ofproto/ofproto-dpif-rid.h
> +++ b/ofproto/ofproto-dpif-rid.h
> @@ -100,7 +100,7 @@ struct rule;
> /* Metadata for restoring pipeline context after recirculation. Helpers
> * are inlined below to keep them together with the definition for easier
> * updates. */
> -BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> +BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> struct frozen_metadata {
> /* Metadata in struct flow. */
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index 920d998e642f..7af65f86d13b 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -4477,7 +4477,7 @@ compose_output_action__(struct xlate_ctx *ctx,
> ofp_port_t ofp_port,
>
> /* If 'struct flow' gets additional metadata, we'll need to zero it out
> * before traversing a patch port. */
> - BUILD_ASSERT_DECL(FLOW_WC_SEQ == 42);
> + BUILD_ASSERT_DECL(FLOW_WC_SEQ == 43);
>
> if (!check_output_prerequisites(ctx, xport, flow, check_stp)) {
> return;
> diff --git a/python/ovs_build_helpers/extract_ofp_fields.py
> b/python/ovs_build_helpers/extract_ofp_fields.py
> index a70eabdb0071..bb049f5323f3 100644
> --- a/python/ovs_build_helpers/extract_ofp_fields.py
> +++ b/python/ovs_build_helpers/extract_ofp_fields.py
> @@ -136,7 +136,7 @@ def parse_oxm(s, prefix, n_bytes):
> global match_types
>
> m = re.match(
> - r"([A-Z0-9_]+)\(([0-9]+)\) since(?: OF(1\.[0-9]+) and)?
> v([12]\.[0-9]+)$", # noqa: E501
> + r"([A-Z0-9_]+)\(([0-9]+)\) since(?: OF(1\.[0-9]+) and)?
> v([123]\.[0-9]+)$", # noqa: E501
> s,
> )
> if not m:
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index 2889f81fb171..a65e612bd151 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -2351,9 +2351,9 @@ head_table () {
> instructions: meter apply_actions clear_actions write_actions
> write_metadata goto_table
> Write-Actions and Apply-Actions features:
> actions: output group set_field strip_vlan push_vlan mod_nw_ttl
> dec_ttl set_mpls_ttl dec_mpls_ttl push_mpls pop_mpls set_queue
> - supported on Set-Field:
> tun_{id,src,dst,ipv6_{src,dst},flags,gbp_{id,flags},erspan_{idx,ver,dir,hwid},metadata0...metadata63}
> metadata in_{port,port_oxm} pkt_mark ct_{mark,label} reg0...reg15
> xreg0...xreg7 xxreg0...xxreg3 eth_{src,dst} vlan_{tci,vid,pcp}
> mpls_{label,tc,ttl} ip_{src,dst} ipv6_{src,dst,label} nw_tos ip_dscp
> nw_{ecn,ttl} arp_{op,spa,tpa,sha,tha} tcp_{src,dst} udp_{src,dst}
> sctp_{src,dst} icmp_{type,code} icmpv6_{type,code}
> nd_{target,sll,tll,reserved,options_type} nsh_{flags,spi,si,c1...c4,ttl}
> + supported on Set-Field:
> tun_{id,src,dst,ipv6_{src,dst},flags,gbp_{id,flags},erspan_{idx,ver,dir,hwid},metadata0...metadata63}
> metadata in_{port,port_oxm} pkt_mark ct_{mark,label} reg0...reg31
> xreg0...xreg15 xxreg0...xxreg7 eth_{src,dst} vlan_{tci,vid,pcp}
> mpls_{label,tc,ttl} ip_{src,dst} ipv6_{src,dst,label} nw_tos ip_dscp
> nw_{ecn,ttl} arp_{op,spa,tpa,sha,tha} tcp_{src,dst} udp_{src,dst}
> sctp_{src,dst} icmp_{type,code} icmpv6_{type,code}
> nd_{target,sll,tll,reserved,options_type} nsh_{flags,spi,si,c1...c4,ttl}
> matching:
> - arbitrary mask: dp_hash
> tun_{id,src,dst,ipv6_{src,dst},flags,gbp_{id,flags},erspan_{idx,ver,dir,hwid},gtpu_{flags,msgtype},metadata0...metadata63}
> metadata pkt_mark
> ct_{state,mark,label,nw_{src,dst},ipv6_{src,dst},tp_{src,dst}} reg0...reg15
> xreg0...xreg7 xxreg0...xxreg3 eth_{src,dst} vlan_{tci,vid} ip_{src,dst}
> ipv6_{src,dst,label} ip_frag arp_{spa,tpa,sha,tha} tcp_{src,dst,flags}
> udp_{src,dst} sctp_{src,dst} nd_{target,sll,tll} nsh_{flags,c1...c4}
> + arbitrary mask: dp_hash
> tun_{id,src,dst,ipv6_{src,dst},flags,gbp_{id,flags},erspan_{idx,ver,dir,hwid},gtpu_{flags,msgtype},metadata0...metadata63}
> metadata pkt_mark
> ct_{state,mark,label,nw_{src,dst},ipv6_{src,dst},tp_{src,dst}} reg0...reg31
> xreg0...xreg15 xxreg0...xxreg7 eth_{src,dst} vlan_{tci,vid} ip_{src,dst}
> ipv6_{src,dst,label} ip_frag arp_{spa,tpa,sha,tha} tcp_{src,dst,flags}
> udp_{src,dst} sctp_{src,dst} nd_{target,sll,tll} nsh_{flags,c1...c4}
> exact match or wildcard: recirc_id packet_type conj_id
> in_{port,port_oxm} actset_output ct_{zone,nw_proto} eth_type vlan_pcp
> mpls_{label,tc,bos,ttl} nw_{proto,tos} ip_dscp nw_{ecn,ttl} arp_op
> icmp_{type,code} icmpv6_{type,code} nd_{reserved,options_type}
> nsh_{mdtype,np,spi,si,ttl}
>
> ' "$1"
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index 2363b72aa8c3..cda62718fa08 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -55,6 +55,36 @@ for test_case in \
> 'reg13=13/1 NXM,OXM' \
> 'reg14=14 NXM,OXM' \
> 'reg14=14/1 NXM,OXM' \
> + 'reg15=0 NXM,OXM' \
> + 'reg15=0/1 NXM,OXM' \
> + 'reg16=16 NXM,OXM' \
> + 'reg16=16/1 NXM,OXM' \
> + 'reg17=17 NXM,OXM' \
> + 'reg17=17/1 NXM,OXM' \
> + 'reg18=18 NXM,OXM' \
> + 'reg18=18/1 NXM,OXM' \
> + 'reg19=19 NXM,OXM' \
> + 'reg19=19/1 NXM,OXM' \
> + 'reg20=20 NXM,OXM' \
> + 'reg20=20/1 NXM,OXM' \
> + 'reg21=21 NXM,OXM' \
> + 'reg21=21/1 NXM,OXM' \
> + 'reg22=22 NXM,OXM' \
> + 'reg22=22/1 NXM,OXM' \
> + 'reg23=23/1 NXM,OXM' \
> + 'reg23=23 NXM,OXM' \
> + 'reg24=24/1 NXM,OXM' \
> + 'reg24=24 NXM,OXM' \
> + 'reg25=25 NXM,OXM' \
> + 'reg25=25/1 NXM,OXM' \
> + 'reg26=26 NXM,OXM' \
> + 'reg26=26/1 NXM,OXM' \
> + 'reg27=27 NXM,OXM' \
> + 'reg27=27/1 NXM,OXM' \
> + 'reg28=28 NXM,OXM' \
> + 'reg28=28/1 NXM,OXM' \
> + 'reg29=29 NXM,OXM' \
> + 'reg29=29/1 NXM,OXM' \
> 'xreg0=0 NXM,OXM' \
> 'xreg0=0/1 NXM,OXM' \
> 'xreg1=1 NXM,OXM' \
> @@ -71,6 +101,22 @@ for test_case in \
> 'xreg6=6/1 NXM,OXM' \
> 'xreg7=7 NXM,OXM' \
> 'xreg7=7/1 NXM,OXM' \
> + 'xreg8=8 NXM,OXM' \
> + 'xreg8=8/1 NXM,OXM' \
> + 'xreg9=9 NXM,OXM' \
> + 'xreg9=9/1 NXM,OXM' \
> + 'xreg10=10 NXM,OXM' \
> + 'xreg10=10/1 NXM,OXM' \
> + 'xreg11=11 NXM,OXM' \
> + 'xreg11=11/5 NXM,OXM' \
> + 'xreg12=12 NXM,OXM' \
> + 'xreg12=12/1 NXM,OXM' \
> + 'xreg13=13 NXM,OXM' \
> + 'xreg13=13/1 NXM,OXM' \
> + 'xreg14=14 NXM,OXM' \
> + 'xreg14=14/1 NXM,OXM' \
> + 'xreg15=15 NXM,OXM' \
> + 'xreg15=15/1 NXM,OXM' \
> 'xxreg0=0 NXM,OXM' \
> 'xxreg0=0/1 NXM,OXM' \
> 'xxreg1=1 NXM,OXM' \
> @@ -79,8 +125,18 @@ for test_case in \
> 'xxreg2=2/1 NXM,OXM' \
> 'xxreg3=3 NXM,OXM' \
> 'xxreg3=3/1 NXM,OXM' \
> + 'xxreg4=4 NXM,OXM' \
> + 'xxreg4=4/1 NXM,OXM' \
> + 'xxreg5=5 NXM,OXM' \
> + 'xxreg5=5/1 NXM,OXM' \
> + 'xxreg6=6 NXM,OXM' \
> + 'xxreg6=6/1 NXM,OXM' \
> + 'xxreg7=7 NXM,OXM' \
> + 'xxreg7=7/1 NXM,OXM' \
> 'xxreg3[[0..0]]=1 NXM,OXM' \
> 'xxreg3[[126..127]]=3 NXM,OXM' \
> + 'xxreg7[[0..0]]=1 NXM,OXM' \
> + 'xxreg7[[126..127]]=3 NXM,OXM' \
> 'reg3[[]]=3 NXM,OXM' \
> 'dl_src=00:11:22:33:44:55 any' \
> 'dl_src=00:11:22:33:44:55/00:ff:ff:ff:ff:ff NXM,OXM,OpenFlow11' \
I think, we need to add some more tests here:
1. There are more tests here in ovs-ofctl.at that check parsing and
formating fields, including registers. They mostly focus on the
first tree registers, but since we're using a different class for
the new ones, I think, it's important we add them to those tests
as well.
2. There are 'ofproto-dpif - .* registers' tests in ofproto-dpif.at
that check that registers are working properly and that they are
sharing space properly. Again, since it's a new class, we may
want to update those tests by adding new registers.
There is also one missing bit in this patch - it's missing definitions
for the python flow library in python/ovs/flow/ofp_fields.py and maybe
some tests in python/ovs/tests/test_ofp.py.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev