From: Ondrej Lichtner olichtne@redhat.com

Using context managers via the contextmanager decorator method makes this a little clearer and moves the individual try-finally blocks into their own functions.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Recipes/ENRT/BaseEnrtRecipe.py | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/lnst/Recipes/ENRT/BaseEnrtRecipe.py b/lnst/Recipes/ENRT/BaseEnrtRecipe.py index 18e4040..66f563d 100644 --- a/lnst/Recipes/ENRT/BaseEnrtRecipe.py +++ b/lnst/Recipes/ENRT/BaseEnrtRecipe.py @@ -1,4 +1,5 @@ import re +from contextlib import contextmanager

from lnst.Common.LnstError import LnstError from lnst.Common.Parameters import Param, IntParam, StrParam, BoolParam, ListParam @@ -102,13 +103,9 @@ class BaseEnrtRecipe(PingTestAndEvaluate, PerfRecipe): cpu_perf_tool = Param(default=StatCPUMeasurement)

def test(self): - main_config = self.test_wide_configuration() - - try: + with self._test_wide_context() as main_config: for sub_config in self.generate_sub_configurations(main_config): - self.apply_sub_configuration(main_config, sub_config) - - try: + with self._sub_context(main_config, sub_config) as recipe_config: for ping_config in self.generate_ping_configurations(main_config, sub_config): result = self.ping_test(ping_config) @@ -118,10 +115,14 @@ def test(self): sub_config): result = self.perf_test(perf_config) self.perf_report_and_evaluate(result) - finally: - self.remove_sub_configuration(main_config, sub_config) + + @contextmanager + def _test_wide_context(self): + config = self.test_wide_configuration() + try: + yield config finally: - self.test_wide_deconfiguration(main_config) + self.test_wide_deconfiguration(config)

def test_wide_configuration(self): raise NotImplementedError("Method must be defined by a child class.") @@ -129,6 +130,14 @@ def test_wide_configuration(self): def test_wide_deconfiguration(self, main_config): raise NotImplementedError("Method must be defined by a child class.")

+ @contextmanager + def _sub_context(self, main_config, sub_config): + self.apply_sub_configuration(main_config, sub_config) + try: + yield (main_config, sub_config) + finally: + self.remove_sub_configuration(main_config, sub_config) + def generate_sub_configurations(self, main_config): for offload_settings in self.params.offload_combinations: sub_config = EnrtSubConfiguration()

From: Ondrej Lichtner olichtne@redhat.com

This commit removes the offload sub configuration code from the BaseEnrtRecipe class and instead adds a new parent class that just implements the base sub configuration api.

The offload configuration will be split into its own sub configuration mixin class for those recipes that choose that offloads are something that are relevant for them.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Recipes/ENRT/BaseEnrtRecipe.py | 77 ++++------------------------- 1 file changed, 9 insertions(+), 68 deletions(-)

diff --git a/lnst/Recipes/ENRT/BaseEnrtRecipe.py b/lnst/Recipes/ENRT/BaseEnrtRecipe.py index 3b561ae..24e8166 100644 --- a/lnst/Recipes/ENRT/BaseEnrtRecipe.py +++ b/lnst/Recipes/ENRT/BaseEnrtRecipe.py @@ -9,6 +9,8 @@ from lnst.Controller.Recipe import BaseRecipe, RecipeError from lnst.Controller.RecipeResults import ResultLevel

+from lnst.Recipes.ENRT.ConfigMixins.BaseSubConfigMixin import BaseSubConfigMixin + from lnst.RecipeCommon.Ping import PingTestAndEvaluate, PingConf from lnst.RecipeCommon.Perf.Recipe import Recipe as PerfRecipe from lnst.RecipeCommon.Perf.Recipe import RecipeConf as PerfRecipeConf @@ -69,7 +71,7 @@ def offload_settings(self): def offload_settings(self, value): self._offload_settings = value

