On 3/7/24 20:18, Ilya Maximets wrote:
> On 3/7/24 18:08, Eric Garver wrote:
>> The next commit will convert a dp feature from bool to atomic_bool. As
>> such we have to add support to the macros and functions. We must pass by
>> reference instead of pass by value because all the atomic operations
>> require a reference.
>>
>> Signed-off-by: Eric Garver <e...@garver.life>
>> ---
>
> Not a full review, but a few quick comments inline.
>
>> ofproto/ofproto-dpif.c | 52 ++++++++++++++++++++++++++++++++++++------
>> 1 file changed, 45 insertions(+), 7 deletions(-)
>>
>> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
>> index d732198de5ea..4e22ee0e8045 100644
>> --- a/ofproto/ofproto-dpif.c
>> +++ b/ofproto/ofproto-dpif.c
>> @@ -717,6 +717,8 @@ close_dpif_backer(struct dpif_backer *backer, bool del)
>> }
>>
>> static void check_support(struct dpif_backer *backer);
>> +static void copy_support(struct dpif_backer_support *dst,
>> + struct dpif_backer_support *src);
>>
>> static int
>> open_dpif_backer(const char *type, struct dpif_backer **backerp)
>> @@ -837,7 +839,7 @@ open_dpif_backer(const char *type, struct dpif_backer
>> **backerp)
>> * 'boottime_support' can be checked to prevent 'support' to be changed
>> * beyond the datapath capabilities. In case 'support' is changed by
>> * the user, 'boottime_support' can be used to restore it. */
>> - backer->bt_support = backer->rt_support;
>> + copy_support(&backer->bt_support, &backer->rt_support);
>>
>> return error;
>> }
>> @@ -1611,6 +1613,22 @@ CHECK_FEATURE__(ct_orig_tuple6, ct_orig_tuple6,
>> ct_nw_proto, 1, ETH_TYPE_IPV6)
>> #undef CHECK_FEATURE
>> #undef CHECK_FEATURE__
>>
>> +static void
>> +copy_support(struct dpif_backer_support *dst, struct dpif_backer_support
>> *src)
>> +{
>> +#define DPIF_SUPPORT_FIELD(TYPE, NAME, TITLE) \
>> + if (!strcmp(#TYPE, "atomic_bool")) { \
>> + bool value; \
>> + atomic_read_relaxed((atomic_bool *) &src->NAME, &value); \
>> + atomic_store_relaxed((atomic_bool *) &dst->NAME, value); \
>> + } else { \
>> + dst->NAME = src->NAME; \
>> + }
>> +
>> + DPIF_SUPPORT_FIELDS
>> +#undef DPIF_SUPPORT_FIELD
>
> We need to copy odp_support as well.
>
>> +}
>> +
>> static void
>> check_support(struct dpif_backer *backer)
>> {
>> @@ -6248,20 +6266,30 @@ ofproto_unixctl_dpif_dump_dps(struct unixctl_conn
>> *conn, int argc OVS_UNUSED,
>> }
>>
>> static void
>> -show_dp_feature_bool(struct ds *ds, const char *feature, bool b)
>> +show_dp_feature_bool(struct ds *ds, const char *feature, const bool *b)
>> {
>> - ds_put_format(ds, "%s: %s\n", feature, b ? "Yes" : "No");
>> + ds_put_format(ds, "%s: %s\n", feature, *b ? "Yes" : "No");
>> }
>>
>> static void
static void OVS_UNUSED
Should silence the compiler errors.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
