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

Reply via email to