-class BaseEnrtRecipe(PingTestAndEvaluate, PerfRecipe): +class BaseEnrtRecipe(BaseSubConfigMixin, PingTestAndEvaluate, PerfRecipe): ip_versions = Param(default=("ipv4", "ipv6"))

ping_parallel = BoolParam(default=False) @@ -80,9 +82,6 @@ class BaseEnrtRecipe(PingTestAndEvaluate, PerfRecipe):

perf_tests = Param(default=("tcp_stream", "udp_stream", "sctp_stream"))

- offload_combinations = Param(default=( - dict(gro="on", gso="on", tso="on", tx="on", rx="on"),)) - driver = StrParam(default="ixgbe")

adaptive_rx_coalescing = BoolParam(mandatory=False) @@ -106,7 +105,7 @@ class BaseEnrtRecipe(PingTestAndEvaluate, PerfRecipe): def test(self): with self._test_wide_context() as main_config: for sub_config in self.generate_sub_configurations(main_config): - with self._sub_context(main_config, sub_config) as recipe_config: + with self._sub_context(sub_config) as recipe_config: for ping_config in self.generate_ping_configurations(main_config, sub_config): result = self.ping_test(ping_config) @@ -146,63 +145,13 @@ def generate_test_wide_description(self, config): ]

@contextmanager - def _sub_context(self, main_config, sub_config): - self.apply_sub_configuration(main_config, sub_config) - self.describe_sub_configuration(sub_config) + def _sub_context(self, config): + self.apply_sub_configuration(config) + self.describe_sub_configuration(config) try: - yield (main_config, sub_config) + yield config finally: - self.remove_sub_configuration(main_config, sub_config) - - def generate_sub_configurations(self, main_config): - for offload_settings in self.params.offload_combinations: - sub_config = EnrtSubConfiguration() - sub_config.offload_settings = offload_settings - - yield sub_config - - def apply_sub_configuration(self, main_config, sub_config): - client_nic = main_config.endpoint1 - server_nic = main_config.endpoint2 - client_netns = client_nic.netns - server_netns = server_nic.netns - - if 'sctp_stream' in self.params.perf_tests: - client_netns.run("iptables -I OUTPUT ! -o %s -p sctp -j DROP" % - client_nic.name) - server_netns.run("iptables -I OUTPUT ! -o %s -p sctp -j DROP" % - server_nic.name) - - ethtool_offload_string = "" - for name, value in list(sub_config.offload_settings.items()): - ethtool_offload_string += " %s %s" % (name, value) - - client_netns.run("ethtool -K {} {}".format(client_nic.name, - ethtool_offload_string)) - server_netns.run("ethtool -K {} {}".format(server_nic.name, - ethtool_offload_string)) - - def remove_sub_configuration(self, main_config, sub_config): - client_nic = main_config.endpoint1 - server_nic = main_config.endpoint2 - client_netns = client_nic.netns - server_netns = server_nic.netns - - if 'sctp_stream' in self.params.perf_tests: - client_netns.run("iptables -D OUTPUT ! -o %s -p sctp -j DROP" % - client_nic.name) - server_netns.run("iptables -D OUTPUT ! -o %s -p sctp -j DROP" % - server_nic.name) - - ethtool_offload_string = "" - for name, value in list(sub_config.offload_settings.items()): - ethtool_offload_string += " %s %s" % (name, "on") - - #set all the offloads back to 'on' state - client_netns.run("ethtool -K {} {}".format(client_nic.name, - ethtool_offload_string)) - server_netns.run("ethtool -K {} {}".format(server_nic.name, - ethtool_offload_string)) + self.remove_sub_configuration(config)

def describe_sub_configuration(self, config): description = self.generate_sub_configuration_description(config) @@ -312,14 +261,6 @@ def generate_flow_combinations(self, main_config, sub_config): server_bind = server_nic.ips_filter(family=family)[0]

