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. Also, please reply to questions in patches 9/10 and 10/10 regarding the load of potentially non-existent memory, which are remaining unanswered. And there is also one performance related question. Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
