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?
>> Best regards, Ilya Maximets.
>
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
