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 ?

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

Reply via email to