Hi Eelco, > -----Original Message----- > From: Eelco Chaudron <[email protected]> > Sent: Friday, July 9, 2021 4:42 PM > To: Amber, Kumar <[email protected]> > Cc: Flavio Leitner <[email protected]>; Ferriter, Cian > <[email protected]>; > [email protected]; [email protected] > 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 <[email protected]> > >> Sent: Friday, July 9, 2021 4:36 PM > >> To: Amber, Kumar <[email protected]> > >> Cc: Flavio Leitner <[email protected]>; Ferriter, Cian > >> <[email protected]>; [email protected]; > >> [email protected] > >> 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 😊 > >> <SNIP> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
