| Message ID | 20220713182807.3416578-1-harry.van.haaren@intel.com |
|---|---|
| Headers | show |
| Series | Actions Infrastructure + Optimizations | expand |
On 13 Jul 2022, at 20:27, Harry van Haaren wrote: > This patchset introduces actions infrastructure changes which allows > the user to choose between different action implementations based on > CPU ISA by using different commands. The infrastructure also > provides a way to check the correctness of the ISA optimized action > version against the scalar version. > > This series also introduces optimized versions of the following > actions: > - push_vlan > - pop_vlan > - set_masked eth > - set_masked ipv4 > > Below is a table indicating some relative performance benefits for > these actions. > +-----------------------------------------------+-------------------+-----------------+ > | Actions | Scalar with series| AVX with series | > +-----------------------------------------------+-------------------+-----------------+ > | mod_dl_dst | 1.01x | 1.13x | > +-----------------------------------------------+-------------------+-----------------+ > | push_vlan | 1.01x | 1.10x | > +-----------------------------------------------+-------------------+-----------------+ > | strip_vlan | 1.01x | 1.11x | > +-----------------------------------------------+-------------------+-----------------+ > | mod_ipv4 1 x field | 1.01x | 1.02x | > +-----------------------------------------------+-------------------+-----------------+ > | mod_ipv4 4 x fields | 1.01x | 1.21x | > +-----------------------------------------------+-------------------+-----------------+ > | strip_vlan + mod_dl_dst + mod_ipv4 4 x fields | 1.01x | 1.36x | > +-----------------------------------------------+-------------------+-----------------+ > > --- > 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 ;) 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. Cheers, Eelco
> -----Original Message----- > From: Eelco Chaudron <echaudro@redhat.com> > Sent: Thursday, July 14, 2022 1:55 PM > To: Van Haaren, Harry <harry.van.haaren@intel.com>; Finn, Emma > <emma.finn@intel.com> > Cc: dev@openvswitch.org; i.maximets@ovn.org; Amber, Kumar > <kumar.amber@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>; Stokes, Ian > <ian.stokes@intel.com> > 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! > Cheers, > > Eelco Thanks for keeping us updated to progress, -Harry
On 14 Jul 2022, at 15:04, Van Haaren, Harry wrote: >> -----Original Message----- >> From: Eelco Chaudron <echaudro@redhat.com> >> Sent: Thursday, July 14, 2022 1:55 PM >> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Finn, Emma >> <emma.finn@intel.com> >> Cc: dev@openvswitch.org; i.maximets@ovn.org; Amber, Kumar >> <kumar.amber@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>; Stokes, Ian >> <ian.stokes@intel.com> >> 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
> -----Original Message----- > From: Eelco Chaudron <echaudro@redhat.com> > Sent: Thursday 14 July 2022 14:27 > To: Van Haaren, Harry <harry.van.haaren@intel.com> > Cc: Finn, Emma <emma.finn@intel.com>; dev@openvswitch.org; > i.maximets@ovn.org; Amber, Kumar <kumar.amber@intel.com>; Pai G, Sunil > <sunil.pai.g@intel.com>; Stokes, Ian <ian.stokes@intel.com> > 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 <echaudro@redhat.com> > >> Sent: Thursday, July 14, 2022 1:55 PM > >> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Finn, Emma > >> <emma.finn@intel.com> > >> Cc: dev@openvswitch.org; i.maximets@ovn.org; Amber, Kumar > >> <kumar.amber@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>; > >> Stokes, Ian <ian.stokes@intel.com> > >> 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 Thanks, Emma
On 7/14/22 16:03, Finn, Emma wrote: > > >> -----Original Message----- >> From: Eelco Chaudron <echaudro@redhat.com> >> Sent: Thursday 14 July 2022 14:27 >> To: Van Haaren, Harry <harry.van.haaren@intel.com> >> Cc: Finn, Emma <emma.finn@intel.com>; dev@openvswitch.org; >> i.maximets@ovn.org; Amber, Kumar <kumar.amber@intel.com>; Pai G, Sunil >> <sunil.pai.g@intel.com>; Stokes, Ian <ian.stokes@intel.com> >> 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 <echaudro@redhat.com> >>>> Sent: Thursday, July 14, 2022 1:55 PM >>>> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Finn, Emma >>>> <emma.finn@intel.com> >>>> Cc: dev@openvswitch.org; i.maximets@ovn.org; Amber, Kumar >>>> <kumar.amber@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>; >>>> Stokes, Ian <ian.stokes@intel.com> >>>> 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.
> -----Original Message----- > From: Ilya Maximets <i.maximets@ovn.org> > Sent: Thursday, July 14, 2022 3:10 PM > To: Finn, Emma <emma.finn@intel.com>; Eelco Chaudron > <echaudro@redhat.com>; Van Haaren, Harry <harry.van.haaren@intel.com> > Cc: i.maximets@ovn.org; dev@openvswitch.org; Amber, Kumar > <kumar.amber@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>; Stokes, Ian > <ian.stokes@intel.com>; Flavio Leitner <fbl@sysclose.org> > Subject: Re: [PATCH v10 00/10] Actions Infrastructure + Optimizations > > On 7/14/22 16:03, Finn, Emma wrote: > > > > > >> -----Original Message----- > >> From: Eelco Chaudron <echaudro@redhat.com> > >> Sent: Thursday 14 July 2022 14:27 > >> To: Van Haaren, Harry <harry.van.haaren@intel.com> > >> Cc: Finn, Emma <emma.finn@intel.com>; dev@openvswitch.org; > >> i.maximets@ovn.org; Amber, Kumar <kumar.amber@intel.com>; Pai G, Sunil > >> <sunil.pai.g@intel.com>; Stokes, Ian <ian.stokes@intel.com> > >> 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 <echaudro@redhat.com> > >>>> Sent: Thursday, July 14, 2022 1:55 PM > >>>> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Finn, Emma > >>>> <emma.finn@intel.com> > >>>> Cc: dev@openvswitch.org; i.maximets@ovn.org; Amber, Kumar > >>>> <kumar.amber@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>; > >>>> Stokes, Ian <ian.stokes@intel.com> > >>>> 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. > 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
On 14 Jul 2022, at 16:22, Van Haaren, Harry wrote: >> -----Original Message----- >> From: Ilya Maximets <i.maximets@ovn.org> >> Sent: Thursday, July 14, 2022 3:10 PM >> To: Finn, Emma <emma.finn@intel.com>; Eelco Chaudron >> <echaudro@redhat.com>; Van Haaren, Harry <harry.van.haaren@intel.com> >> Cc: i.maximets@ovn.org; dev@openvswitch.org; Amber, Kumar >> <kumar.amber@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>; Stokes, Ian >> <ian.stokes@intel.com>; Flavio Leitner <fbl@sysclose.org> >> Subject: Re: [PATCH v10 00/10] Actions Infrastructure + Optimizations >> >> On 7/14/22 16:03, Finn, Emma wrote: >>> >>> >>>> -----Original Message----- >>>> From: Eelco Chaudron <echaudro@redhat.com> >>>> Sent: Thursday 14 July 2022 14:27 >>>> To: Van Haaren, Harry <harry.van.haaren@intel.com> >>>> Cc: Finn, Emma <emma.finn@intel.com>; dev@openvswitch.org; >>>> i.maximets@ovn.org; Amber, Kumar <kumar.amber@intel.com>; Pai G, Sunil >>>> <sunil.pai.g@intel.com>; Stokes, Ian <ian.stokes@intel.com> >>>> 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 <echaudro@redhat.com> >>>>>> Sent: Thursday, July 14, 2022 1:55 PM >>>>>> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Finn, Emma >>>>>> <emma.finn@intel.com> >>>>>> Cc: dev@openvswitch.org; i.maximets@ovn.org; Amber, Kumar >>>>>> <kumar.amber@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>; >>>>>> Stokes, Ian <ian.stokes@intel.com> >>>>>> 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
> -----Original Message----- > From: Eelco Chaudron <echaudro@redhat.com> > Sent: Thursday 14 July 2022 15:37 > To: Van Haaren, Harry <harry.van.haaren@intel.com> > Cc: Ilya Maximets <i.maximets@ovn.org>; Finn, Emma > <emma.finn@intel.com>; dev@openvswitch.org; Amber, Kumar > <kumar.amber@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>; Stokes, Ian > <ian.stokes@intel.com>; Flavio Leitner <fbl@sysclose.org> > 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 <i.maximets@ovn.org> > >> Sent: Thursday, July 14, 2022 3:10 PM > >> To: Finn, Emma <emma.finn@intel.com>; Eelco Chaudron > >> <echaudro@redhat.com>; Van Haaren, Harry > <harry.van.haaren@intel.com> > >> Cc: i.maximets@ovn.org; dev@openvswitch.org; Amber, Kumar > >> <kumar.amber@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>; > >> Stokes, Ian <ian.stokes@intel.com>; Flavio Leitner <fbl@sysclose.org> > >> Subject: Re: [PATCH v10 00/10] Actions Infrastructure + Optimizations > >> > >> On 7/14/22 16:03, Finn, Emma wrote: > >>> > >>> > >>>> -----Original Message----- > >>>> From: Eelco Chaudron <echaudro@redhat.com> > >>>> Sent: Thursday 14 July 2022 14:27 > >>>> To: Van Haaren, Harry <harry.van.haaren@intel.com> > >>>> Cc: Finn, Emma <emma.finn@intel.com>; dev@openvswitch.org; > >>>> i.maximets@ovn.org; Amber, Kumar <kumar.amber@intel.com>; Pai > G, > >>>> Sunil <sunil.pai.g@intel.com>; Stokes, Ian <ian.stokes@intel.com> > >>>> 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 <echaudro@redhat.com> > >>>>>> Sent: Thursday, July 14, 2022 1:55 PM > >>>>>> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Finn, Emma > >>>>>> <emma.finn@intel.com> > >>>>>> Cc: dev@openvswitch.org; i.maximets@ovn.org; Amber, Kumar > >>>>>> <kumar.amber@intel.com>; Pai G, Sunil <sunil.pai.g@intel.com>; > >>>>>> Stokes, Ian <ian.stokes@intel.com> > >>>>>> 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>
This patchset introduces actions infrastructure changes which allows the user to choose between different action implementations based on CPU ISA by using different commands. The infrastructure also provides a way to check the correctness of the ISA optimized action version against the scalar version. This series also introduces optimized versions of the following actions: - push_vlan - pop_vlan - set_masked eth - set_masked ipv4 Below is a table indicating some relative performance benefits for these actions. +-----------------------------------------------+-------------------+-----------------+ | Actions | Scalar with series| AVX with series | +-----------------------------------------------+-------------------+-----------------+ | mod_dl_dst | 1.01x | 1.13x | +-----------------------------------------------+-------------------+-----------------+ | push_vlan | 1.01x | 1.10x | +-----------------------------------------------+-------------------+-----------------+ | strip_vlan | 1.01x | 1.11x | +-----------------------------------------------+-------------------+-----------------+ | mod_ipv4 1 x field | 1.01x | 1.02x | +-----------------------------------------------+-------------------+-----------------+ | mod_ipv4 4 x fields | 1.01x | 1.21x | +-----------------------------------------------+-------------------+-----------------+ | strip_vlan + mod_dl_dst + mod_ipv4 4 x fields | 1.01x | 1.36x | +-----------------------------------------------+-------------------+-----------------+ --- 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) --- v9: - Moved avx512 probe and init functions to later patch. - Dependency on userspace datapath has been resolved. - Fixed up comments from Sunil as posted on v8 - Note: Harry is sending this patchset, but it is Emma's rework, except for rebasing to lastest git, and very minor fixups. --- v8 - First patch changing unit tests has been removed from series. - AVX checksum implementation has been reworked. --- v7: - Fix review comments from Eelco. --- v6: - Rebase to master - Add ISA implementation of set_masked eth and ipv4 actions - Fix incorrect checksums in input packets for ofproto-dpif unit tests --- v5: - Rebase to master - Minor change to variable names - Added Tags from Harry. --- v4: - Rebase to master - Add ISA implementation of push_vlan action --- v3: - Refactored to fix unit test failures - Removed some sign-off on commits --- v2: - Fix the CI build issues --- Emma Finn (8): odp-execute: Add function pointers to odp-execute for different action implementations. odp-execute: Add function pointer for pop_vlan action. odp-execute: Add auto validation function for actions. odp-execute: Add command to switch action implementation. odp-execute: Add ISA implementation of actions. odp-execute: Add ISA implementation of push_vlan action. odp-execute: Add ISA implementation of set_masked ETH odp-execute: Add ISA implementation of set_masked IPv4 action Harry van Haaren (1): odp-execute: Add ISA implementation of pop_vlan action. Kumar Amber (1): acinclude: Add configure option to enable actions autovalidator at build time. Documentation/topics/dpdk/bridge.rst | 30 ++ Documentation/topics/testing.rst | 24 +- NEWS | 7 + acinclude.m4 | 21 ++ configure.ac | 1 + lib/automake.mk | 7 + lib/cpu.c | 1 + lib/cpu.h | 1 + lib/dp-packet.c | 24 ++ lib/dp-packet.h | 4 + lib/odp-execute-avx512.c | 535 +++++++++++++++++++++++++++ lib/odp-execute-private.c | 270 ++++++++++++++ lib/odp-execute-private.h | 106 ++++++ lib/odp-execute-unixctl.man | 10 + lib/odp-execute.c | 202 ++++++++-- lib/odp-execute.h | 10 + m4/openvswitch.m4 | 29 ++ tests/ofproto-macros.at | 1 + tests/pmd.at | 39 ++ vswitchd/bridge.c | 3 + vswitchd/ovs-vswitchd.8.in | 1 + 21 files changed, 1280 insertions(+), 46 deletions(-) create mode 100644 lib/odp-execute-avx512.c create mode 100644 lib/odp-execute-private.c create mode 100644 lib/odp-execute-private.h create mode 100644 lib/odp-execute-unixctl.man