for perf_test in self.params.perf_tests: - offload_values = list(sub_config.offload_settings.values()) - offload_items = list(sub_config.offload_settings.items()) - if ((perf_test == 'udp_stream' and ('gro', 'off') in offload_items) - or - (perf_test == 'sctp_stream' and 'off' in offload_values and - ('gso', 'on') in offload_items)): - continue - for size in self.params.perf_msg_sizes: flow = PerfFlow( type = perf_test,

From: Ondrej Lichtner olichtne@redhat.com

Refactoring the ping and perf test loops into smaller methods and refactoring the generate_{ping, perf}_configuration methods to use endpoint generator methods instead of accessing a static attribute of the recipe config object.

This makes the tests more flexible as each recipe can define its own endpoints, as many of them as is relevant and also as many in parallel as is required. It can also implement its own logic and generate the endpoints dynamically.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Recipes/ENRT/BaseEnrtRecipe.py | 214 ++++++++++++++-------------- 1 file changed, 106 insertions(+), 108 deletions(-)

diff --git a/lnst/Recipes/ENRT/BaseEnrtRecipe.py b/lnst/Recipes/ENRT/BaseEnrtRecipe.py index 1d75430..0e6c333 100644 --- a/lnst/Recipes/ENRT/BaseEnrtRecipe.py +++ b/lnst/Recipes/ENRT/BaseEnrtRecipe.py @@ -85,15 +85,7 @@ def test(self): with self._test_wide_context() as main_config: for sub_config in self.generate_sub_configurations(main_config): with self._sub_context(sub_config) as recipe_config: - for ping_config in self.generate_ping_configurations(main_config, - sub_config): - result = self.ping_test(ping_config) - self.ping_evaluate_and_report(ping_config, result) - - for perf_config in self.generate_perf_configurations(main_config, - sub_config): - result = self.perf_test(perf_config) - self.perf_report_and_evaluate(result) + self.do_tests(recipe_config)

@contextmanager def _test_wide_context(self): @@ -139,65 +131,65 @@ def describe_sub_configuration(self, config): def generate_sub_configuration_description(self, config): return ["Sub configuration description:"]

- def generate_ping_configurations(self, main_config, sub_config): - client_nic = main_config.endpoint1 - server_nic = main_config.endpoint2 - - count = self.params.ping_count - interval = self.params.ping_interval - size = self.params.ping_psize - common_args = {'count' : count, 'interval' : interval, 'size' : size} - - for ipv in self.params.ip_versions: - kwargs = {} - if ipv == "ipv4": - kwargs.update(family = AF_INET) - elif ipv == "ipv6": - kwargs.update(family = AF_INET6) - kwargs.update(is_link_local = False) - - client_ips = client_nic.ips_filter(**kwargs) - server_ips = server_nic.ips_filter(**kwargs) - if ipv == "ipv6": - client_ips = client_ips[::-1] - server_ips = server_ips[::-1] - - if len(client_ips) != len(server_ips) or len(client_ips) * len(server_ips) == 0: - raise LnstError("Source/destination ip lists are of different size or empty.") - - ping_conf_list = [] - for src_addr, dst_addr in zip(client_ips, server_ips): - pconf = PingConf(client = client_nic.netns, - client_bind = src_addr, - destination = server_nic.netns, - destination_address = dst_addr, - **common_args) - - ping_conf_list.append(pconf) - - if self.params.ping_bidirect: - rev_pconf = self._create_reverse_ping(pconf, common_args) - ping_conf_list.append(rev_pconf) - - if not self.params.ping_parallel: - break - - yield ping_conf_list - - def generate_perf_configurations(self, main_config, sub_config): - client_nic = main_config.endpoint1 - server_nic = main_config.endpoint2 - client_netns = client_nic.netns - server_netns = server_nic.netns - - flow_combinations = self.generate_flow_combinations( - main_config, sub_config - ) + def do_tests(self, recipe_config): + self.do_ping_tests(recipe_config) + self.do_perf_tests(recipe_config) + + def do_ping_tests(self, recipe_config): + for ping_config in self.generate_ping_configurations(recipe_config): + result = self.ping_test(ping_config) + self.ping_evaluate_and_report(ping_config, result) + + def do_perf_tests(self, recipe_config): + for perf_config in self.generate_perf_configurations(recipe_config): + result = self.perf_test(perf_config) + self.perf_report_and_evaluate(result) + + def generate_ping_configurations(self, config): + for endpoint1, endpoint2 in self.generate_ping_endpoints(config): + for ipv in self.params.ip_versions: + ip_filter = {} + if ipv == "ipv4": + ip_filter.update(family = AF_INET) + elif ipv == "ipv6": + ip_filter.update(family = AF_INET6) + ip_filter.update(is_link_local = False) + + endpoint1_ips = endpoint1.ips_filter(**ip_filter) + endpoint2_ips = endpoint2.ips_filter(**ip_filter) + + if len(endpoint1_ips) != len(endpoint2_ips): + raise LnstError("Source/destination ip lists are of different size.") + + ping_conf_list = [] + for src_addr, dst_addr in zip(endpoint1_ips, endpoint2_ips): + pconf = PingConf(client = endpoint1.netns, + client_bind = src_addr, + destination = endpoint2.netns, + destination_address = dst_addr, + count = self.params.ping_count, + interval = self.params.ping_interval, + size = self.params.ping_psize, + ) + + ping_conf_list.append(pconf) + + if self.params.ping_bidirect: + ping_conf_list.append(self._create_reverse_ping(pconf)) + + if not self.params.ping_parallel: + break + + yield ping_conf_list + + def generate_ping_endpoints(self, config): + return []

- for flows in flow_combinations: + def generate_perf_configurations(self, config): + for flows in self.generate_flow_combinations(config): perf_recipe_conf=dict( - main_config=main_config, - sub_config=sub_config, + main_config=config, + sub_config=config, flows=flows, )

@@ -206,8 +198,13 @@ def generate_perf_configurations(self, main_config, sub_config): perf_recipe_conf )

