[PATCH 1/5] Recipes.ENRT.PerfTestMixins.SctpFirewallPerfTestMixin: move _get_flow_measurement_from_config() to base class
This method will be reused by other mixin classes.
Signed-off-by: Jan Tluka jtluka@redhat.com --- lnst/Recipes/ENRT/PerfTestMixins/BasePerfTestTweakMixin.py | 6 ++++++ .../ENRT/PerfTestMixins/SctpFirewallPerfTestMixin.py | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/lnst/Recipes/ENRT/PerfTestMixins/BasePerfTestTweakMixin.py b/lnst/Recipes/ENRT/PerfTestMixins/BasePerfTestTweakMixin.py index 380fc72e..5e008a68 100644 --- a/lnst/Recipes/ENRT/PerfTestMixins/BasePerfTestTweakMixin.py +++ b/lnst/Recipes/ENRT/PerfTestMixins/BasePerfTestTweakMixin.py @@ -1,3 +1,5 @@ +from lnst.RecipeCommon.Perf.Measurements.BaseFlowMeasurement import BaseFlowMeasurement + class BasePerfTestTweakMixin(object): """ This is a base class that defines common API for specific *perf test* @@ -13,3 +15,7 @@ class BasePerfTestTweakMixin(object): def remove_perf_test_tweak(self, perf_config): # TODO: check if anything left in the perf_config.perf_test_tweak_config pass + + def _get_flow_measurement_from_config(self, perf_config): + flow_measurements = [ m for m in perf_config.measurements if isinstance(m, BaseFlowMeasurement) ] + return flow_measurements[0] diff --git a/lnst/Recipes/ENRT/PerfTestMixins/SctpFirewallPerfTestMixin.py b/lnst/Recipes/ENRT/PerfTestMixins/SctpFirewallPerfTestMixin.py index 6c6cc416..c0520405 100644 --- a/lnst/Recipes/ENRT/PerfTestMixins/SctpFirewallPerfTestMixin.py +++ b/lnst/Recipes/ENRT/PerfTestMixins/SctpFirewallPerfTestMixin.py @@ -1,13 +1,7 @@ from lnst.Controller.RecipeResults import ResultLevel from lnst.Recipes.ENRT.PerfTestMixins import BasePerfTestTweakMixin -from lnst.RecipeCommon.Perf.Measurements.BaseFlowMeasurement import BaseFlowMeasurement
class SctpFirewallPerfTestMixin(BasePerfTestTweakMixin): - - def _get_flow_measurement_from_config(self, perf_config): - flow_measurements = [ m for m in perf_config.measurements if isinstance(m, BaseFlowMeasurement) ] - return flow_measurements[0] - def apply_perf_test_tweak(self, perf_config): super().apply_perf_test_tweak(perf_config)
This mixin class provides recipe parameter 'disable_idlestates' that can be used to control the CPU idle states before running a performance test through BaseEnrtRecipe.do_perf_tests().
The value of the parameter is passed as the latency argument (-D) of the 'cpupower idle-set' that will disable all idle states with a equal or higher latency than the specified value.
If the value is 0 this will effectively disable all CPU idle states.
Signed-off-by: Jan Tluka jtluka@redhat.com --- .../PerfTestMixins/DisableIdleStatesMixin.py | 42 +++++++++++++++++++ lnst/Recipes/ENRT/PerfTestMixins/__init__.py | 1 + 2 files changed, 43 insertions(+) create mode 100644 lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py
diff --git a/lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py b/lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py new file mode 100644 index 00000000..4281016f --- /dev/null +++ b/lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py @@ -0,0 +1,42 @@ +from lnst.Common.Parameters import IntParam +from lnst.Recipes.ENRT.PerfTestMixins import BasePerfTestTweakMixin + +class DisableIdleStatesMixin(BasePerfTestTweakMixin): + disable_idlestates = IntParam(default=-1) + + def generate_perf_test_tweak_description(self, perf_config): + description = super().generate_perf_test_tweak_description(perf_config) + latency = self.params.disable_idlestates + tweak_config = perf_config.perf_test_tweak_config + if 'idlestates' in tweak_config: + for host in tweak_config['idlestates']['hosts']: + desc = "disabled idle states with latency higher than {} "\ + "on host {}".format(latency, host.hostid) + description.append(desc) + else: + description.append("skipped disabling idle states") + + return description + + def apply_perf_test_tweak(self, perf_config): + super().apply_perf_test_tweak(perf_config) + latency = self.params.disable_idlestates + if latency != -1: + tweak_config = perf_config.perf_test_tweak_config + tweak_config['idlestates'] = {'hosts': []} + + flow_measurement = self._get_flow_measurement_from_config(perf_config) + flow = flow_measurement.conf[0] + for host in [flow.generator, flow.receiver]: + # TODO: save previous state + host.run("cpupower idle-set -D {}".format(latency)) + tweak_config['idlestates']['hosts'].append(host) + + def remove_perf_test_tweak(self, perf_config): + tweak_config = perf_config.perf_test_tweak_config + if 'idlestates' in tweak_config: + for host in tweak_config['idlestates']['hosts']: + host.run("cpupower idle-set -E") + del tweak_config['idlestates'] + + super().remove_perf_test_tweak(perf_config) diff --git a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py index 9f320118..fc5b4747 100644 --- a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py +++ b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py @@ -1,2 +1,3 @@ from lnst.Recipes.ENRT.PerfTestMixins.BasePerfTestTweakMixin import BasePerfTestTweakMixin from lnst.Recipes.ENRT.PerfTestMixins.SctpFirewallPerfTestMixin import SctpFirewallPerfTestMixin +from lnst.Recipes.ENRT.PerfTestMixins.DisableIdleStatesMixin import DisableIdleStatesMixin
On Mon, Oct 12, 2020 at 07:15:12PM +0200, Jan Tluka wrote:
This mixin class provides recipe parameter 'disable_idlestates' that can be used to control the CPU idle states before running a performance test through BaseEnrtRecipe.do_perf_tests().
The value of the parameter is passed as the latency argument (-D) of the 'cpupower idle-set' that will disable all idle states with a equal or higher latency than the specified value.
If the value is 0 this will effectively disable all CPU idle states.
Signed-off-by: Jan Tluka jtluka@redhat.com
.../PerfTestMixins/DisableIdleStatesMixin.py | 42 +++++++++++++++++++ lnst/Recipes/ENRT/PerfTestMixins/__init__.py | 1 + 2 files changed, 43 insertions(+) create mode 100644 lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py
diff --git a/lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py b/lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py new file mode 100644 index 00000000..4281016f --- /dev/null +++ b/lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py @@ -0,0 +1,42 @@ +from lnst.Common.Parameters import IntParam +from lnst.Recipes.ENRT.PerfTestMixins import BasePerfTestTweakMixin
+class DisableIdleStatesMixin(BasePerfTestTweakMixin):
- disable_idlestates = IntParam(default=-1)
You don't need to set a "special" value to indicate that the parameter wasn't set.
If the parameter is not mandatory and you don't provide a value for it on Recipe initialization, the parameter isn't available during test execution, e.g.:
class Recipe(): optional_param = IntParam(mandatory=False)
def test(self): if "optional_param" in self.params: configure(self.params.optional_param)
- def generate_perf_test_tweak_description(self, perf_config):
description = super().generate_perf_test_tweak_description(perf_config)latency = self.params.disable_idlestatestweak_config = perf_config.perf_test_tweak_configif 'idlestates' in tweak_config:for host in tweak_config['idlestates']['hosts']:desc = "disabled idle states with latency higher than {} "\"on host {}".format(latency, host.hostid)description.append(desc)else:description.append("skipped disabling idle states")return description- def apply_perf_test_tweak(self, perf_config):
super().apply_perf_test_tweak(perf_config)latency = self.params.disable_idlestates
I'm not sure about the name of the parameter... why call it "disable_idlestates" when in reality it refers to the latency to be used? shouldn't it somehow be reflected in the name of the parameter itself?
if latency != -1:tweak_config = perf_config.perf_test_tweak_configtweak_config['idlestates'] = {'hosts': []}flow_measurement = self._get_flow_measurement_from_config(perf_config)flow = flow_measurement.conf[0]for host in [flow.generator, flow.receiver]:
I'm wondering how this will work for Virtual recipes, the generator and receiver in those cases refer to the guest vms, can we also disable idlestates for virtual cores?
Should we consider disabling them for the hypervisor hosts?
-Ondrej
# TODO: save previous statehost.run("cpupower idle-set -D {}".format(latency))tweak_config['idlestates']['hosts'].append(host)- def remove_perf_test_tweak(self, perf_config):
tweak_config = perf_config.perf_test_tweak_configif 'idlestates' in tweak_config:for host in tweak_config['idlestates']['hosts']:host.run("cpupower idle-set -E")del tweak_config['idlestates']super().remove_perf_test_tweak(perf_config)diff --git a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py index 9f320118..fc5b4747 100644 --- a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py +++ b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py @@ -1,2 +1,3 @@ from lnst.Recipes.ENRT.PerfTestMixins.BasePerfTestTweakMixin import BasePerfTestTweakMixin from lnst.Recipes.ENRT.PerfTestMixins.SctpFirewallPerfTestMixin import SctpFirewallPerfTestMixin
+from lnst.Recipes.ENRT.PerfTestMixins.DisableIdleStatesMixin import DisableIdleStatesMixin
2.21.3 _______________________________________________ LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org To unsubscribe send an email to lnst-developers-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedorahos...
Tue, Oct 13, 2020 at 10:21:27AM CEST, olichtne@redhat.com wrote:
On Mon, Oct 12, 2020 at 07:15:12PM +0200, Jan Tluka wrote:
This mixin class provides recipe parameter 'disable_idlestates' that can be used to control the CPU idle states before running a performance test through BaseEnrtRecipe.do_perf_tests().
The value of the parameter is passed as the latency argument (-D) of the 'cpupower idle-set' that will disable all idle states with a equal or higher latency than the specified value.
If the value is 0 this will effectively disable all CPU idle states.
Signed-off-by: Jan Tluka jtluka@redhat.com
.../PerfTestMixins/DisableIdleStatesMixin.py | 42 +++++++++++++++++++ lnst/Recipes/ENRT/PerfTestMixins/__init__.py | 1 + 2 files changed, 43 insertions(+) create mode 100644 lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py
diff --git a/lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py b/lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py new file mode 100644 index 00000000..4281016f --- /dev/null +++ b/lnst/Recipes/ENRT/PerfTestMixins/DisableIdleStatesMixin.py @@ -0,0 +1,42 @@ +from lnst.Common.Parameters import IntParam +from lnst.Recipes.ENRT.PerfTestMixins import BasePerfTestTweakMixin
+class DisableIdleStatesMixin(BasePerfTestTweakMixin):
- disable_idlestates = IntParam(default=-1)
You don't need to set a "special" value to indicate that the parameter wasn't set.
If the parameter is not mandatory and you don't provide a value for it on Recipe initialization, the parameter isn't available during test execution, e.g.:
class Recipe(): optional_param = IntParam(mandatory=False)
def test(self): if "optional_param" in self.params: configure(self.params.optional_param)
I'll fix this.
- def generate_perf_test_tweak_description(self, perf_config):
description = super().generate_perf_test_tweak_description(perf_config)latency = self.params.disable_idlestatestweak_config = perf_config.perf_test_tweak_configif 'idlestates' in tweak_config:for host in tweak_config['idlestates']['hosts']:desc = "disabled idle states with latency higher than {} "\"on host {}".format(latency, host.hostid)description.append(desc)else:description.append("skipped disabling idle states")return description- def apply_perf_test_tweak(self, perf_config):
super().apply_perf_test_tweak(perf_config)latency = self.params.disable_idlestatesI'm not sure about the name of the parameter... why call it "disable_idlestates" when in reality it refers to the latency to be used? shouldn't it somehow be reflected in the name of the parameter itself?
Yeah, probably 'minimal_idlestates_latency' is better.
if latency != -1:tweak_config = perf_config.perf_test_tweak_configtweak_config['idlestates'] = {'hosts': []}flow_measurement = self._get_flow_measurement_from_config(perf_config)flow = flow_measurement.conf[0]for host in [flow.generator, flow.receiver]:I'm wondering how this will work for Virtual recipes, the generator and receiver in those cases refer to the guest vms, can we also disable idlestates for virtual cores?
No idea. I'll have to check.
Should we consider disabling them for the hypervisor hosts?
Very likely. I'll work on enhancing this and send v2.
-Ondrej
# TODO: save previous statehost.run("cpupower idle-set -D {}".format(latency))tweak_config['idlestates']['hosts'].append(host)- def remove_perf_test_tweak(self, perf_config):
tweak_config = perf_config.perf_test_tweak_configif 'idlestates' in tweak_config:for host in tweak_config['idlestates']['hosts']:host.run("cpupower idle-set -E")del tweak_config['idlestates']super().remove_perf_test_tweak(perf_config)diff --git a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py index 9f320118..fc5b4747 100644 --- a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py +++ b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py @@ -1,2 +1,3 @@ from lnst.Recipes.ENRT.PerfTestMixins.BasePerfTestTweakMixin import BasePerfTestTweakMixin from lnst.Recipes.ENRT.PerfTestMixins.SctpFirewallPerfTestMixin import SctpFirewallPerfTestMixin
+from lnst.Recipes.ENRT.PerfTestMixins.DisableIdleStatesMixin import DisableIdleStatesMixin
2.21.3 _______________________________________________ LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org To unsubscribe send an email to lnst-developers-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedorahos...
This mixin class provides recipe parameter 'disable_turboboost' that can be used to disable CPU turboboost before running a performance test through BaseEnrtRecipe.do_perf_tests().
It uses intel_pstate sysfs interface to disable the CPU feature and so it is usable only by systems with Intel CPUs.
To use it pass 'disable_turboost=True' parameter when creating an instance of BaseEnrtRecipe.
Signed-off-by: Jan Tluka jtluka@redhat.com --- .../PerfTestMixins/DisableTurboboostMixin.py | 55 +++++++++++++++++++ lnst/Recipes/ENRT/PerfTestMixins/__init__.py | 1 + 2 files changed, 56 insertions(+) create mode 100644 lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py
diff --git a/lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py b/lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py new file mode 100644 index 00000000..8beec018 --- /dev/null +++ b/lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py @@ -0,0 +1,55 @@ +from lnst.Common.Parameters import BoolParam +from lnst.Recipes.ENRT.PerfTestMixins import BasePerfTestTweakMixin + +class DisableTurboboostMixin(BasePerfTestTweakMixin): + disable_turboboost = BoolParam(default=False) + + def generate_perf_test_tweak_description(self, perf_config): + description = super().generate_perf_test_tweak_description(perf_config) + tweak_config = perf_config.perf_test_tweak_config + if 'turboboost' in tweak_config: + for host, host_data in tweak_config['turboboost'].items(): + description.append(host_data['desc']) + else: + description.append("skipped disabling turboboost through intel_pstate") + + return description + + def _is_turboboost_supported(self, host): + file_check = host.run("ls /sys/devices/system/cpu/intel_pstate/no_turbo") + return file_check.passed + + def apply_perf_test_tweak(self, perf_config): + super().apply_perf_test_tweak(perf_config) + if self.params.disable_turboboost: + tweak_config = perf_config.perf_test_tweak_config + tweak_config['turboboost'] = {} + + flow_measurement = self._get_flow_measurement_from_config(perf_config) + flow = flow_measurement.conf[0] + for host in [flow.generator, flow.receiver]: + if self._is_turboboost_supported(host): + # TODO: save previous state + host.run("echo 1 > /sys/devices/system/cpu/intel_pstate/no_turbo") + desc = "disabled turboboost through intel_pstate on {}".format(host.hostid) + turboboost_status = 'disabled' + else: + desc = "warning: user requested to disable turboboost "\ + "through intel_pstate but the sysfs file is not available "\ + "on host {}".format(host.hostid) + turboboost_status = 'unsupported' + tweak_config['turboboost'][host] = { + 'desc': desc, + 'status': turboboost_status, + } + + def remove_perf_test_tweak(self, perf_config): + tweak_config = perf_config.perf_test_tweak_config + if 'turboboost' in tweak_config: + for host, host_data in tweak_config['turboboost'].items(): + if host_data['status'] == 'disabled': + host.run("echo 0 > /sys/devices/system/cpu/intel_pstate/no_turbo") + + del tweak_config['turboboost'] + + super().remove_perf_test_tweak(perf_config) diff --git a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py index fc5b4747..cbc9eda8 100644 --- a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py +++ b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py @@ -1,3 +1,4 @@ from lnst.Recipes.ENRT.PerfTestMixins.BasePerfTestTweakMixin import BasePerfTestTweakMixin from lnst.Recipes.ENRT.PerfTestMixins.SctpFirewallPerfTestMixin import SctpFirewallPerfTestMixin from lnst.Recipes.ENRT.PerfTestMixins.DisableIdleStatesMixin import DisableIdleStatesMixin +from lnst.Recipes.ENRT.PerfTestMixins.DisableTurboboostMixin import DisableTurboboostMixin
On Mon, Oct 12, 2020 at 07:15:13PM +0200, Jan Tluka wrote:
This mixin class provides recipe parameter 'disable_turboboost' that can be used to disable CPU turboboost before running a performance test through BaseEnrtRecipe.do_perf_tests().
It uses intel_pstate sysfs interface to disable the CPU feature and so it is usable only by systems with Intel CPUs.
To use it pass 'disable_turboost=True' parameter when creating an instance of BaseEnrtRecipe.
Signed-off-by: Jan Tluka jtluka@redhat.com
.../PerfTestMixins/DisableTurboboostMixin.py | 55 +++++++++++++++++++ lnst/Recipes/ENRT/PerfTestMixins/__init__.py | 1 + 2 files changed, 56 insertions(+) create mode 100644 lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py
diff --git a/lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py b/lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py new file mode 100644 index 00000000..8beec018 --- /dev/null +++ b/lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py @@ -0,0 +1,55 @@ +from lnst.Common.Parameters import BoolParam +from lnst.Recipes.ENRT.PerfTestMixins import BasePerfTestTweakMixin
+class DisableTurboboostMixin(BasePerfTestTweakMixin):
- disable_turboboost = BoolParam(default=False)
- def generate_perf_test_tweak_description(self, perf_config):
description = super().generate_perf_test_tweak_description(perf_config)tweak_config = perf_config.perf_test_tweak_configif 'turboboost' in tweak_config:for host, host_data in tweak_config['turboboost'].items():description.append(host_data['desc'])else:description.append("skipped disabling turboboost through intel_pstate")return description- def _is_turboboost_supported(self, host):
file_check = host.run("ls /sys/devices/system/cpu/intel_pstate/no_turbo")return file_check.passed- def apply_perf_test_tweak(self, perf_config):
super().apply_perf_test_tweak(perf_config)if self.params.disable_turboboost:tweak_config = perf_config.perf_test_tweak_configtweak_config['turboboost'] = {}flow_measurement = self._get_flow_measurement_from_config(perf_config)flow = flow_measurement.conf[0]for host in [flow.generator, flow.receiver]:
Same comment as for previous patch - how should this be handled for Virtual recipes?
maybe a "disable_turboboost_host_list" property could work for these cases?
if self._is_turboboost_supported(host):# TODO: save previous statehost.run("echo 1 > /sys/devices/system/cpu/intel_pstate/no_turbo")desc = "disabled turboboost through intel_pstate on {}".format(host.hostid)turboboost_status = 'disabled'else:desc = "warning: user requested to disable turboboost "\"through intel_pstate but the sysfs file is not available "\"on host {}".format(host.hostid)turboboost_status = 'unsupported'tweak_config['turboboost'][host] = {'desc': desc,'status': turboboost_status,}- def remove_perf_test_tweak(self, perf_config):
tweak_config = perf_config.perf_test_tweak_configif 'turboboost' in tweak_config:for host, host_data in tweak_config['turboboost'].items():if host_data['status'] == 'disabled':host.run("echo 0 > /sys/devices/system/cpu/intel_pstate/no_turbo")
whitespace here
-Ondrej
del tweak_config['turboboost']super().remove_perf_test_tweak(perf_config)diff --git a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py index fc5b4747..cbc9eda8 100644 --- a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py +++ b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py @@ -1,3 +1,4 @@ from lnst.Recipes.ENRT.PerfTestMixins.BasePerfTestTweakMixin import BasePerfTestTweakMixin from lnst.Recipes.ENRT.PerfTestMixins.SctpFirewallPerfTestMixin import SctpFirewallPerfTestMixin from lnst.Recipes.ENRT.PerfTestMixins.DisableIdleStatesMixin import DisableIdleStatesMixin
+from lnst.Recipes.ENRT.PerfTestMixins.DisableTurboboostMixin import DisableTurboboostMixin
2.21.3 _______________________________________________ LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org To unsubscribe send an email to lnst-developers-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedorahos...
Tue, Oct 13, 2020 at 10:27:08AM CEST, olichtne@redhat.com wrote:
On Mon, Oct 12, 2020 at 07:15:13PM +0200, Jan Tluka wrote:
This mixin class provides recipe parameter 'disable_turboboost' that can be used to disable CPU turboboost before running a performance test through BaseEnrtRecipe.do_perf_tests().
It uses intel_pstate sysfs interface to disable the CPU feature and so it is usable only by systems with Intel CPUs.
To use it pass 'disable_turboost=True' parameter when creating an instance of BaseEnrtRecipe.
Signed-off-by: Jan Tluka jtluka@redhat.com
.../PerfTestMixins/DisableTurboboostMixin.py | 55 +++++++++++++++++++ lnst/Recipes/ENRT/PerfTestMixins/__init__.py | 1 + 2 files changed, 56 insertions(+) create mode 100644 lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py
diff --git a/lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py b/lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py new file mode 100644 index 00000000..8beec018 --- /dev/null +++ b/lnst/Recipes/ENRT/PerfTestMixins/DisableTurboboostMixin.py @@ -0,0 +1,55 @@ +from lnst.Common.Parameters import BoolParam +from lnst.Recipes.ENRT.PerfTestMixins import BasePerfTestTweakMixin
+class DisableTurboboostMixin(BasePerfTestTweakMixin):
- disable_turboboost = BoolParam(default=False)
- def generate_perf_test_tweak_description(self, perf_config):
description = super().generate_perf_test_tweak_description(perf_config)tweak_config = perf_config.perf_test_tweak_configif 'turboboost' in tweak_config:for host, host_data in tweak_config['turboboost'].items():description.append(host_data['desc'])else:description.append("skipped disabling turboboost through intel_pstate")return description- def _is_turboboost_supported(self, host):
file_check = host.run("ls /sys/devices/system/cpu/intel_pstate/no_turbo")return file_check.passed- def apply_perf_test_tweak(self, perf_config):
super().apply_perf_test_tweak(perf_config)if self.params.disable_turboboost:tweak_config = perf_config.perf_test_tweak_configtweak_config['turboboost'] = {}flow_measurement = self._get_flow_measurement_from_config(perf_config)flow = flow_measurement.conf[0]for host in [flow.generator, flow.receiver]:Same comment as for previous patch - how should this be handled for Virtual recipes?
maybe a "disable_turboboost_host_list" property could work for these cases?
Generally it is a good idea, I'm just wondering what the user should pass as the argument value.
I guess they should be strings matching hosts in self.matched.*.
So for example in case of VirtualBridgeVlansOverBondRecipe, which defines:
host1 = HostReq() ... host2 = HostReq()
this could be:
disable_turboboost_host_list=['host1, host2']
Or is there a better way how to deal with this? I believe there's no other test machine representation than that.
if self._is_turboboost_supported(host):# TODO: save previous statehost.run("echo 1 > /sys/devices/system/cpu/intel_pstate/no_turbo")desc = "disabled turboboost through intel_pstate on {}".format(host.hostid)turboboost_status = 'disabled'else:desc = "warning: user requested to disable turboboost "\"through intel_pstate but the sysfs file is not available "\"on host {}".format(host.hostid)turboboost_status = 'unsupported'tweak_config['turboboost'][host] = {'desc': desc,'status': turboboost_status,}- def remove_perf_test_tweak(self, perf_config):
tweak_config = perf_config.perf_test_tweak_configif 'turboboost' in tweak_config:for host, host_data in tweak_config['turboboost'].items():if host_data['status'] == 'disabled':host.run("echo 0 > /sys/devices/system/cpu/intel_pstate/no_turbo")whitespace here
Thanks for catching this!
-Jan
-Ondrej
del tweak_config['turboboost']super().remove_perf_test_tweak(perf_config)diff --git a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py index fc5b4747..cbc9eda8 100644 --- a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py +++ b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py @@ -1,3 +1,4 @@ from lnst.Recipes.ENRT.PerfTestMixins.BasePerfTestTweakMixin import BasePerfTestTweakMixin from lnst.Recipes.ENRT.PerfTestMixins.SctpFirewallPerfTestMixin import SctpFirewallPerfTestMixin from lnst.Recipes.ENRT.PerfTestMixins.DisableIdleStatesMixin import DisableIdleStatesMixin
+from lnst.Recipes.ENRT.PerfTestMixins.DisableTurboboostMixin import DisableTurboboostMixin
2.21.3 _______________________________________________ LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org To unsubscribe send an email to lnst-developers-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedorahos...
On Wed, Oct 14, 2020 at 11:26:29AM +0200, Jan Tluka wrote:
Tue, Oct 13, 2020 at 10:27:08AM CEST, olichtne@redhat.com wrote:
On Mon, Oct 12, 2020 at 07:15:13PM +0200, Jan Tluka wrote:
- def apply_perf_test_tweak(self, perf_config):
super().apply_perf_test_tweak(perf_config)if self.params.disable_turboboost:tweak_config = perf_config.perf_test_tweak_configtweak_config['turboboost'] = {}flow_measurement = self._get_flow_measurement_from_config(perf_config)flow = flow_measurement.conf[0]for host in [flow.generator, flow.receiver]:Same comment as for previous patch - how should this be handled for Virtual recipes?
maybe a "disable_turboboost_host_list" property could work for these cases?
Generally it is a good idea, I'm just wondering what the user should pass as the argument value.
I guess they should be strings matching hosts in self.matched.*.
So for example in case of VirtualBridgeVlansOverBondRecipe, which defines:
host1 = HostReq() ... host2 = HostReq()this could be:
disable_turboboost_host_list=['host1, host2']Or is there a better way how to deal with this? I believe there's no other test machine representation than that.
Why not the objects from the self.matched.* directly? e.g.:
@property def disable_turboboost_host_list(self): return [self.matched.m1, self.matched.m2, ...]
These are the same object references as flow.generator, but self.matched contains all of them, and the property can select the relevant ones.
-Ondrej
Wed, Oct 14, 2020 at 11:53:11AM CEST, olichtne@redhat.com wrote:
On Wed, Oct 14, 2020 at 11:26:29AM +0200, Jan Tluka wrote:
Tue, Oct 13, 2020 at 10:27:08AM CEST, olichtne@redhat.com wrote:
On Mon, Oct 12, 2020 at 07:15:13PM +0200, Jan Tluka wrote:
- def apply_perf_test_tweak(self, perf_config):
super().apply_perf_test_tweak(perf_config)if self.params.disable_turboboost:tweak_config = perf_config.perf_test_tweak_configtweak_config['turboboost'] = {}flow_measurement = self._get_flow_measurement_from_config(perf_config)flow = flow_measurement.conf[0]for host in [flow.generator, flow.receiver]:Same comment as for previous patch - how should this be handled for Virtual recipes?
maybe a "disable_turboboost_host_list" property could work for these cases?
Generally it is a good idea, I'm just wondering what the user should pass as the argument value.
I guess they should be strings matching hosts in self.matched.*.
So for example in case of VirtualBridgeVlansOverBondRecipe, which defines:
host1 = HostReq() ... host2 = HostReq()this could be:
disable_turboboost_host_list=['host1, host2']Or is there a better way how to deal with this? I believe there's no other test machine representation than that.
Why not the objects from the self.matched.* directly? e.g.:
@property def disable_turboboost_host_list(self): return [self.matched.m1, self.matched.m2, ...]
These are the same object references as flow.generator, but self.matched contains all of them, and the property can select the relevant ones.
-Ondrej
My question was about how to pass this as a recipe parameter while creating the instance.
r = VirtualBridgeVlansOverBondRecipe( disable_turboboost_host_list=['host1, host2'] )
But solving this through a property is better.
-Jan
On Wed, Oct 14, 2020 at 12:10:46PM +0200, Jan Tluka wrote:
Wed, Oct 14, 2020 at 11:53:11AM CEST, olichtne@redhat.com wrote:
On Wed, Oct 14, 2020 at 11:26:29AM +0200, Jan Tluka wrote:
Tue, Oct 13, 2020 at 10:27:08AM CEST, olichtne@redhat.com wrote:
On Mon, Oct 12, 2020 at 07:15:13PM +0200, Jan Tluka wrote:
- def apply_perf_test_tweak(self, perf_config):
super().apply_perf_test_tweak(perf_config)if self.params.disable_turboboost:tweak_config = perf_config.perf_test_tweak_configtweak_config['turboboost'] = {}flow_measurement = self._get_flow_measurement_from_config(perf_config)flow = flow_measurement.conf[0]for host in [flow.generator, flow.receiver]:Same comment as for previous patch - how should this be handled for Virtual recipes?
maybe a "disable_turboboost_host_list" property could work for these cases?
Generally it is a good idea, I'm just wondering what the user should pass as the argument value.
I guess they should be strings matching hosts in self.matched.*.
So for example in case of VirtualBridgeVlansOverBondRecipe, which defines:
host1 = HostReq() ... host2 = HostReq()this could be:
disable_turboboost_host_list=['host1, host2']Or is there a better way how to deal with this? I believe there's no other test machine representation than that.
Why not the objects from the self.matched.* directly? e.g.:
@property def disable_turboboost_host_list(self): return [self.matched.m1, self.matched.m2, ...]
These are the same object references as flow.generator, but self.matched contains all of them, and the property can select the relevant ones.
-Ondrej
My question was about how to pass this as a recipe parameter while creating the instance.
r = VirtualBridgeVlansOverBondRecipe( disable_turboboost_host_list=['host1, host2'] )
But solving this through a property is better.
-Jan
Ah, not sure if I would expose that as a parameter, maybe just a boolean of disable/enable turboboosts, but "which hosts" I would keep on the implementation of the property itself.
-Ondrej
This class is a joint of SctpFirewallPerfTestMixin, DisableIdleStatesMixin and DisableTurboboostMixin classes for a user convenience.
Signed-off-by: Jan Tluka jtluka@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 diff --git a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py index cbc9eda8..431135ac 100644 --- a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py +++ b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py @@ -2,3 +2,4 @@ from lnst.Recipes.ENRT.PerfTestMixins.BasePerfTestTweakMixin import BasePerfTest from lnst.Recipes.ENRT.PerfTestMixins.SctpFirewallPerfTestMixin import SctpFirewallPerfTestMixin from lnst.Recipes.ENRT.PerfTestMixins.DisableIdleStatesMixin import DisableIdleStatesMixin from lnst.Recipes.ENRT.PerfTestMixins.DisableTurboboostMixin import DisableTurboboostMixin +from lnst.Recipes.ENRT.PerfTestMixins.CommonPerfTestTweakMixin import CommonPerfTestTweakMixin
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@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
diff --git a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py index cbc9eda8..431135ac 100644 --- a/lnst/Recipes/ENRT/PerfTestMixins/__init__.py +++ b/lnst/Recipes/ENRT/PerfTestMixins/__init__.py @@ -2,3 +2,4 @@ from lnst.Recipes.ENRT.PerfTestMixins.BasePerfTestTweakMixin import BasePerfTest from lnst.Recipes.ENRT.PerfTestMixins.SctpFirewallPerfTestMixin import SctpFirewallPerfTestMixin from lnst.Recipes.ENRT.PerfTestMixins.DisableIdleStatesMixin import DisableIdleStatesMixin from lnst.Recipes.ENRT.PerfTestMixins.DisableTurboboostMixin import DisableTurboboostMixin
+from lnst.Recipes.ENRT.PerfTestMixins.CommonPerfTestTweakMixin import CommonPerfTestTweakMixin
2.21.3 _______________________________________________ LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org To unsubscribe send an email to lnst-developers-leave@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedorahos...
Tue, Oct 13, 2020 at 10:28:58AM CEST, olichtne@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@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
On Wed, Oct 14, 2020 at 11:07:55AM +0200, Jan Tluka wrote:
Tue, Oct 13, 2020 at 10:28:58AM CEST, olichtne@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@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
Wed, Oct 14, 2020 at 12:03:29PM CEST, olichtne@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@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@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
On Wed, Oct 14, 2020 at 02:58:53PM +0200, Jan Tluka wrote:
Wed, Oct 14, 2020 at 12:03:29PM CEST, olichtne@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@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@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
Talked about this offline, yes, I believe moving this to inherit from BaseSubConfigMixin is better.
Introducing some intermediate mixin loop between test_wide_configuration and sub_configuration may be an interesting idea but I think we should follow up on that later.
-Ondrej
SctpFirewallPerfTest mixin is replaced with CommonPerfTestTweakMixin to include additional mixins in much simpler way.
Signed-off-by: Jan Tluka jtluka@redhat.com --- lnst/Recipes/ENRT/BaseEnrtRecipe.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lnst/Recipes/ENRT/BaseEnrtRecipe.py b/lnst/Recipes/ENRT/BaseEnrtRecipe.py index 4d0c93c8..a5296e94 100644 --- a/lnst/Recipes/ENRT/BaseEnrtRecipe.py +++ b/lnst/Recipes/ENRT/BaseEnrtRecipe.py @@ -13,7 +13,7 @@ from lnst.Common.Parameters import ( from lnst.Common.IpAddress import AF_INET, AF_INET6
from lnst.Recipes.ENRT.ConfigMixins.BaseSubConfigMixin import BaseSubConfigMixin -from lnst.Recipes.ENRT.PerfTestMixins import SctpFirewallPerfTestMixin +from lnst.Recipes.ENRT.PerfTestMixins import CommonPerfTestTweakMixin
from lnst.RecipeCommon.Ping.Recipe import PingTestAndEvaluate, PingConf from lnst.RecipeCommon.Perf.Recipe import Recipe as PerfRecipe @@ -33,7 +33,7 @@ class EnrtConfiguration(object): """ pass
-class BaseEnrtRecipe(SctpFirewallPerfTestMixin, BaseSubConfigMixin, +class BaseEnrtRecipe(CommonPerfTestTweakMixin, BaseSubConfigMixin, PingTestAndEvaluate, PerfRecipe): """Base Recipe class for the ENRT recipe package
lnst-developers@lists.fedorahosted.org
-
Jan Tluka -
Ondrej Lichtner