[PATCH] Recipes.ENRT.SimpleMacsecRecipe: fix flow combination generation

From: Ondrej Lichtner

The SimpleMacsecRecipe was overriding the generate_flow_combinations
method originally defined in BaseEnrtRecipe because there was no
separation of flow_endpoints yet.

This method override also contained a bug up until now - the
"ips_filter" was only using "family" filtering, but for ipv6 it should
also have used "is_link_local=False". This lead to undeteremenistic
behaviour when simetimes the link local address was used for the flow
measurement.

The solution is to remove this method and us the inherited method from
the IperfMeasurementGenerator class that has since been improved to use
"generate_perf_endpoints" which is added to the recipe instead.

I also added a wait_tentative_ips() call after calling "up()" on both
the eth and the macsec device in "apply_sub_configuration()" as this can
help avoid race condition issues when an ipv6 address isn't ready.

Signed-off-by: Ondrej Lichtner 
---
 lnst/Recipes/ENRT/SimpleMacsecRecipe.py | 36 +++----------------------
 1 file changed, 3 insertions(+), 33 deletions(-)

diff --git a/lnst/Recipes/ENRT/SimpleMacsecRecipe.py b/lnst/Recipes/ENRT/SimpleMacsecRecipe.py
index a903dea..75161fb 100644
--- a/lnst/Recipes/ENRT/SimpleMacsecRecipe.py
+++ b/lnst/Recipes/ENRT/SimpleMacsecRecipe.py
@@ -105,6 +105,7 @@ def apply_sub_configuration(self, config):
                     "/64"))
                 host.eth0.up()
                 host.msec0.up()
+                self.wait_tentative_ips([host.eth0, host.msec0])

def remove_sub_configuration(self, config):
         if config.encrypt:
@@ -201,39 +202,8 @@ def generate_perf_configurations(self, config):

yield perf_conf

-    def generate_flow_combinations(self, config):
-        client_nic = config.host1.msec0
-        server_nic = config.host2.msec0
-        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:
-                    pstreams = self.params.perf_parallel_streams
-                    flow = PerfFlow(
-                        type = perf_test,
-                        generator = client_netns,
-                        generator_bind = client_bind,
-                        generator_nic = client_nic,
-                        receiver = server_netns,
-                        receiver_bind = server_bind,
-                        receiver_nic = server_nic,
-                        msg_size = size,
-                        duration = self.params.perf_duration,
-                        parallel_streams = pstreams,
-                        cpupin = self.params.perf_tool_cpu if (
-                            "perf_tool_cpu" in self.params) else None
-                            )
-                    yield [flow]
+    def generate_perf_endpoints(self, config):
+        return [(self.matched.host1.msec0, self.matched.host2.msec0)]

@property
     def mtu_hw_config_dev_list(self):

[Ondrej Lichtner]

pushing after ack from jtluka off list.

-Ondrej

On Wed, Feb 10, 2021 at 09:20:19AM +0100, [e-mail] wrote:
> From: Ondrej Lichtner
>
> The SimpleMacsecRecipe was overriding the generate_flow_combinations
> method originally defined in BaseEnrtRecipe because there was no
> separation of flow_endpoints yet.
>
> This method override also contained a bug up until now - the
> "ips_filter" was only using "family" filtering, but for ipv6 it should
> also have used "is_link_local=False". This lead to undeteremenistic
> behaviour when simetimes the link local address was used for the flow
> measurement.
>
> The solution is to remove this method and us the inherited method from
> the IperfMeasurementGenerator class that has since been improved to use
> "generate_perf_endpoints" which is added to the recipe instead.
>
> I also added a wait_tentative_ips() call after calling "up()" on both
> the eth and the macsec device in "apply_sub_configuration()" as this can
> help avoid race condition issues when an ipv6 address isn't ready.
> Signed-off-by: Ondrej Lichtner
>
> lnst/Recipes/ENRT/SimpleMacsecRecipe.py | 36 +++----------------------
>  1 file changed, 3 insertions(+), 33 deletions(-)
>
> diff --git a/lnst/Recipes/ENRT/SimpleMacsecRecipe.py b/lnst/Recipes/ENRT/SimpleMacsecRecipe.py
> index a903dea..75161fb 100644
> --- a/lnst/Recipes/ENRT/SimpleMacsecRecipe.py
> +++ b/lnst/Recipes/ENRT/SimpleMacsecRecipe.py
> @@ -105,6 +105,7 @@ def apply_sub_configuration(self, config):
>                      "/64"))
>                  host.eth0.up()
>                  host.msec0.up()
>            self.wait_tentative_ips([host.eth0, host.msec0])
>
> def remove_sub_configuration(self, config):
>        if config.encrypt:
>
> @@ -201,39 +202,8 @@ def generate_perf_configurations(self, config):
>              yield perf_conf
>
> def generate_flow_combinations(self, config):
>    client_nic = config.host1.msec0
>    server_nic = config.host2.msec0
>    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:
>                pstreams = self.params.perf_parallel_streams
>                flow = PerfFlow(
>                    type = perf_test,
>                    generator = client_netns,
>                    generator_bind = client_bind,
>                    generator_nic = client_nic,
>                    receiver = server_netns,
>                    receiver_bind = server_bind,
>                    receiver_nic = server_nic,
>                    msg_size = size,
>                    duration = self.params.perf_duration,
>                    parallel_streams = pstreams,
>                    cpupin = self.params.perf_tool_cpu if (
>                        "perf_tool_cpu" in self.params) else None
>                        )
>                yield [flow]
>
> def generate_perf_endpoints(self, config):
>    return [(self.matched.host1.msec0, self.matched.host2.msec0)]
>
> @property
>    def mtu_hw_config_dev_list(self):
>
> -- 
> 2.30.0