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…

>> 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.
>
> Hah, I was literally typing up responses as your email arrived.
> Replies sent to both questions "at the speed I can type replies" 😊
>
> loading/k-masks: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2022-July/396201.html
> performance: 
> https://mail.openvswitch.org/pipermail/ovs-dev/2022-July/396203.html
>
>> Best regards, Ilya Maximets.
>
> Regards, -Harry

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

Reply via email to