+ cpu_measurement_hosts = set() + for flow in flows: + cpu_measurement_hosts.add(flow.generator) + cpu_measurement_hosts.add(flow.receiver) + cpu_measurement = self.params.cpu_perf_tool( - [client_netns, server_netns], + cpu_measurement_hosts, perf_recipe_conf, )

@@ -225,38 +222,38 @@ def generate_perf_configurations(self, main_config, sub_config):

yield perf_conf

- def generate_flow_combinations(self, main_config, sub_config): - client_nic = main_config.endpoint1 - server_nic = main_config.endpoint2 - client_netns = client_nic.netns - server_netns = server_nic.netns - for ipv in self.params.ip_versions: - if ipv == "ipv4": - family = AF_INET - elif ipv == "ipv6": - family = AF_INET6 - - client_bind = client_nic.ips_filter(family=family)[0] - server_bind = server_nic.ips_filter(family=family)[0] - - for perf_test in self.params.perf_tests: - for size in self.params.perf_msg_sizes: - flow = PerfFlow( - type = perf_test, - generator = client_netns, - generator_bind = client_bind, - receiver = server_netns, - receiver_bind = server_bind, - msg_size = size, - duration = self.params.perf_duration, - parallel_streams = self.params.perf_parallel_streams, - cpupin = self.params.perf_tool_cpu if "perf_tool_cpu" in self.params else None - ) - yield [flow] - - if self.params.perf_reverse: - reverse_flow = self._create_reverse_flow(flow) - yield [reverse_flow] + def generate_flow_combinations(self, config): + for client_nic, server_nic in self.generate_perf_endpoints(config): + for ipv in self.params.ip_versions: + if ipv == "ipv4": + family = AF_INET + elif ipv == "ipv6": + family = AF_INET6 + + client_bind = client_nic.ips_filter(family=family)[0] + server_bind = server_nic.ips_filter(family=family)[0] + + for perf_test in self.params.perf_tests: + for size in self.params.perf_msg_sizes: + flow = PerfFlow( + type = perf_test, + generator = client_nic.netns, + generator_bind = client_bind, + receiver = server_nic.netns, + receiver_bind = server_bind, + msg_size = size, + duration = self.params.perf_duration, + parallel_streams = self.params.perf_parallel_streams, + cpupin = self.params.perf_tool_cpu if "perf_tool_cpu" in self.params else None + ) + yield [flow] + + if self.params.perf_reverse: + reverse_flow = self._create_reverse_flow(flow) + yield [reverse_flow] + + def generate_perf_endpoints(self, config): + return []

