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

> Hi Eelco,
>
>
>
>> -----Original Message-----
>> From: Eelco Chaudron <[email protected]>
>> Sent: Thursday, July 15, 2021 1:38 PM
>> To: Amber, Kumar <[email protected]>
>> Cc: [email protected]; [email protected]; [email protected]; Van
>> Haaren, Harry <[email protected]>; Ferriter, Cian
>> <[email protected]>; Stokes, Ian <[email protected]>
>> Subject: Re: [v12 01/11] dpif-netdev: Add command line and function pointer 
>> for
>> miniflow extract
>>
>>
>> On 14 Jul 2021, at 16:14, kumar Amber wrote:
>>
>>> From: Kumar Amber <[email protected]>
>>>
>>> This patch introduces the MFEX function pointers which allows the user
>>> to switch between different miniflow extract implementations which are
>>> provided by the OVS based on optimized ISA CPU.
>>>
>>> The user can query for the available minflow extract variants
>>> available for that CPU by following commands:
>>>
>>> $ovs-appctl dpif-netdev/miniflow-parser-get
>>>
>>> Similarly an user can set the miniflow implementation by the following
>>> command :
>>>
>>> $ ovs-appctl dpif-netdev/miniflow-parser-set name
>>>
>>> This allows for more performance and flexibility to the user to choose
>>> the miniflow implementation according to the needs.
>>>
>>> Signed-off-by: Kumar Amber <[email protected]>
>>> Co-authored-by: Harry van Haaren <[email protected]>
>>> Signed-off-by: Harry van Haaren <[email protected]>
>>> Acked-by: Eelco Chaudron <[email protected]>
>>
>> Although I ACKed this patchset, I noticed one small additional change see 
>> below.
>>
>> <SNIP>
>>
>>> +
>>> +/* This function checks all available MFEX implementations, and
>>> +selects and
>>> + * returns the function pointer to the one requested by "name". If
>>> +nothing
>>> + * is found it returns error.
>>> + */
>>> +int
>>> +dp_mfex_impl_get_by_name(const char *name, miniflow_extract_func
>>> +*out_func) {
>>> +    if ((name == NULL) || (out_func == NULL)) {
>>> +        return -EINVAL;
>>> +    }
>>> +
>>
>> You do not need the extra parenthesis so you could just do:
>>
>>   if (name == NULL || out_func == NULL) {
>>
>> Or even shorter:
>>
>>   if (!name || !out_func) {
>
> Will include this should I keep you ACK while re-push ?

Thanks, if that is the only change, keep the ACK.

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to