On 9/1/26 7:41 AM, Han Zhou wrote:
> On Tue, Aug 18, 2026 at 2:27 AM Dumitru Ceara <[email protected]> wrote:
>>
>> On 7/22/26 7:49 AM, Han Zhou wrote:
>>> OVS 3.7 (OVS commit 3900653) increased FLOW_N_REGS from 16 to 32, making
>>> registers reg16..reg31 (xxreg4..xxreg7) usable. However, whether the
>>> running ovs-vswitchd actually supports them depends on its version, so
>>> OVN must detect this at runtime before relying on those registers.
>>>
>>> Add an OVS OpenFlow feature probe (OVS_REG32_SUPPORT) that pushes a
>>> discarded bundle containing a flow_mod matching reg16. An ovs-vswitchd
>>> that does not support 32 registers rejects it with an error; a newer one
>>> accepts it, so the barrier is reached without error.
>>>
>>> ovn-controller advertises the result to the SB Chassis via
>>> other_config:ovn-32-registers, and ovn-northd aggregates it across all
>>> local chassis into chassis_features.reg32. This lets later features use
>>> registers beyond reg15 only when every chassis supports them, and fall
>>> back otherwise.
>>>
>>> Assisted-by: Claude Opus 4.8, Cursor
>>> Signed-off-by: Han Zhou <[email protected]>
>>> ---
>>
>> Hi Han,
>>
>> This is not really needed for the "Support per-route VNI for learned
>> EVPN routes." patch-set as I see you reworked it without new registers.
>> But I think it would be a nice to have feature. I'm sure we will need
>> the detection mechanism in the near future.
>>
>> Do you mind posting this one separately as a regular patch?
>
> Thanks Dumitru. Could you review this patch as a separate one? If it looks
> good, I can merge this one and post the 2nd patch again.
>
Hi Han,
Sure, sounds good.
> Best,
> Han
>
>>
>> Regards,
>> Dumitru
>>
>>> controller/chassis.c | 12 +++++++
>>> include/ovn/features.h | 6 ++++
>>> lib/features.c | 75 +++++++++++++++++++++++++++++++++++++++
>>> northd/en-global-config.c | 13 +++++++
>>> northd/en-global-config.h | 1 +
>>> 5 files changed, 107 insertions(+)
>>>
>>> diff --git a/controller/chassis.c b/controller/chassis.c
>>> index 0e3bd37d8752..2160c84d5c91 100644
>>> --- a/controller/chassis.c
>>> +++ b/controller/chassis.c
>>> @@ -74,6 +74,8 @@ struct ovs_chassis_cfg {
>>> bool sample_with_regs;
>>> /* Does OVS support flushing CT zones using label/mark? */
>>> bool ct_label_flush;
>>> + /* Does OVS support 32 registers (reg16..reg31)? */
>>> + bool reg32;
>>> };
>>>
>>> enum chassis_update_status {
>>> @@ -385,6 +387,7 @@ chassis_parse_ovs_config(const struct
> ovsrec_open_vswitch_table *ovs_table,
>>> ovs_feature_is_supported(OVS_SAMPLE_REG_SUPPORT);
>>> ovs_cfg->ct_label_flush =
>>> ovs_feature_is_supported(OVS_CT_LABEL_FLUSH_SUPPORT);
>>> + ovs_cfg->reg32 = ovs_feature_is_supported(OVS_REG32_SUPPORT);
>>>
>>> return true;
>>> }
>>> @@ -424,6 +427,8 @@ chassis_build_other_config(const struct
> ovs_chassis_cfg *ovs_cfg,
>>> smap_replace(config, OVN_FEATURE_CT_LABEL_FLUSH,
>>> ovs_cfg->ct_label_flush ? "true" :"false");
>>> smap_replace(config, OVN_FEATURE_CT_STATE_SAVE, "true");
>>> + smap_replace(config, OVN_FEATURE_REG32,
>>> + ovs_cfg->reg32 ? "true" : "false");
>>> }
>>>
>>> /*
>>> @@ -597,6 +602,12 @@ chassis_other_config_changed(const struct
> ovs_chassis_cfg *ovs_cfg,
>>> return true;
>>> }
>>>
>>> + bool chassis_reg32 =
>>> + smap_get_bool(&chassis_rec->other_config, OVN_FEATURE_REG32,
> false);
>>> + if (chassis_reg32 != ovs_cfg->reg32) {
>>> + return true;
>>> + }
>>> +
>>> return false;
>>> }
>>>
>>> @@ -779,6 +790,7 @@ update_supported_sset(struct sset *supported)
>>> sset_add(supported, OVN_FEATURE_CT_NEXT_ZONE);
>>> sset_add(supported, OVN_FEATURE_CT_LABEL_FLUSH);
>>> sset_add(supported, OVN_FEATURE_CT_STATE_SAVE);
>>> + sset_add(supported, OVN_FEATURE_REG32);
>>> }
>>>
>>> static void
>>> diff --git a/include/ovn/features.h b/include/ovn/features.h
>>> index 7207da67fd20..bb7c85ea5cff 100644
>>> --- a/include/ovn/features.h
>>> +++ b/include/ovn/features.h
>>> @@ -27,6 +27,7 @@
>>> #define OVN_FEATURE_CT_NEXT_ZONE "ct-next-zone"
>>> #define OVN_FEATURE_CT_LABEL_FLUSH "ct-label-flush"
>>> #define OVN_FEATURE_CT_STATE_SAVE "ct-state-save"
>>> +#define OVN_FEATURE_REG32 "ovn-32-registers"
Should we also update chassis_features_list() to list the state of the
new feature? And I guess we should also update the ovn-controller.at
"AT_SETUP([features])" test to exclude this new feature.
>>>
>>> /* DEPRECATED: The following features can be removed
>>> * after the next LTS version release. */
>>> @@ -45,6 +46,7 @@ enum ovs_feature_support_bits {
>>> OVS_OF_GROUP_SUPPORT_BIT,
>>> OVS_SAMPLE_REG_SUPPORT_BIT,
>>> OVS_CT_LABEL_FLUSH_BIT,
>>> + OVS_REG32_SUPPORT_BIT,
>>> };
>>>
>>> enum ovs_feature_value {
>>> @@ -55,6 +57,10 @@ enum ovs_feature_value {
>>> OVS_OF_GROUP_SUPPORT = (1 << OVS_OF_GROUP_SUPPORT_BIT),
>>> OVS_SAMPLE_REG_SUPPORT = (1 << OVS_SAMPLE_REG_SUPPORT_BIT),
>>> OVS_CT_LABEL_FLUSH_SUPPORT = (1 << OVS_CT_LABEL_FLUSH_BIT),
>>> + /* OVS (and its OpenFlow interface) supports 32 32-bit registers,
>>> + * i.e. registers reg16..reg31 (xxreg4..xxreg7) are usable. Older
> OVS
>>> + * versions only support 16 registers. */
>>> + OVS_REG32_SUPPORT = (1 << OVS_REG32_SUPPORT_BIT),
>>> };
>>>
>>> void ovs_feature_support_destroy(void);
>>> diff --git a/lib/features.c b/lib/features.c
>>> index 7bee1c3806e6..b787bf7e8529 100644
>>> --- a/lib/features.c
>>> +++ b/lib/features.c
>>> @@ -269,6 +269,73 @@ sample_with_reg_handle_barrier(struct
> ovs_openflow_feature *feature OVS_UNUSED)
>>> return true;
>>> }
>>>
>>> +static void
>>> +reg32_send_request(struct ovs_openflow_feature *feature)
>>> +{
>>> + struct ofputil_bundle_ctrl_msg ctrl = {
>>> + .bundle_id = 0,
>>> + .flags = OFPBF_ORDERED | OFPBF_ATOMIC,
>>> + .type = OFPBCT_OPEN_REQUEST,
>>> + };
>>> + rconn_send(swconn,
>>> + ofputil_encode_bundle_ctrl_request(OFP15_VERSION,
> &ctrl), NULL);
>>> +
>>> + struct ofputil_flow_mod fm = {
>>> + .priority = 0,
>>> + .table_id = 0,
>>> + .ofpacts = NULL,
>>> + .ofpacts_len = 0,
>>> + .command = OFPFC_ADD,
>>> + .new_cookie = htonll(0),
>>> + .buffer_id = UINT32_MAX,
>>> + .out_port = OFPP_ANY,
>>> + .out_group = OFPG_ANY,
>>> + };
>>> +
>>> + /* Match on reg16, which only exists when the running OVS supports
> 32
>>> + * registers. Older OVS versions reject the flow_mod with an
> error. */
>>> + struct match match;
>>> + match_init_catchall(&match);
>>> + match_set_reg(&match, MFF_REG16 - MFF_REG0, 1);
Maybe it's safer to match on MFF_REG31 instead? What do you think?
>>> + minimatch_init(&fm.match, &match);
>>> +
>>> + struct ofpbuf *fm_msg = ofputil_encode_flow_mod(&fm,
> OFPUTIL_P_OF15_OXM);
>>> +
>>> + struct ofputil_bundle_add_msg bam = {
>>> + .bundle_id = ctrl.bundle_id,
>>> + .flags = ctrl.flags,
>>> + .msg = fm_msg->data,
>>> + };
>>> + struct ofpbuf *msg = ofputil_encode_bundle_add(OFP15_VERSION,
> &bam);
>>> +
>>> + feature->xid = ((struct ofp_header *) msg->data)->xid;
>>> + rconn_send(swconn, msg, NULL);
>>> +
>>> + ctrl.type = OFPBCT_DISCARD_REQUEST;
>>> + rconn_send(swconn,
>>> + ofputil_encode_bundle_ctrl_request(OFP15_VERSION,
> &ctrl), NULL);
>>> +
>>> + minimatch_destroy(&fm.match);
>>> + ofpbuf_delete(fm_msg);
>>> +}
>>> +
>>> +static bool
>>> +reg32_handle_response(struct ovs_openflow_feature *feature,
>>> + enum ofptype type, const struct ofp_header *oh)
>>> +{
>>> + if (type != OFPTYPE_ERROR) {
>>> + log_unexpected_reply(feature, oh);
>>> + }
>>> +
>>> + return false;
>>> +}
>>> +
>>> +static bool
>>> +reg32_handle_barrier(struct ovs_openflow_feature *feature OVS_UNUSED)
>>> +{
>>> + return true;
>>> +}
>>> +
>>> static void
>>> ct_label_flush_send_request(struct ovs_openflow_feature *feature)
>>> {
>>> @@ -345,6 +412,13 @@ static struct ovs_openflow_feature
> all_openflow_features[] = {
>>> .handle_response = ct_label_flush_handle_response,
>>> .handle_barrier = ct_label_flush_handle_barrier,
>>> },
>>> + {
>>> + .value = OVS_REG32_SUPPORT,
>>> + .name = "reg32",
>>> + .send_request = reg32_send_request,
>>> + .handle_response = reg32_handle_response,
>>> + .handle_barrier = reg32_handle_barrier,
>>> + },
>>> };
>>>
>>> static bool
>>> @@ -418,6 +492,7 @@ ovs_feature_is_valid(enum ovs_feature_value feature)
>>> case OVS_OF_GROUP_SUPPORT:
>>> case OVS_SAMPLE_REG_SUPPORT:
>>> case OVS_CT_LABEL_FLUSH_SUPPORT:
>>> + case OVS_REG32_SUPPORT:
>>> return true;
>>> default:
>>> return false;
>>> diff --git a/northd/en-global-config.c b/northd/en-global-config.c
>>> index 4e6b07ebe50e..0667e11ffe8a 100644
>>> --- a/northd/en-global-config.c
>>> +++ b/northd/en-global-config.c
>>> @@ -520,6 +520,7 @@ northd_enable_all_features(struct
> ed_type_global_config *data)
>>> .ct_next_zone = true,
>>> .ct_label_flush = true,
>>> .ct_state_save = true,
>>> + .reg32 = true,
>>> };
>>> }
>>>
>>> @@ -590,6 +591,14 @@ build_chassis_features(const struct
> sbrec_chassis_table *sbrec_chassis_table,
>>> chassis_features->ct_state_save) {
>>> chassis_features->ct_state_save = false;
>>> }
>>> +
>>> + bool reg32 =
>>> + smap_get_bool(&chassis->other_config,
>>> + OVN_FEATURE_REG32,
>>> + false);
The indentation is weird here but it was weird for the rest of the
features above so let's keep it as is.
>>> + if (!reg32 && chassis_features->reg32) {
>>> + chassis_features->reg32 = false;
>>> + }
>>> }
>>> }
>>>
>>> @@ -747,6 +756,10 @@ chassis_features_changed(const struct
> chassis_features *present,
>>> return true;
>>> }
>>>
>>> + if (present->reg32 != updated->reg32) {
>>> + return true;
>>> + }
>>> +
>>> return false;
>>> }
>>>
>>> diff --git a/northd/en-global-config.h b/northd/en-global-config.h
>>> index 5a7b7ecda766..9b75ecff3d21 100644
>>> --- a/northd/en-global-config.h
>>> +++ b/northd/en-global-config.h
>>> @@ -20,6 +20,7 @@ struct chassis_features {
>>> bool ct_next_zone;
>>> bool ct_label_flush;
>>> bool ct_state_save;
>>> + bool reg32;
>>> };
>>>
>>> struct global_config_tracked_data {
>
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