@property def cpu_perf_evaluators(self): @@ -281,15 +278,16 @@ def _create_reverse_flow(self, flow): ) return rev_flow

- def _create_reverse_ping(self, pconf, args): - rev_pconf = PingConf( - client = pconf.destination, - client_bind = pconf.destination_address, - destination = pconf.client, - destination_address = pconf.client_bind, - **args - ) - return rev_pconf + def _create_reverse_ping(self, pconf): + return PingConf( + client = pconf.destination, + client_bind = pconf.destination_address, + destination = pconf.client, + destination_address = pconf.client_bind, + count = pconf.ping_count, + interval = pconf.ping_interval, + size = pconf.ping_psize, + )

def _pin_dev_interrupts(self, dev, cpu): netns = dev.netns

From: Ondrej Lichtner olichtne@redhat.com

This will me split off into it's own ConfigMixin class.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Recipes/ENRT/BaseEnrtRecipe.py | 36 --------------------------- lnst/Recipes/ENRT/SimplePerfRecipe.py | 11 -------- 2 files changed, 47 deletions(-)

diff --git a/lnst/Recipes/ENRT/BaseEnrtRecipe.py b/lnst/Recipes/ENRT/BaseEnrtRecipe.py index cacbd94..6582751 100644 --- a/lnst/Recipes/ENRT/BaseEnrtRecipe.py +++ b/lnst/Recipes/ENRT/BaseEnrtRecipe.py @@ -41,7 +41,6 @@ class BaseEnrtRecipe(BaseSubConfigMixin, PingTestAndEvaluate, PerfRecipe):

mtu = IntParam(mandatory=False)

- dev_intr_cpu = IntParam(mandatory=False) perf_tool_cpu = IntParam(mandatory=False)

perf_duration = IntParam(default=60) @@ -262,38 +261,3 @@ def _create_reverse_ping(self, pconf): interval = pconf.ping_interval, size = pconf.ping_psize, ) - - def _pin_dev_interrupts(self, dev, cpu): - netns = dev.netns - cpu_info = netns.run("lscpu", job_level=ResultLevel.DEBUG).stdout - regex = "CPU(s): *([0-9]*)" - num_cpus = int(re.search(regex, cpu_info).groups()[0]) - if cpu < 0 or cpu > num_cpus - 1: - raise RecipeError("Invalid CPU value given: %d. Accepted value %s." % - (cpu, "is: 0" if num_cpus == 1 else "are: 0..%d" % - (num_cpus - 1))) - - res = netns.run( - "grep {} /proc/interrupts | cut -f1 -d: | sed 's/ //'".format( - dev.name - ), - job_level=ResultLevel.DEBUG, - - ) - intrs = res.stdout - split = res.stdout.split("\n") - if len(split) == 1 and split[0] == "": - res = netns.run( - "dev_irqs=/sys/class/net/{}/device/msi_irqs; " - "[ -d $dev_irqs ] && ls -1 $dev_irqs".format(dev.name), - job_level=ResultLevel.DEBUG, - ) - intrs = res.stdout - - for intr in intrs.split("\n"): - try: - int(intr) - netns.run("echo -n {} > /proc/irq/{}/smp_affinity_list" - .format(cpu, intr.strip())) - except: - pass diff --git a/lnst/Recipes/ENRT/SimplePerfRecipe.py b/lnst/Recipes/ENRT/SimplePerfRecipe.py index 263bc83..05e2828 100644 --- a/lnst/Recipes/ENRT/SimplePerfRecipe.py +++ b/lnst/Recipes/ENRT/SimplePerfRecipe.py @@ -47,12 +47,6 @@ def test_wide_configuration(self): for host in [host1, host2]: host.eth0.adaptive_tx_coalescing = self.params.adaptive_tx_coalescing

