Wed, Oct 14, 2020 at 12:03:29PM CEST, olichtne(a)redhat.com wrote:
On Wed, Oct 14, 2020 at 11:07:55AM +0200, Jan Tluka wrote:
> Tue, Oct 13, 2020 at 10:28:58AM CEST, olichtne(a)redhat.com wrote:
> >On Mon, Oct 12, 2020 at 07:15:14PM +0200, Jan Tluka wrote:
> >> This class is a joint of SctpFirewallPerfTestMixin, DisableIdleStatesMixin
> >> and DisableTurboboostMixin classes for a user convenience.
> >>
> >> Signed-off-by: Jan Tluka <jtluka(a)redhat.com>
> >> ---
> >> .../ENRT/PerfTestMixins/CommonPerfTestTweakMixin.py | 9 +++++++++
> >> lnst/Recipes/ENRT/PerfTestMixins/__init__.py | 1 +
> >> 2 files changed, 10 insertions(+)
> >> create mode 100644
lnst/Recipes/ENRT/PerfTestMixins/CommonPerfTestTweakMixin.py
> >>
> >> diff --git a/lnst/Recipes/ENRT/PerfTestMixins/CommonPerfTestTweakMixin.py
b/lnst/Recipes/ENRT/PerfTestMixins/CommonPerfTestTweakMixin.py
> >> new file mode 100644
> >> index 00000000..3efff9f7
> >> --- /dev/null
> >> +++ b/lnst/Recipes/ENRT/PerfTestMixins/CommonPerfTestTweakMixin.py
> >> @@ -0,0 +1,9 @@
> >> +from lnst.Recipes.ENRT.PerfTestMixins import (
> >> + SctpFirewallPerfTestMixin,
> >> + DisableIdleStatesMixin,
> >> + DisableTurboboostMixin,
> >> + )
> >> +
> >> +class CommonPerfTestTweakMixin(SctpFirewallPerfTestMixin,
DisableTurboboostMixin,
> >> + DisableIdleStatesMixin):
> >> + pass
> >
> >I'm not sure if the DisableIdleStatesMixin and DisableTurboboostMixin
> >are actualy perf tweaks to be honest. To me they feel a little bit
> >different than the SctpFirewallPerfTestMixin, I think I would probably
> >have put them as generic Subconfiguration mixins.
> >
> >In the context of adding a "*host_list" I think it also makes a little
> >more sense as the mixins no longer need to access the Flow measurement
> >object anymore.
> >
> >What do you think?
> >
> >-Ondrej
> >
>
> I think that these mixins are required only for the performance test so
> it kind of makes sense to have it isolated just for this.
>
> But I have no problem with moving this to subconfig level.
>
> -Jan
I guess that's true, they shouldn't affect the functionality or
connectivity.
I was thinking from the opposite view - these mixins don't require any
of the additional context provided by the actual perf configuration -
which means that they're going to be applied and removed for each perf
test, but in exactly the same way.
On the other hand the SctpFirewallPerfTestMixin is applied based on
specifics defined by the flow.
This means that moving the Disable* mixins to the subconfig level will
"save" some amount of apply-remove cycles that were spent for the
multiple flow combinations generated.
-Ondrej
More thoughts on this.
Just to be clear, are you proposing to change the mixins to inherit from
the BaseSubConfigMixin? Or should this be a different concept?
Secondly, if we want to save the apply-remove cycles, it would make more
sense to move this between the test_wide configuration and
sub_configuration. With this there will be just one apply-remove cycle.
-Jan