> -----Original Message----- > From: Eelco Chaudron <[email protected]> > Sent: Thursday 14 July 2022 15:37 > To: Van Haaren, Harry <[email protected]> > Cc: Ilya Maximets <[email protected]>; Finn, Emma > <[email protected]>; [email protected]; Amber, Kumar > <[email protected]>; Pai G, Sunil <[email protected]>; Stokes, Ian > <[email protected]>; Flavio Leitner <[email protected]> > Subject: Re: [PATCH v10 00/10] Actions Infrastructure + Optimizations > > > > On 14 Jul 2022, at 16:22, Van Haaren, Harry wrote: > > >> -----Original Message----- > >> From: Ilya Maximets <[email protected]> > >> Sent: Thursday, July 14, 2022 3:10 PM > >> To: Finn, Emma <[email protected]>; Eelco Chaudron > >> <[email protected]>; Van Haaren, Harry > <[email protected]> > >> Cc: [email protected]; [email protected]; Amber, Kumar > >> <[email protected]>; Pai G, Sunil <[email protected]>; > >> Stokes, Ian <[email protected]>; Flavio Leitner <[email protected]> > >> Subject: Re: [PATCH v10 00/10] Actions Infrastructure + Optimizations > >> > >> On 7/14/22 16:03, Finn, Emma wrote: > >>> > >>> > >>>> -----Original Message----- > >>>> From: Eelco Chaudron <[email protected]> > >>>> Sent: Thursday 14 July 2022 14:27 > >>>> To: Van Haaren, Harry <[email protected]> > >>>> Cc: Finn, Emma <[email protected]>; [email protected]; > >>>> [email protected]; Amber, Kumar <[email protected]>; Pai > G, > >>>> Sunil <[email protected]>; Stokes, Ian <[email protected]> > >>>> Subject: Re: [PATCH v10 00/10] Actions Infrastructure + > >>>> Optimizations > >>>> > >>>> > >>>> > >>>> On 14 Jul 2022, at 15:04, Van Haaren, Harry wrote: > >>>> > >>>>>> -----Original Message----- > >>>>>> From: Eelco Chaudron <[email protected]> > >>>>>> Sent: Thursday, July 14, 2022 1:55 PM > >>>>>> To: Van Haaren, Harry <[email protected]>; Finn, Emma > >>>>>> <[email protected]> > >>>>>> Cc: [email protected]; [email protected]; Amber, Kumar > >>>>>> <[email protected]>; Pai G, Sunil <[email protected]>; > >>>>>> Stokes, Ian <[email protected]> > >>>>>> Subject: Re: [PATCH v10 00/10] Actions Infrastructure + > >>>>>> Optimizations > >>>>> > >>>>> <snip cover letter contents> > >>>>> > >>>>>>> V10; > >>>>>>> - Fixed CI build issue on OSX around AVX512 linking (jenkins CI) > >>>>>>> - Moved docs and reworded sections (thanks Ilya for feedback) > >>>>>>> - Reworked one instance of <= OVS_ATTR_MAX back to original > >>>>>>> form(Eelco) > >>>>>> > >>>>>> I’ve just finished my review purely based on visual inspection > >>>>>> and compile success, and I will send out some minor comments > >>>>>> after the break ;) > >>>>> > >>>>> Sure - please keep in mind that theres few working hours before > >>>>> merge window ends, so unless absolutely critical to fix *before* > >>>>> merge, we can > >>>> fixup things next week. > >>>>> > >>>>> To be very clear; if there a genuine issue, yes lets fix. Variable > >>>>> renames, tidys etc, can all be handled starting from next week. > >>>>> > >>>>>> After that, I will do the actual functional testing on an AVX512 > >>>>>> machine and let you know the results. This will give you time to > >>>>>> fix/discuss the comment while I’m testing. > >>>>> > >>>>> Looking forward! > >>>> > >>>> They should be in your inbox, most of them can be changed quickly, > >>>> just sent out the v11 once done, as I need time to test this anyway > >>>> before I can give my final ACKs. > >>>> > >>>> My plan is to test v10 (or v11 if it’s there) tomorrow. > >>>> > >>>> //Eelco > >>> > >>> Thanks for the comments Eelco. As Harry mentioned above, next > >>> revision will only > >> include critical fixes. Comment tidy ups and variable renaming will > >> be left out until next week. > >>> > >>> Things I will fix for next revision: > >>> 01/10 - Atomic store refactor. > >>> 07/10 - Refactor of avx init/probe functions. > >>> 09/10 - Swap OVS_ACTION_ATTR_MAX tobe first in check. > >>> > >>> Comments that will be addressed next week: > >>> 01/10 - comment clean up > >>> 02/10 - comment clean up > >>> 04/10 - 2x remove the trailing \n > >>> - Move unit test to odp.at > >>> 05/10 - comment clean up > >>> 06/10 - comment clean up > >>> 09/10 - indentation if off > >>> 10/10 - renaming variable/functions > >> > >> These are fairly trivial, can we fix them as well, please? > >> Submission next weak will mean backports of these cosmetic changes or > >> conflicts with later backports of bug fixes. > > > > I'll leave this to Emma to decide. > > I agree with Ilya here, we should get all of this in one patch, should not be > too much work. And it safes a lot afterwards… >
Sure, I will address all comments for next revisions. <SNIP> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