- #TODO better service handling through HostAPI - if "dev_intr_cpu" in self.params: - for host in [host1, host2]: - host.run("service irqbalance stop") - self._pin_dev_interrupts(host.eth0, self.params.dev_intr_cpu) - if self.params.perf_parallel_streams > 1: for host in [host1, host2]: host.run("tc qdisc replace dev %s root mq" % host.eth0.name) @@ -61,8 +55,3 @@ def test_wide_configuration(self):

def test_wide_deconfiguration(self, config): host1, host2 = self.matched.host1, self.matched.host2 - - #TODO better service handling through HostAPI - if "dev_intr_cpu" in self.params: - for host in [host1, host2]: - host.run("service irqbalance start")

From: Ondrej Lichtner olichtne@redhat.com

Implementing the ping and perf endpoint generator methods as are now expected from the BaseEnrtRecipe test loop implementation. At the same time it's now not required to track these in the EnrtConfiguration object so we can remove them.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Recipes/ENRT/SimplePerfRecipe.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lnst/Recipes/ENRT/SimplePerfRecipe.py b/lnst/Recipes/ENRT/SimplePerfRecipe.py index 05e2828..42491e3 100644 --- a/lnst/Recipes/ENRT/SimplePerfRecipe.py +++ b/lnst/Recipes/ENRT/SimplePerfRecipe.py @@ -24,8 +24,6 @@ def test_wide_configuration(self): host1, host2 = self.matched.host1, self.matched.host2

configuration = EnrtConfiguration() - configuration.endpoint1 = host1.eth0 - configuration.endpoint2 = host2.eth0 configuration.params = self.params

if "mtu" in self.params: @@ -55,3 +53,9 @@ def test_wide_configuration(self):

def test_wide_deconfiguration(self, config): host1, host2 = self.matched.host1, self.matched.host2 + + def generate_ping_endpoints(self, config): + return [(self.matched.host1.eth0, self.matched.host2.eth0)] + + def generate_perf_endpoints(self, config): + return [(self.matched.host1.eth0, self.matched.host2.eth0)]

From: Ondrej Lichtner olichtne@redhat.com

The OffloadSubConfigMixin is an EnrtBaseRecipe subconfiguration mixin class implementing the offload subconfiguration looping. This includes generating, applying, removing and describing the subconfiguration.

It also includes overriding the default perf test flow generation loop so that certain test configurations are ignored when certain offloads are configured.

Enabling this mixin for the SimplePerfRecipe means simply adding it as a base class and defining the required property 'offload_nics' indicating which devices should the offload configuration be done for.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- .../ConfigMixins/OffloadSubConfigMixin.py | 103 ++++++++++++++++++ lnst/Recipes/ENRT/SimplePerfRecipe.py | 13 ++- 2 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 lnst/Recipes/ENRT/ConfigMixins/OffloadSubConfigMixin.py

