On 7 Jul 2021, at 11:33, Van Haaren, Harry wrote:
>> -----Original Message----- >> From: Eelco Chaudron <[email protected]> >> Sent: Wednesday, July 7, 2021 9:59 AM >> To: Van Haaren, Harry <[email protected]> >> Cc: Amber, Kumar <[email protected]>; [email protected]; >> [email protected]; Flavio Leitner <[email protected]>; Stokes, Ian >> <[email protected]> >> Subject: Re: [ovs-dev] [v4 02/12] dpif-netdev: Add auto validation function >> for >> miniflow extract >> >> >> >> On 6 Jul 2021, at 15:58, Van Haaren, Harry wrote: >> >>>> -----Original Message----- >>>> From: Eelco Chaudron <[email protected]> >>>> Sent: Friday, July 2, 2021 8:10 AM >>>> To: Van Haaren, Harry <[email protected]> >>>> Cc: Amber, Kumar <[email protected]>; [email protected]; >>>> [email protected]; Flavio Leitner <[email protected]>; Stokes, Ian >>>> <[email protected]> >>>> Subject: Re: [ovs-dev] [v4 02/12] dpif-netdev: Add auto validation >>>> function for >>>> miniflow extract >>>> >>>> >>>> >>>> On 1 Jul 2021, at 19:24, Van Haaren, Harry wrote: >>>> >>>>>> -----Original Message----- >>>>>> From: Eelco Chaudron <[email protected]> >>> >>> <snip ascii table/data and previous conversations> >>> >>>>>> I’ll share the google sheet with you directly as it also has the config, >>>>>> and PVP >>>> results. >>>>> >>>>> I can't actually access that doc, sorry. Results above are enough to go >>>>> by for >> now :) >>>> >>>> It’s attached. >>> >>> Thanks. >>> >>>>> We can investigate if there's any optimizations to be done to improve the >> scalar DPIF >>>>> enabling of the miniflow extract func ptr, but I'm not sure there is. >>> >>> Note the v6 of MFEX has some minor changes/optimizations in place, as per >> scalar DPIF enabling in this patch: >>> >> https://patchwork.ozlabs.org/project/openvswitch/patch/20210706131150.45513 >> [email protected]/ >>> >>> >>>>> If we cannot improve the perf data from above, there is an option to not >> enable >>>> the scalar DPIF with the AVX512 MFEX optimizations. (Logic being if AVX512 >>>> is >> present, >>>> running both the DPIF + MFEX makes sense). What do you think? >>> >>> If you feel it is required before merge, would you re-run the benchmark on >>> v6? >>> If so, we're targeting Thursday for merge, so data ASAP, or by EOD tomorrow >> would be required. >> >> I’m reviewing your v6 now, so I have no cycles to also do the testing before >> the >> end of the week. But the tests are simple, so maybe you guys can try it and >> report the difference with and without the two patchsets applied on a non >> AVX512 machine? > > Yes, we have done scalar-only code benchmarking of master vs with DPIF > patchset. > By not enabling AVX512 at runtime we get the "non AVX512 machine" behaviour. > (All the scalar code is common, no need to a specific CPU in that instance). > > Testing OVS master branch vs with patchset did not show up any performance > delta > on the test machines here, so there's nothing I can do. > > By removing scalar DPIF enabling of MFEX opt pointer (details below) we > remove any > urgency on benchmark results? I’ll wrap up the review first, and hopefully, when you are working on potential changes, I can run the tests and get some results. I understand now what you meant with disabling it in the scalar part, so if I still see 1%+ deltas I’ll try it out. >>> As mentioned above, there is an option to remove the AVX512-Optimized >> MFEX enabling >>> from the scalar datapath, if there is measurable/significant performance >> reduction in this v6 code. >> >> It not clear to me what you mean by this? Can you elaborate? I’m running >> this on >> a non AVX512 machine, all with default configs. > > I'm suggesting that if you're not OK with merging the ~1.x% negative > performance on scalar > DPIF performance to enable MFEX, we can remove the MFEX enabling from the > scalar DPIF. > > Logically, if AVX512 is in use for MFEX, it is logical to use the AVX512 DPIF > too, hence > this is a workable solution/workaround for scalar DPIF performance loss. > > Taking this approach would ensure that scalar DPIF performance is not reduced > in > this release, and we can re-visit scalar DPIF enabling of MFEX in future if > desired? > > Overall, this seems the pragmatic way of reducing risk around performance and > getting merged. > > >>>> This is on a system without AVX512 support, so all is disabled. The >>>> “without >> patch” >>>> has both the new AVX patches removed (mfex and dpif framework). >>>> >>>>> >>>>>> //Eelco >>> >>> Thanks again for testing & follow up! Regards, -Harry _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
