On Tue, Mar 05, 2019 at 05:09:07PM +0100, csfakian(a)redhat.com wrote:
From: Christos Sfakianakis <csfakian(a)redhat.com>
Enable hadnling of perf tests from both sides in BaseEnrtRecipe.
Add this feature in 3 of the recipes (it is used in their old
variants). Do not define the corresponding parameter in
BaseEnrtRecipe, as it is only used with these 3 recipes.
I actually would like the parameter defined in the Base class, without a
default value and set as mandatory=False. Then the 3 recipes can have a
default=True.
But since the parameter is in the base class the tester now has the
ability to set it to True if he wants. If it's not defined in the
BaseEnrtRecipe class then he can only choose to turn it off for the 3
recipes.
Signed-off-by: Christos Sfakianakis <csfakian(a)redhat.com>
---
lnst/Recipes/ENRT/BaseEnrtRecipe.py | 24 ++++++++++++++++++++++++
lnst/Recipes/ENRT/DoubleTeamRecipe.py | 3 ++-
lnst/Recipes/ENRT/TeamRecipe.py | 3 ++-
lnst/Recipes/ENRT/TeamVsBondRecipe.py | 3 ++-
4 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/lnst/Recipes/ENRT/BaseEnrtRecipe.py b/lnst/Recipes/ENRT/BaseEnrtRecipe.py
index 5dc3a5c..df4b456 100644
--- a/lnst/Recipes/ENRT/BaseEnrtRecipe.py
+++ b/lnst/Recipes/ENRT/BaseEnrtRecipe.py
@@ -176,6 +176,20 @@ class BaseEnrtRecipe(PingTestAndEvaluate, PerfRecipe):
destination = server_netns,
destination_address = server_bind)
+ def _create_reverse_flow(self, flow):
+ rev_flow = PerfFlow(
+ type = flow.type,
+ generator = flow.receiver,
+ generator_bind = flow.receiver_bind,
+ receiver = flow.generator,
+ receiver_bind = flow.generator_bind,
+ msg_size = flow.msg_size,
+ duration = flow.duration,
+ parallel_streams = flow.parallel_streams,
+ cpupin = flow.cpupin
+ )
+ return rev_flow
+
In a class it's often more readable to place helper/private methods like
this to the bottom of the class or at least bellow their use. When I'm
reading the class implementation from top to bottom I'll first see this
method without having to see the context of when/why it's used.
On the other hand if it's correctly named, and well implemented when I'm
reading the generate_perf_configurations method and see that it calls a
"create_reverse_flow" method I have a reasonable understanding of what
it does and understand it's use in the context, then if I'm interested
in the specifics of it's implementation I can "drill down" and continue
reading it's implementation.
Not an issue, just a style choice. I took this from the "Clean code"
book, I don't fully agree with everything it says but it has some good
ideas and points, and I like this particular one :).
> def generate_perf_configurations(self, main_config, sub_config):
> client_nic = main_config.endpoint1
> server_nic = main_config.endpoint2
> @@ -212,6 +226,16 @@ class BaseEnrtRecipe(PingTestAndEvaluate, PerfRecipe):
> ],
> iterations=self.params.perf_iterations)
>
> + if "perf_reverse" in self.params and
self.params.perf_reverse:
> + reverse_flow = self._create_reverse_flow(flow)
> + reverse_flow_measurement =
self.params.net_perf_tool([reverse_flow])
> + yield PerfRecipeConf(
> + measurements=[
> + self.params.cpu_perf_tool([server_netns,
client_netns]),
> + reverse_flow_measurement
> + ],
> + iterations=self.params.perf_iterations)
> +
> def _pin_dev_interrupts(self, dev, cpu):
> netns = dev.netns
>
> diff --git a/lnst/Recipes/ENRT/DoubleTeamRecipe.py
b/lnst/Recipes/ENRT/DoubleTeamRecipe.py
> index e10ea79..1dd5899 100644
> --- a/lnst/Recipes/ENRT/DoubleTeamRecipe.py
> +++ b/lnst/Recipes/ENRT/DoubleTeamRecipe.py
> @@ -2,7 +2,7 @@
> Implements scenario similar to regression_tests/phase2/
> ({active_backup,round_robin}_double_team.xml + team_test.py)
> """
> -from lnst.Common.Parameters import Param, IntParam, StrParam
> +from lnst.Common.Parameters import Param, IntParam, StrParam, BoolParam
> from lnst.Common.IpAddress import ipaddress
> from lnst.Controller import HostReq, DeviceReq
> from lnst.Recipes.ENRT.BaseEnrtRecipe import BaseEnrtRecipe, EnrtConfiguration
> @@ -23,6 +23,7 @@ class DoubleTeamRecipe(BaseEnrtRecipe):
> dict(gro="on", gso="off", tso="off",
tx="on"),
> dict(gro="on", gso="on", tso="off",
tx="off")))
>
> + perf_reverse = BoolParam(default=True)
> runner_name = StrParam(mandatory=True)
>
> def test_wide_configuration(self):
> diff --git a/lnst/Recipes/ENRT/TeamRecipe.py b/lnst/Recipes/ENRT/TeamRecipe.py
> index 6b95ccd..35bd988 100644
> --- a/lnst/Recipes/ENRT/TeamRecipe.py
> +++ b/lnst/Recipes/ENRT/TeamRecipe.py
> @@ -2,7 +2,7 @@
> Implements scenario similar to regression_tests/phase2/
> ({active_backup,round_robin}_team.xml + team_test.py)
> """
> -from lnst.Common.Parameters import Param, IntParam, StrParam
> +from lnst.Common.Parameters import Param, IntParam, StrParam, BoolParam
> from lnst.Common.IpAddress import ipaddress
> from lnst.Controller import HostReq, DeviceReq
> from lnst.Recipes.ENRT.BaseEnrtRecipe import BaseEnrtRecipe, EnrtConfiguration
> @@ -22,6 +22,7 @@ class TeamRecipe(BaseEnrtRecipe):
> dict(gro="on", gso="off", tso="off",
tx="on"),
> dict(gro="on", gso="on", tso="off",
tx="off")))
>
> + perf_reverse = BoolParam(default=True)
> runner_name = StrParam(mandatory=True)
>
> def test_wide_configuration(self):
> diff --git a/lnst/Recipes/ENRT/TeamVsBondRecipe.py
b/lnst/Recipes/ENRT/TeamVsBondRecipe.py
> index 7e83e15..69d038f 100644
> --- a/lnst/Recipes/ENRT/TeamVsBondRecipe.py
> +++ b/lnst/Recipes/ENRT/TeamVsBondRecipe.py
> @@ -3,7 +3,7 @@ Implements scenario similar to regression_tests/phase2/
> ({active_backup,round_robin}_team_vs_{active_backup,round_robin}
> _bond.xml + team_test.py)
> """
> -from lnst.Common.Parameters import Param, IntParam, StrParam
> +from lnst.Common.Parameters import Param, IntParam, StrParam, BoolParam
> from lnst.Common.IpAddress import ipaddress
> from lnst.Controller import HostReq, DeviceReq
> from lnst.Recipes.ENRT.BaseEnrtRecipe import BaseEnrtRecipe, EnrtConfiguration
> @@ -25,6 +25,7 @@ class TeamVsBondRecipe(BaseEnrtRecipe):
> dict(gro="on", gso="off", tso="off",
tx="on"),
> dict(gro="on", gso="on", tso="off",
tx="off")))
>
> + perf_reverse = BoolParam(default=True)
> runner_name = StrParam(mandatory = True)
> bonding_mode = StrParam(mandatory = True)
> miimon_value = IntParam(mandatory = True)
> --
> 2.17.1
> _______________________________________________
> LNST-developers mailing list -- lnst-developers(a)lists.fedorahosted.org
> To unsubscribe send an email to lnst-developers-leave(a)lists.fedorahosted.org
> Fedora Code of Conduct:
https://getfedora.org/code-of-conduct.html
> List Guidelines:
https://fedoraproject.org/wiki/Mailing_list_guidelines
> List Archives:
https://lists.fedorahosted.org/archives/list/lnst-developers@lists.fedora...