diff --git a/lnst/Recipes/ENRT/ConfigMixins/OffloadSubConfigMixin.py b/lnst/Recipes/ENRT/ConfigMixins/OffloadSubConfigMixin.py new file mode 100644 index 0000000..f50a165 --- /dev/null +++ b/lnst/Recipes/ENRT/ConfigMixins/OffloadSubConfigMixin.py @@ -0,0 +1,103 @@ +import copy + +from lnst.Common.Parameters import Param +from lnst.Controller.RecipeResults import ResultLevel +from lnst.Recipes.ENRT.ConfigMixins.BaseSubConfigMixin import BaseSubConfigMixin + + +class OffloadSubConfigMixin(BaseSubConfigMixin): + offload_combinations = Param( + default=(dict(gro="on", gso="on", tso="on", tx="on", rx="on"),) + ) + + @property + def offload_nics(self): + raise NotImplementedError("Subclass must implement this property") + + def generate_sub_configurations(self, config): + for parent_config in super().generate_sub_configurations(config): + for offload_settings in self.params.offload_combinations: + new_config = copy.copy(config) + new_config.offload_settings = offload_settings + + yield new_config + + def apply_sub_configuration(self, config): + super().apply_sub_configuration(config) + + offload_settings = getattr(config, "offload_settings", None) + if offload_settings: + ethtool_offload_string = "" + for name, value in list(offload_settings.items()): + ethtool_offload_string += " %s %s" % (name, value) + + for nic in self.offload_nics: + if "sctp_stream" in self.params.perf_tests: + nic.netns.run( + "iptables -I OUTPUT ! -o %s -p sctp -j DROP" % nic.name, + job_level=ResultLevel.NORMAL, + ) + + nic.netns.run( + "ethtool -K {} {}".format(nic.name, ethtool_offload_string), + job_level=ResultLevel.NORMAL, + ) + + def generate_sub_configuration_description(self, config): + description = super().generate_sub_configuration_description(config) + description.append( + "Currently configured offload combination: {}".format( + " ".join( + [ + "{}={}".format(k, v) + for k, v in config.offload_settings.items() + ] + ) + ) + ) + return description + + def remove_sub_configuration(self, config): + offload_settings = getattr(config, "offload_settings", None) + if offload_settings: + ethtool_offload_string = "" + for name, value in list(offload_settings.items()): + ethtool_offload_string += " %s %s" % (name, "on") + + for nic in self.offload_nics: + if "sctp_stream" in self.params.perf_tests: + nic.netns.run( + "iptables -D OUTPUT ! -o %s -p sctp -j DROP" % nic.name, + job_level=ResultLevel.NORMAL, + ) + + # set all the offloads back to 'on' state + nic.netns.run( + "ethtool -K {} {}".format(nic.name, ethtool_offload_string), + job_level=ResultLevel.NORMAL, + ) + + return super().remove_sub_configuration(config) + + def generate_flow_combinations(self, config): + for flows in super().generate_flow_combinations(config): + if self._check_test_offload_conflicts(config, flows): + # TODO log skip + continue + else: + yield flows + + def _check_test_offload_conflicts(self, config, flows): + for flow in flows: + if ( + flow.type == "udp_stream" + and config.offload_settings.get("gro", "on") == "off" + ): + return True + elif ( + flow.type == "sctp_stream" + and "off" in config.offload_settings.values() + and config.offload_settings.get("gso", "on") == "on" + ): + return True + return False diff --git a/lnst/Recipes/ENRT/SimplePerfRecipe.py b/lnst/Recipes/ENRT/SimplePerfRecipe.py index d558bea..b5414c8 100644 --- a/lnst/Recipes/ENRT/SimplePerfRecipe.py +++ b/lnst/Recipes/ENRT/SimplePerfRecipe.py @@ -6,7 +6,14 @@

from lnst.Recipes.ENRT.BaseEnrtRecipe import BaseEnrtRecipe

-class SimplePerfRecipe(BaseEnrtRecipe): +from lnst.Recipes.ENRT.ConfigMixins.OffloadSubConfigMixin import ( + OffloadSubConfigMixin, +) + + +class SimplePerfRecipe( + OffloadSubConfigMixin, BaseEnrtRecipe +): host1 = HostReq() host1.eth0 = DeviceReq(label="net1", driver=RecipeParam("driver"))

@@ -72,3 +79,7 @@ def generate_ping_endpoints(self, config):

def generate_perf_endpoints(self, config): return [(self.matched.host1.eth0, self.matched.host2.eth0)] + + @property + def offload_nics(self): + return [self.matched.host1.eth0, self.matched.host2.eth0]

From: Ondrej Lichtner olichtne@redhat.com

Inheriting from the CommonHWConfigMixin automatically includes all the common configuration, it's descriptions and takes care of deconfiguration.

We can therefore remove the code that's doing the same thing from the SimplePerfRecipe.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Recipes/ENRT/SimplePerfRecipe.py | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-)

