[ovs-dev] [v6 01/11] dpif-netdev: Add command line and function pointer for miniflow extract

[Eelco Chaudron]

On 9 Jul 2021, at 13:10, Amber, Kumar wrote:

> Hi Eelco,
>
>> -----Original Message-----
>> From: Eelco Chaudron <echaudro at redhat.com>
>> Sent: Friday, July 9, 2021 4:36 PM
>> To: Amber, Kumar <kumar.amber at intel.com>
>> Cc: Flavio Leitner <fbl at sysclose.org>; Ferriter, Cian <cian.ferriter at intel.com>;
>> ovs-dev at openvswitch.org; i.maximets at ovn.org
>> Subject: Re: [ovs-dev] [v6 01/11] dpif-netdev: Add command line and function
>> pointer for miniflow extract
>>
>>
>>
>> On 8 Jul 2021, at 16:01, Amber, Kumar wrote:
>>
>>> Hi Flavio,
>>>
>>> Thanks for the Review
>>> Replies are inline.
>>>
>>> <Snip>
>>>
>>>>> +miniflow_extract_func
>>>>> +dp_mfex_impl_get_default(void)
>>>>> +{
>>>>> +    /* For the first call, this will be NULL. Compute the compile time default.
>>>>> +     */
>>>>> +    if (!default_mfex_func) {
>>>>> +
>>>>> +        VLOG_INFO("Default MFEX implementation is %s.\n",
>>>>> +                  mfex_impls[MFEX_IMPL_SCALAR].name);
>>>>> +        default_mfex_func = mfex_impls[MFEX_IMPL_SCALAR].extract_func;
>>>>> +    }
>>>>> +
>>>>> +    return default_mfex_func;
>>>>
>>>> Eelco asked to use VLOG_INFO_ONCE to avoid flooding the log, which in
>>>> the end will use a static variable. Perhaps it would be better to
>>>> define a static boolean like:
>>>>
>>>> miniflow_extract_func
>>>> dp_mfex_impl_get_default(void)
>>>> {
>>>>    /* For the first call, this will be NULL. Compute the compile time default.
>>>>     */
>>>>    static bool default_mfex_func_set = false;
>>>>
>>>>    if (OVS_UNLIKELY(!default_mfex_func_set)) {
>>>>        VLOG_INFO("Default MFEX implementation is %s.\n",
>>>>                  mfex_impls[MFEX_IMPL_SCALAR].name);
>>>>        // FIXME: Atomic set?
>>>>        default_mfex_func = mfex_impls[MFEX_IMPL_SCALAR].extract_func;
>>>>        default_mfex_func_set = true;
>>>>    }
>>>>
>>>>    return default_mfex_func;
>>>> }
>>>>
>>>
>>> Sound good taking into v7.
>>
>> As you already sent out a v7, I guess you mean v8?
>>
>> Are you planning to send out a v8 after you incorporate all Flavio's comments? If
>> so, I hold off on v7 and wait for v8.
>>
>
> The changes are in v7 itself.

Ok, Anyway, I’ll wait for Flavio to finish his review before I start on v7 to avoid having to look at v8 again.

>> <SNIP>