On 9 Jul 2021, at 13:16, Amber, Kumar wrote:
> Hi Eelco,
>
>> -----Original Message-----
>> From: Eelco Chaudron <echau...@redhat.com>
>> Sent: Friday, July 9, 2021 4:42 PM
>> To: Amber, Kumar <kumar.am...@intel.com>
>> Cc: Flavio Leitner <f...@sysclose.org>; Ferriter, Cian
>> <cian.ferri...@intel.com>;
>> ovs-dev@openvswitch.org; i.maxim...@ovn.org
>> Subject: Re: [ovs-dev] [v6 01/11] dpif-netdev: Add command line and function
>> pointer for miniflow extract
>>
>>
>>
>> On 9 Jul 2021, at 13:10, Amber, Kumar wrote:
>>
>>> Hi Eelco,
>>>
>>>> -----Original Message-----
>>>> From: Eelco Chaudron <echau...@redhat.com>
>>>> Sent: Friday, July 9, 2021 4:36 PM
>>>> To: Amber, Kumar <kumar.am...@intel.com>
>>>> Cc: Flavio Leitner <f...@sysclose.org>; Ferriter, Cian
>>>> <cian.ferri...@intel.com>; ovs-dev@openvswitch.org;
>>>> i.maxim...@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.
>>
>
> You can review other patches in the series in the meanwhile 😊
But then I have again to review the v8, I have some other stuff to work on in
the meantime ;)
I’m assuming Flavio will look at the whole set. Flavio can you ping us when
you’re done.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