diff --git a/lnst/Recipes/ENRT/SimplePerfRecipe.py b/lnst/Recipes/ENRT/SimplePerfRecipe.py index b5414c8..29c7c2c 100644 --- a/lnst/Recipes/ENRT/SimplePerfRecipe.py +++ b/lnst/Recipes/ENRT/SimplePerfRecipe.py @@ -9,10 +9,13 @@ from lnst.Recipes.ENRT.ConfigMixins.OffloadSubConfigMixin import ( OffloadSubConfigMixin, ) +from lnst.Recipes.ENRT.ConfigMixins.CommonHWConfigMixin import ( + CommonHWConfigMixin, +)

class SimplePerfRecipe( - OffloadSubConfigMixin, BaseEnrtRecipe + OffloadSubConfigMixin, CommonHWConfigMixin, BaseEnrtRecipe ): host1 = HostReq() host1.eth0 = DeviceReq(label="net1", driver=RecipeParam("driver")) @@ -32,10 +35,6 @@ def test_wide_configuration(self): configuration = super().test_wide_configuration() configuration.test_wide_devices = []

- if "mtu" in self.params: - host1.eth0.mtu = self.params.mtu - host2.eth0.mtu = self.params.mtu - host1.eth0.ip_add(ipaddress("192.168.101.1/24")) host1.eth0.ip_add(ipaddress("fc00::1/64")) host1.eth0.up() @@ -46,16 +45,6 @@ def test_wide_configuration(self): host2.eth0.up() configuration.test_wide_devices.append(host2.eth0)

- if "adaptive_rx_coalescing" in self.params: - for host in [host1, host2]: - host.eth0.adaptive_rx_coalescing = self.params.adaptive_rx_coalescing - if "adaptive_tx_coalescing" in self.params: - for host in [host1, host2]: - host.eth0.adaptive_tx_coalescing = self.params.adaptive_tx_coalescing - - if self.params.perf_parallel_streams > 1: - for host in [host1, host2]: - host.run("tc qdisc replace dev %s root mq" % host.eth0.name)

return configuration

@@ -83,3 +72,7 @@ def generate_perf_endpoints(self, config): @property def offload_nics(self): return [self.matched.host1.eth0, self.matched.host2.eth0] + + @property + def hw_config_dev_list(self): + return [self.matched.host1.eth0, self.matched.host2.eth0]

From: Ondrej Lichtner olichtne@redhat.com

The return value of compare_result_with_baseline should be a list of strings that describe the comparison. The result gets used in the evaluate_group_results method which joins all the lists together.

Returning a string here would still work becuase it can be iterated but it would result in incorrect formatting of the result.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/RecipeCommon/Perf/Evaluators/BaselineEvaluator.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lnst/RecipeCommon/Perf/Evaluators/BaselineEvaluator.py b/lnst/RecipeCommon/Perf/Evaluators/BaselineEvaluator.py index 20e5a94..72a19e9 100644 --- a/lnst/RecipeCommon/Perf/Evaluators/BaselineEvaluator.py +++ b/lnst/RecipeCommon/Perf/Evaluators/BaselineEvaluator.py @@ -39,4 +39,4 @@ def get_baseline(self, recipe, result): return None

def compare_result_with_baseline(self, recipe, result, baseline): - return False, "Result to baseline comparison not implemented" + return False, ["Result to baseline comparison not implemented"]

From: Ondrej Lichtner olichtne@redhat.com

Calling "iperf3 --version" isn't very useful or part of the actual test so the job level should be lowered to DEBUG.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py index c16eb94..9faf7ec 100644 --- a/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py +++ b/lnst/RecipeCommon/Perf/Measurements/IperfFlowMeasurement.py @@ -40,7 +40,7 @@ def version(self): "hosts_iperf_versions": self._hosts_versions}

def _get_host_iperf_version(self, host): - version_job = host.run("iperf3 --version") + version_job = host.run("iperf3 --version", job_level=ResultLevel.DEBUG) if version_job.passed: match = re.match(r"iperf (.+?) .*", version_job.stdout) if match: