On Mon, Jan 14, 2019 at 05:36:17PM +0100, csfakian(a)redhat.com wrote:
From: Christos Sfakianakis <csfakian(a)redhat.com>
lnst.Common: edit IpAddress
Add "link_local" attribute in Ip6Address to be used for filtering
off ipv6 link-local addresses when this is desirable (e.g by using
the "ips_filter" method of the Device module). The value of this
attribute is determined by the "is_link_local" method.
So this is exatcly something that I'd split into a standalone commit
since it can be seen as a standalone feature. Yes it's connected to the
other code in this commit because it's immediately used, but it could
also just stand on it's own as an enhancement of the IpAddress class.
lnst.RecipeCommon: edit Ping
Add methods in PingTestAndEvaluate to handle parallel scenarios:
a) "ping_test_bg" to start a ping job in the background
b) "parallel_ping_test" as the analogous of "ping_test" for
the parallel case
c) "parallel_ping_evaluate_and_report" as the analogous of
"ping_evaluate_and_report" for the parallel case
d) "single_ping_evaluate_and_report" to report which ip's
are used each time in the parallel case
lnst.Recipes.ENRT: edit BaseEnrtRecipe
Include parameters to allow the user to specify ping interval, count,
packet size. Include additional parameters for specifying parallel
scenarios, as well as bidirectional cases.
Modify "generate_ping_configurations" method to account for parallel
scenarios. Assume "ip_versions" and the 2 endpoints are compatible in
that the latter share equal numbers of corresponding ip's and output
error otherwise. Filter off link-local ipv6 addresses. Generate a
list of ping configurations for each ip version specified in a
parallel scenario, or the content of a single-element list in the
default, non-parallel case.
These two I'd leave as a single commit called:
"lnst.RecipeCommon.Ping: redesign to handle parallel scenarios"
And continue with the rest of the commit message which is written fine.
Signed-off-by: Christos Sfakianakis <csfakian(a)redhat.com>
---
lnst/Common/IpAddress.py | 8 +-
lnst/RecipeCommon/Ping.py | 60 +++++++++++++++
lnst/Recipes/ENRT/BaseEnrtRecipe.py | 110 +++++++++++++++++++++++++++-
3 files changed, 173 insertions(+), 5 deletions(-)
diff --git a/lnst/Common/IpAddress.py b/lnst/Common/IpAddress.py
index e54553d..9121fd7 100644
--- a/lnst/Common/IpAddress.py
+++ b/lnst/Common/IpAddress.py
@@ -12,6 +12,8 @@ olichtne(a)redhat.com (Ondrej Lichtner)
import re
from socket import inet_pton, inet_ntop, AF_INET, AF_INET6
+from binascii import hexlify
+import socket
from lnst.Common.LnstError import LnstError
#TODO create various generators for IPNetworks and IPaddresses in the same
@@ -20,7 +22,6 @@ from lnst.Common.LnstError import LnstError
class BaseIpAddress(object):
def __init__(self, addr):
self.addr, self.prefixlen = self._parse_addr(addr)
-
self.family = None
def __str__(self):
@@ -77,6 +78,7 @@ class Ip6Address(BaseIpAddress):
super(Ip6Address, self).__init__(addr)
self.family = AF_INET6
+ self.link_local = self.is_link_local()
@staticmethod
def _parse_addr(addr):
@@ -97,6 +99,10 @@ class Ip6Address(BaseIpAddress):
return addr, prefixlen
+ def is_link_local(self):
+ left_half = hexlify(socket.inet_pton(socket.AF_INET6, str(self)))[:16]
+ return left_half == 'fe80000000000000'
+
def ipaddress(addr):
"""Factory method to create a BaseIpAddress object"""
if isinstance(addr, BaseIpAddress):
diff --git a/lnst/RecipeCommon/Ping.py b/lnst/RecipeCommon/Ping.py
index f5cd652..8e21b1a 100644
--- a/lnst/RecipeCommon/Ping.py
+++ b/lnst/RecipeCommon/Ping.py
@@ -1,5 +1,9 @@
+from copy import copy
+
from lnst.Controller.Recipe import BaseRecipe
from lnst.Tests import Ping
+from lnst.Common.NetTestCommand import DEFAULT_TIMEOUT
+from lnst.Controller.RecipeResults import ResultLevel
class PingConf(object):
def __init__(self,
@@ -44,6 +48,9 @@ class PingConf(object):
class PingTestAndEvaluate(BaseRecipe):
def ping_test(self, ping_config):
+ if isinstance(ping_config, list):
+ return self.parallel_ping_test(ping_config)
+
I'd do this the other way around - if it's not a list, create a list
with that one item and just use the parallel_ping_test method from then
on. Better yet, replace the ping_test method with the parallel_ping_test
method and just add that check there...
Main reason is that this way you have duplicated code - the method
ping_test_bg essentialy does exactly the same thing as the ping_test
method after the isinstance check...
client = ping_config.client
destination = ping_config.destination
@@ -53,13 +60,66 @@ class PingTestAndEvaluate(BaseRecipe):
ping_job = client.run(ping)
return ping_job.result
+ #create backgrounded ping job for parallel scenarios
+ def ping_test_bg(self, ping_config):
+ client = ping_config.client
+ destination = ping_config.destination
+
+ kwargs = self._generate_ping_kwargs(ping_config)
+ ping = Ping(**kwargs)
+ ping_job = client.prepare_job(ping, job_level=ResultLevel.NORMAL)
+ ping_job.start(bg = True)
+
+ return ping_job
I'd move this method below the parallel_ping_test method - when reading
Top to Bottom, it's easier if you first see the context where a method
is used and only later read what that "more specific" method does later.
It gives the impression of diving deeper into more detailed parts of the
code the longer you read.
+
+ #equivalent of ping_test for parallel scenarios
+ def parallel_ping_test(self, parallelpingconf):
^ I'd just rename this to pingconfs
+ results = {}
+
+ running_ping_array = []
+ for pingconf in parallelpingconf:
+ running_ping = self.ping_test_bg(pingconf)
+ running_ping_array.append((pingconf, running_ping))
+
+ for _, ping in running_ping_array:
+ try:
+ ping.wait(timeout=DEFAULT_TIMEOUT)
^
this is now the default so no need to set
this manually or import the DEFAULT_TIMEOUT
anymore
+ finally:
+ ping.kill()
+
+ for pingconf, pingjob in running_ping_array:
+ result = pingjob.result
+ results[pingconf] = result
+
+ return results
+
def ping_evaluate_and_report(self, ping_config, results):
+ if isinstance(ping_config, list):
+ self.parallel_ping_evaluate_and_report(results)
+ return
# do we want to use the "perf" measurements (store a baseline etc...)
as well?
if results["rate"] > 50:
self.add_result(True, "Ping succesful", results)
else:
self.add_result(False, "Ping unsuccesful", results)
+ #parallel version of ping_evaluate_and_report
+ def parallel_ping_evaluate_and_report(self, results):
+ for pingconf, result in results.items():
+ self.single_ping_evaluate_and_report(pingconf, result)
+
+ #clarify source/destination in reporting for parallel scenarios
+ def single_ping_evaluate_and_report(self, ping_config, results):
+ fmt = "From: <{0.client.hostid} ({0.client_bind})> To: " \
+ "<{0.destination.hostid} ({0.destination_address})>"
+ description = fmt.format(ping_config)
+ if results["rate"] > 50:
+ message = "Ping successful --- " + description
+ self.add_result(True, message, results)
+ else:
+ message = "Ping unsuccessful --- " + description
+ self.add_result(False, message, results)
+
def _generate_ping_kwargs(self, ping_config):
kwargs = dict(dst=ping_config.destination_address,
interface=ping_config.client_bind)
diff --git a/lnst/Recipes/ENRT/BaseEnrtRecipe.py b/lnst/Recipes/ENRT/BaseEnrtRecipe.py
index d7d1aec..02e4947 100644
--- a/lnst/Recipes/ENRT/BaseEnrtRecipe.py
+++ b/lnst/Recipes/ENRT/BaseEnrtRecipe.py
@@ -65,6 +65,13 @@ class EnrtSubConfiguration(object):
class BaseEnrtRecipe(PingTestAndEvaluate, PerfRecipe):
ip_versions = Param(default=("ipv4", "ipv6"))
+
+ ping_parallel = BoolParam(default=False)
+ ping_bidirect = BoolParam(default=False)
+ ping_count = IntParam(default = 100)
+ ping_interval = StrParam(default = 0.2)
+ ping_psize = IntParam(default = None)
+
perf_tests = Param(default=("tcp_stream", "udp_stream",
"sctp_stream"))
offload_combinations = Param(default=(
@@ -158,6 +165,47 @@ class BaseEnrtRecipe(PingTestAndEvaluate, PerfRecipe):
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}
^^^ you started using tabs to indent here, mixing spaces and tabs is
very much not recommended in python and bad form anywhere else anyway...
+
+ 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(link_local = False)
+
+ client_ips = client_nic.ips_filter(**kwargs)
+ server_ips = server_nic.ips_filter(**kwargs)
+
+ 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.")
+
+ number_of_ips = len(client_ips)
+ ping_conf_list = []
+ client_nic.valid_ips = client_ips
+ server_nic.valid_ips = server_ips
+ for n in range(number_of_ips):
+ for client_nic, server_nic in [(client_nic, server_nic), (server_nic,
client_nic)]:
+ client_bind = client_nic.valid_ips[n]
+ if not self.params.ping_bidirect:
+ break
+
+ if not self.params.ping_parallel:
+ break
+
+ if not self.params.ping_bidirect and not self.params.ping_parallel:
+ yield ping_conf_list[0]
+ else:
+ 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
@@ -170,10 +218,64 @@ class BaseEnrtRecipe(PingTestAndEvaluate, PerfRecipe):
client_bind = client_nic.ips_filter(family=family)[0]
server_bind = server_nic.ips_filter(family=family)[0]
- yield PingConf(client = client_netns,
- client_bind = client_bind,
- destination = server_netns,
- destination_address = server_bind)
+ for perf_test in self.params.perf_tests:
+ flow = PerfFlow(
+ type = perf_test,
+ generator = client_netns,
+ generator_bind = client_bind,
+ receiver = server_netns,
+ receiver_bind = server_bind,
+ msg_size = self.params.perf_msg_size,
+ duration = self.params.perf_duration,
+ parallel_streams = self.params.perf_parallel_streams)
+
+ flow_measurement = self.params.net_perf_tool([flow])
+ yield PerfRecipeConf(
+ measurements=[
+ self.params.cpu_perf_tool([client_netns, server_netns]),
+ flow_measurement
+ ],
+ iterations=self.params.perf_iterations)
+
+ def _pin_dev_interrupts(self, dev, cpu):
+ netns = dev.netns
+
+ res = netns.run("grep {} /proc/interrupts | cut -f1 -d: | sed 's/
//'"
+ .format(dev.name))
+ 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))
+ 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 server_bind = server_nic.valid_ips[n]
+
+ pconf = PingConf(client = client_nic.netns,
+ client_bind = client_bind,
+ destination = server_nic.netns,
+ destination_address = server_bind,
+ **common_args)
There's some more tabs here for some reason...
Also this looks weirdly broken (copy paste errors or something) because
you have PingConf object creatin inside the _pin_dev_interrupts methods
which really shouldn't be there...
-Ondrej
> +
> + ping_conf_list.append(pconf)
> +
> + if not self.params.ping_bidirect:
> + break
> +
> + if not self.params.ping_parallel:
> + break
> +
> + if not self.params.ping_bidirect and not self.params.ping_parallel:
> + yield ping_conf_list[0]
> + else:
> + yield ping_conf_list
>
> def generate_perf_configurations(self, main_config, sub_config):
> client_nic = main_config.endpoint1
> --
> 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...