On 7/31/24 14:14, Dumitru Ceara wrote: > On 7/31/24 12:45, Ilya Maximets wrote: >> On 7/31/24 12:38, Ilya Maximets wrote: >>> On 7/31/24 11:05, Dumitru Ceara wrote: >>>> From: Ales Musil <[email protected]> >>>> >>>> Add detection for sample action that allows to configure >>>> obs_domain_id and obs_point_id via registers. This feature >>>> is available only from OvS version 3.4. >>>> >>>> Signed-off-by: Ales Musil <[email protected]> >>>> --- >>>> controller/chassis.c | 10 +++++ >>>> include/ovn/features.h | 3 ++ >>>> lib/features.c | 91 +++++++++++++++++++++++++++++++++++++++ >>>> northd/en-global-config.c | 10 +++++ >>>> northd/en-global-config.h | 1 + >>>> 5 files changed, 115 insertions(+) >>>> >>>> diff --git a/controller/chassis.c b/controller/chassis.c >>>> index 4942ba281d..1aa51b9d6c 100644 >>>> --- a/controller/chassis.c >>>> +++ b/controller/chassis.c >>>> @@ -372,6 +372,9 @@ chassis_build_other_config(const struct >>>> ovs_chassis_cfg *ovs_cfg, >>>> smap_replace(config, OVN_FEATURE_LS_DPG_COLUMN, "true"); >>>> smap_replace(config, OVN_FEATURE_CT_COMMIT_NAT_V2, "true"); >>>> smap_replace(config, OVN_FEATURE_CT_COMMIT_TO_ZONE, "true"); >>>> + bool sample = ovs_feature_is_supported(OVS_DP_SAMPLE_REG_SUPPORT); >>>> + smap_replace(config, OVN_FEATURE_SAMPLE_WITH_REGISTERS, >>>> + sample ? "true" : "false"); >>>> } >>>> >>>> /* >>>> @@ -523,6 +526,12 @@ chassis_other_config_changed(const struct >>>> ovs_chassis_cfg *ovs_cfg, >>>> return true; >>>> } >>>> >>>> + if (!smap_get_bool(&chassis_rec->other_config, >>>> + OVN_FEATURE_SAMPLE_WITH_REGISTERS, >>>> + false)) { >>>> + return true; >>>> + } >> >> Also, this doesn't look right. Unlike other features, this one >> is conditional. So, chassis_other_config_changed() will always >> return 'true' if OVS doesn't support the feature. >> > > Good point, this needs fixing. > >>>> + >>>> return false; >>>> } >>>> >>>> @@ -656,6 +665,7 @@ update_supported_sset(struct sset *supported) >>>> sset_add(supported, OVN_FEATURE_LS_DPG_COLUMN); >>>> sset_add(supported, OVN_FEATURE_CT_COMMIT_NAT_V2); >>>> sset_add(supported, OVN_FEATURE_CT_COMMIT_TO_ZONE); >>>> + sset_add(supported, OVN_FEATURE_SAMPLE_WITH_REGISTERS); >>>> } >>>> >>>> static void >>>> diff --git a/include/ovn/features.h b/include/ovn/features.h >>>> index 7b67b2f777..5c90a36450 100644 >>>> --- a/include/ovn/features.h >>>> +++ b/include/ovn/features.h >>>> @@ -29,6 +29,7 @@ >>>> #define OVN_FEATURE_LS_DPG_COLUMN "ls-dpg-column" >>>> #define OVN_FEATURE_CT_COMMIT_NAT_V2 "ct-commit-nat-v2" >>>> #define OVN_FEATURE_CT_COMMIT_TO_ZONE "ct-commit-to-zone" >>>> +#define OVN_FEATURE_SAMPLE_WITH_REGISTERS "ovn-sample-with-registers" >>>> >>>> /* OVS datapath supported features. Based on availability OVN might >>>> generate >>>> * different types of openflows. >>>> @@ -39,6 +40,7 @@ enum ovs_feature_support_bits { >>>> OVS_CT_TUPLE_FLUSH_BIT, >>>> OVS_DP_HASH_L4_SYM_BIT, >>>> OVS_DP_GROUP_SUPPORT_BIT, >>>> + OVS_DP_SAMPLE_REG_SUPPORT_BIT, >>>> }; >>>> >>>> enum ovs_feature_value { >>>> @@ -47,6 +49,7 @@ enum ovs_feature_value { >>>> OVS_CT_TUPLE_FLUSH_SUPPORT = (1 << OVS_CT_TUPLE_FLUSH_BIT), >>>> OVS_DP_HASH_L4_SYM_SUPPORT = (1 << OVS_DP_HASH_L4_SYM_BIT), >>>> OVS_DP_GROUP_SUPPORT = (1 << OVS_DP_GROUP_SUPPORT_BIT), >>>> + OVS_DP_SAMPLE_REG_SUPPORT = (1 << OVS_DP_SAMPLE_REG_SUPPORT_BIT), >>> >>> Same here. The 'DP' part makes no sense as this is feature has >>> nothing to do with the datapath implementation. >>> > > FWIW, in my eyes, OVN's dataplane (datapath) is OVS so everything listed > here is datapath related from OVN's perspective. But OK, for > CT_TUPLE_FLUSH we don't use the "DP" particle, I guess we can skip that > here too. > > What name would make more sense to you? OVS_SAMPLE_REG_SUPPORT?
Sounds fine. > >>> Best regards, Ilya Maximets. >> > > Regards, > Dumitru > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
