Re: [PATCH 4/5] Recipes.ENRT.PerfTestMixins: add CommonPerfTestTweakMixin

[Jan Tluka]

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&gt;
> > ---
> >  .../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