On 1/5/26 3:53 PM, Dumitru Ceara wrote:
> On 12/20/25 12:02 AM, Ilya Maximets wrote:
>> 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.
>>
>
> Hi Ilya,
>
> Thanks for the review!
>
>> nit: Maybe the subject line should specify 'openflow:' or 'meta-flow:'
>> area. All the other stuff in the patch follows the primary openflow
>> change.
>>
>
> Ack, changing it to 'openflow: ...'.
>
>>
>>> ---
>>> 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.
>>
>
> Ack, sounds good.
>
>>>
>>>
>>> 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+.
>>
>
> Ah, right, I'll change that accordingly.
>
>>> #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.
>>
>
> Would something like this work for you (I added a line to each type of
> register, including XREGs and XXREGs):
>
> diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml
> index c996cddad341..ca6394b0685f 100644
> --- a/lib/meta-flow.xml
> +++ b/lib/meta-flow.xml
> @@ -2902,11 +2902,19 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
> </field>
>
> <field id="MFF_REG0" title="Register 0">
> - 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,
> - version 2.6 added registers 8 through 15 and version 3.7 added
> registers
> - 16 through 31.
> + <p>
> + 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, version 2.6 added registers 8 through 15 and version 3.7 added
nit: Missed the last time, but the oxford comma before the final 'and' seems
like something we should preserve.
> + registers 16 through 31.
> + </p>
> +
> + <p>
> + Registers 0 through 15 are defined using OXM the
"using OXM the" ? Sounds strange.
> + <code>OFPXMC_NXM_1</code> class while registers 16 through 31 are
> + defined using the OXM <code>OFPXMC_EXPERIMENTER</code> class.
I think, it may be less confusing if we talk here in terms of code point
prefixes instead of OXM classes. Since we document OXM/NXM code points
here and 32-bit registers do not have OXM code points, mentioning the OXM
class may be hard to follow. Also, OFPXMC_EXPERIMENTER doesn't specify
which experimenter class is being used.
Maybe something like "Code points for registers 0 through 15 are defined
within NXM_NX code point prefix, while regitsters 16 through 31 are
defined within NXOXM_ET, due to limited space within the NXM_NX class."
What do you think?
> + </p>
> </field>
> <!-- XXX series -->
> <field id="MFF_REG1" title="Register 1" hidden="yes"/>
> @@ -2952,6 +2960,11 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
> 2.6 and later) or 16 (in version 3.7 and later).
> </p>
>
> + <p>
> + The registers are defined using OXM the <code>OFPXMC_NXM_1</code>
This should be OFPXMC_PACKET_REGS instead. But anyways, the comment
above applies here as well. Maybe something like:
"Code points for all of the extended registers are defined within
OXM_OF_PKT_REG code point prefix."
> + class.
> + </p>
> +
> <p>
> Each of the 64-bit extended registers overlays two of the 32-bit
> registers: <code>xreg0</code> overlays <code>reg0</code> and
> @@ -2999,6 +3012,12 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123)
> vSwitch 4 128-bit registers (in versions 2.6 and later) and 8
> (in version 3.7 and later).
> </p>
> +
> + <p>
> + Registers 0 through 3 are defined using OXM the
> + <code>OFPXMC_NXM_1</code> class while registers 4 through 7 are
> + defined using the OXM <code>OFPXMC_EXPERIMENTER</code> class.
> + </p>
"Code points for double-extended registers 0 through 3 ..."
> </field>
>
> <!-- XXX series -->
>
>>>
>>> <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.
>>
>
> Sure, I can add more tests for the new registers in v2.
>
>> 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.
>>
>
> I'm not sure I follow this part. ofp_fields.py is generated with
> extract_ofp_fields.py. The class I used for the new registers,
> NXOXM_ET, is already supported there.
>
> Checking the generated ofp_fields.py with my patch applied, I see
> the new registers there. Am I missing something?
Ah, my bad. Ignore this part.
>
> Ack on adding new tests to test_ofp.py, I'll do that in v2.
>
>> Best regards, Ilya Maximets.
>>
>
> Regards,
> Dumitru
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
