[PATCH 0/6] add L2TPTunnelRecipe

[Jan Tluka]

This patch set adds a recipe to perform a simple L2TP tunnel test.

A minor modification to the BaseTunnelRecipe was required to enable
users to bypass running the packet assert test if it's not required.

The Device.ip_add() method was updated to support the configuration of
addresses with peer address included. This is a typical use case of the
L2TP deployments.

I have also found several issues in the LNST code while implementing the
test, so I'm including them here.

Most important one is handling of RTM_NEWADDR netlink update message
that contained a bug for the point-to-point network address assignment.

Jan Tluka (6):
  Recipes.ENRT.BaseTunnelRecipe: handle empty packet assert configs
  Recipes.ENRT: add L2TPTunnelRecipe
  RecipeCommon.Ping.Evaluators.RatePingEvaluator: fix the rate_text
  RatePingEvaluator: handle None result of the Ping test
  Devices.Device: fix RTM_NEWADDR handling
  Devices.Device: modify _ip_add_one method to handle peer addresses

 docs/source/l2tp_tunnel_recipe.rst            |   6 +
 docs/source/specific_scenarios.rst            |   1 +
 lnst/Devices/Device.py                        |  50 ++++--
 .../Ping/Evaluators/RatePingEvaluator.py      |   8 +-
 lnst/Recipes/ENRT/BaseTunnelRecipe.py         |  15 +-
 lnst/Recipes/ENRT/L2TPTunnelRecipe.py         | 147 ++++++++++++++++++
 6 files changed, 209 insertions(+), 18 deletions(-)
 create mode 100644 docs/source/l2tp_tunnel_recipe.rst
 create mode 100755 lnst/Recipes/ENRT/L2TPTunnelRecipe.py

-- 
2.26.3

[PATCH 1/6] Recipes.ENRT.BaseTunnelRecipe: handle empty packet assert configs

[Jan Tluka]

Some of the recipes inheriting from the BaseTunnelRecipe may not require
packet assert test. This could be solved by not including the
PacketAssertTestAndEvaluate in the BaseTunnelRecipe inheritance but I plan
to send additional tests that use the packet assert test and it's
probably better to add a condition to execute the test.

The BaseTunnelRecipe is modified to perform the packet assert test only
if the get_packet_assert_config() of the derived class returns anything
other than None value.

Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
---
 lnst/Recipes/ENRT/BaseTunnelRecipe.py | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lnst/Recipes/ENRT/BaseTunnelRecipe.py
b/lnst/Recipes/ENRT/BaseTunnelRecipe.py
index 4a354a9d..d4bb6962 100644
--- a/lnst/Recipes/ENRT/BaseTunnelRecipe.py
+++ b/lnst/Recipes/ENRT/BaseTunnelRecipe.py
@@ -111,11 +111,15 @@ class BaseTunnelRecipe(
 
     def ping_test(self, ping_configs):
         pa_config = self.get_packet_assert_config(ping_configs[0])
-        self.packet_assert_test_start(pa_config)
-        self.ctl.wait(2)
+        if pa_config is not None:
+            self.packet_assert_test_start(pa_config)
+            self.ctl.wait(2)
         ping_result = super().ping_test(ping_configs)
-        self.ctl.wait(2)
-        pa_result = self.packet_assert_test_stop()
+        if pa_config is not None:
+            self.ctl.wait(2)
+            pa_result = self.packet_assert_test_stop()
+        else:
+            pa_result = None
 
         result = ((ping_result, pa_config, pa_result),)
 
@@ -124,7 +128,8 @@ class BaseTunnelRecipe(
     def ping_report_and_evaluate(self, results):
         for res in results:
             super().ping_report_and_evaluate(res[0])
-            self.packet_assert_evaluate_and_report(res[1], res[2])
+            if res[1] is not None:
+                self.packet_assert_evaluate_and_report(res[1], res[2])
 
     def get_packet_assert_config(self, ping_config):
         """
-- 
2.26.3

[PATCH 2/6] Recipes.ENRT: add L2TPTunnelRecipe

[Jan Tluka]

This adds a simple L2TP tunnel test.

Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
---
 docs/source/l2tp_tunnel_recipe.rst    |   6 ++
 docs/source/specific_scenarios.rst    |   1 +
 lnst/Recipes/ENRT/L2TPTunnelRecipe.py | 147 ++++++++++++++++++++++++++
 3 files changed, 154 insertions(+)
 create mode 100644 docs/source/l2tp_tunnel_recipe.rst
 create mode 100755 lnst/Recipes/ENRT/L2TPTunnelRecipe.py

diff --git a/docs/source/l2tp_tunnel_recipe.rst b/docs/source/l2tp_tunnel_recipe.rst
new file mode 100644
index 00000000..b967a939
--- /dev/null
+++ b/docs/source/l2tp_tunnel_recipe.rst
@@ -0,0 +1,6 @@
+L2TPTunnelRecipe
+^^^^^^^^^^^^^^^^
+
+.. autoclass:: lnst.Recipes.ENRT.L2TPTunnelRecipe.L2TPTunnelRecipe
+    :members:
+    :show-inheritance:
diff --git a/docs/source/specific_scenarios.rst b/docs/source/specific_scenarios.rst
index fb0b4eee..0cf999cf 100644
--- a/docs/source/specific_scenarios.rst
+++ b/docs/source/specific_scenarios.rst
@@ -8,3 +8,4 @@ Specific ENRT scenarios
     vlans_recipe
     vlans_over_bond_recipe
     gre_tunnel_recipe
+    l2tp_tunnel_recipe
diff --git a/lnst/Recipes/ENRT/L2TPTunnelRecipe.py
b/lnst/Recipes/ENRT/L2TPTunnelRecipe.py
new file mode 100755
index 00000000..080eb2b1
--- /dev/null
+++ b/lnst/Recipes/ENRT/L2TPTunnelRecipe.py
@@ -0,0 +1,147 @@
+from lnst.Controller import HostReq, DeviceReq, RecipeParam
+from lnst.Common.IpAddress import AF_INET
+from lnst.Common.Parameters import ChoiceParam, StrParam
+from lnst.RecipeCommon.L2TPManager import L2TPManager
+from lnst.Devices import L2TPSessionDevice
+from lnst.RecipeCommon.Ping.PingEndpoints import PingEndpoints
+from lnst.Recipes.ENRT.BaseTunnelRecipe import BaseTunnelRecipe
+from lnst.Recipes.ENRT.ConfigMixins.CommonHWSubConfigMixin import (
+    CommonHWSubConfigMixin,
+)
+
+
+class L2TPTunnelRecipe(CommonHWSubConfigMixin, BaseTunnelRecipe):
+    """
+    This class implements a recipe that configures a simple L2TP tunnel with
+    one tunnel session between two hosts.
+
+    .. code-block:: none
+
+                        .--------.
+                 .------| switch |-----.
+                 |      '--------'     |
+                 |                     |
+         .-------|------.      .-------|------.
+         |    .--'-.    |      |    .--'-.    |
+         |    |eth0|    |      |    |eth0|    |
+         |    '----'    |      |    '----'    |
+         |      | |     |      |      | |     |
+         |  ----' '---  |      |  ----' '---  |
+         |  L2TP tunnel |      |  L2TPtunnel  |
+         |  ----------  |      |  ----------  |
+         |              |      |              |
+         |    host1     |      |    host2     |
+         '--------------'      '--------------'
+
+    The actual test machinery is implemented in the :any:`BaseEnrtRecipe` class.
+
+    The test wide configuration is implemented in the :any:`BaseTunnelRecipe`
+    class.
+
+    :param l2tp_encapsulation:
+        (mandatory test parameter) the encapsulation mode for the L2TP tunnel,
+        can be either **udp** or **ip**
+    """
+
+    host1 = HostReq()
+    host1.eth0 = DeviceReq(label="net1",
driver=RecipeParam("driver"))
+
+    host2 = HostReq()
+    host2.eth0 = DeviceReq(label="net1",
driver=RecipeParam("driver"))
+
+    l2tp_encapsulation = ChoiceParam(
+        type=StrParam, choices=set(["udp", "ip"]), mandatory=True
+    )
+
+    def configure_underlying_network(self, configuration):
+        """
+        The underlying network for the tunnel consists of the Ethernet
+        devices on the matched hosts.
+        """
+        host1 = self.matched.host1
+        host2 = self.matched.host2
+
+        for i, device in enumerate([host1.eth0, host2.eth0]):
+            device.ip_add("192.168.200." + str(i + 1) + "/24")
+            device.up()
+            configuration.test_wide_devices.append(device)
+
+        configuration.tunnel_endpoints = (host1.eth0, host2.eth0)
+
+    def create_tunnel(self, configuration):
+        """
+        One L2TP tunnel is configured on both hosts using the
+        :any:`L2TPManager`. Each host configures one L2TP session for the
+        tunnel. IPv4 addresses are assigned to the l2tp session devices.
+        """
+        endpoint1, endpoint2 = configuration.tunnel_endpoints
+        host1 = endpoint1.netns
+        host2 = endpoint2.netns
+        ip_filter = {"family": AF_INET}
+        endpoint1_ip = endpoint1.ips_filter(**ip_filter)[0]
+        endpoint2_ip = endpoint2.ips_filter(**ip_filter)[0]
+
+        host1.l2tp = host1.init_class(L2TPManager)
+        host2.l2tp = host2.init_class(L2TPManager)
+
+        host1.l2tp.create_tunnel(
+            tunnel_id=1000,
+            peer_tunnel_id=1000,
+            encap=self.params.l2tp_encapsulation,
+            local=str(endpoint1_ip),
+            remote=str(endpoint2_ip),
+            udp_sport=5000,
+            udp_dport=5000,
+        )
+        host2.l2tp.create_tunnel(
+            tunnel_id=1000,
+            peer_tunnel_id=1000,
+            encap=self.params.l2tp_encapsulation,
+            local=str(endpoint2_ip),
+            remote=str(endpoint1_ip),
+            udp_sport=5000,
+            udp_dport=5000,
+        )
+        host1.l2tp_session1 = L2TPSessionDevice(
+            tunnel_id=1000,
+            session_id=2000,
+            peer_session_id=2000,
+        )
+        host2.l2tp_session1 = L2TPSessionDevice(
+            tunnel_id=1000,
+            session_id=2000,
+            peer_session_id=2000,
+        )
+
+        for device in [host1.l2tp_session1, host2.l2tp_session1]:
+            device.up()
+
+        ip1 = "10.42.1.1/8"
+        ip2 = "10.42.1.2/8"
+        host1.l2tp_session1.ip_add((ip1, ip2))
+        host2.l2tp_session1.ip_add((ip2, ip1))
+
+        configuration.tunnel_devices.extend([host1.l2tp_session1, host2.l2tp_session1])
+
+    def test_wide_deconfiguration(self, config):
+        for host in [self.matched.host1, self.matched.host2]:
+            host.l2tp.cleanup()
+
+        super().test_wide_deconfiguration(config)
+
+    def generate_ping_endpoints(self, config):
+        """
+        The ping endpoints for this recipe are simply the tunnel endpoints
+
+        Returned as::
+
+            [PingEndpoints(self.matched.host1.l2tp_session1,
self.matched.host2.l2tp_session1)]
+        """
+        return [
+            PingEndpoints(
+                self.matched.host1.l2tp_session1, self.matched.host2.l2tp_session1
+            )
+        ]
+
+    def get_packet_assert_config(self, ping_config):
+        return None
-- 
2.26.3

[PATCH 3/6] RecipeCommon.Ping.Evaluators.RatePingEvaluator: fix the rate_text

[Jan Tluka]

The rate_text string in this branch is formatted too early. Also
insufficient number of values for formatting is used causing crashes.

Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
---
 lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py
b/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py
index 6760d32a..6ed7d02f 100644
--- a/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py
+++ b/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py
@@ -43,9 +43,7 @@ class RatePingEvaluator(BaseResultEvaluator):
                     )
 
         if self.rate is not None:
-            rate_text = 'measured rate {} is {} rate({})'.format(
-                    ping_rate, self.rate)
-
+            rate_text = 'measured rate {} is {} rate({})'
             if ping_rate != int(self.rate):
                 result_status = False
                 result_text.append(
-- 
2.26.3

[PATCH 4/6] RatePingEvaluator: handle None result of the Ping test

[Jan Tluka]

If Ping test does not succeed, the result data does not contain the 'rate'
key and would fail with a traceback. This patch handles this situation
and creates a fail result with explanatory message.

Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
---
 lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py
b/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py
index 6ed7d02f..365329f9 100644
--- a/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py
+++ b/lnst/RecipeCommon/Ping/Evaluators/RatePingEvaluator.py
@@ -13,6 +13,10 @@ class RatePingEvaluator(BaseResultEvaluator):
                     )
 
     def evaluate_results(self, recipe, result):
+        if result is None or 'rate' not in result:
+            recipe.add_result(False, 'Insufficient data for the evaluation of Ping
test')
+            return
+
         result_status = True
         ping_rate = int(result['rate'])
 
-- 
2.26.3

[PATCH 5/6] Devices.Device: fix RTM_NEWADDR handling

[Jan Tluka]

The handler of RTM_NEWADDR incorrectly fetches the IFA_ADDRESS
attribute to update the device address. This is not an issue for IPv6 or
typical update of the device address. However it is a bug for the
assignment of address with peer address included (typical for point to
point networks), for example:

ip address add 192.168.100.1/24 peer 192.169.100.2 dev enp7s0

In this case, the IFA_ADDRESS attribute value is the peer address value
and the device address is the value of IFA_LOCAL.

When the peer address is not specified, both IFA_LOCAL and IFA_ADDRESS
contain the same value which is the reason this has been working without
any issues previously.

The fix is to use the IFA_LOCAL attribute value to update the device
address for IPv4 scenarios.

Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
---
 lnst/Devices/Device.py | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/lnst/Devices/Device.py b/lnst/Devices/Device.py
index c31a018f..68b5f1af 100644
--- a/lnst/Devices/Device.py
+++ b/lnst/Devices/Device.py
@@ -25,7 +25,7 @@ from lnst.Common.ExecCmd import exec_cmd, ExecCmdFail
 from lnst.Common.DeviceError import DeviceError, DeviceDeleted, DeviceDisabled
 from lnst.Common.DeviceError import DeviceConfigError, DeviceConfigValueError
 from lnst.Common.DeviceError import DeviceFeatureNotSupported
-from lnst.Common.IpAddress import ipaddress
+from lnst.Common.IpAddress import ipaddress, AF_INET
 from lnst.Common.HWAddress import hwaddress
 
 from pyroute2.netlink.rtnl import RTM_NEWLINK
@@ -205,8 +205,23 @@ class Device(object, metaclass=DeviceMeta):
         if nl_msg['header']['type'] == RTM_NEWLINK:
             self._nl_msg = nl_msg
         elif nl_msg['header']['type'] == RTM_NEWADDR:
-            addr = ipaddress(nl_msg.get_attr('IFA_ADDRESS'),
-                             flags=nl_msg.get_attr("IFA_FLAGS"))
+            if nl_msg['family'] == AF_INET:
+                """
+                from if_addr.h:
+                /*
+                 * Important comment:
+                 * IFA_ADDRESS is prefix address, rather than local interface address.
+                 * It makes no difference for normally configured broadcast interfaces,
+                 * but for point-to-point IFA_ADDRESS is DESTINATION address,
+                 * local address is supplied in IFA_LOCAL attribute.
+                 */
+                """
+                addr = ipaddress(nl_msg.get_attr('IFA_LOCAL'),
+                                 flags=nl_msg.get_attr("IFA_FLAGS"))
+            else:
+                addr = ipaddress(nl_msg.get_attr('IFA_ADDRESS'),
+                                 flags=nl_msg.get_attr("IFA_FLAGS"))
+
             addr.prefixlen = nl_msg["prefixlen"]
 
             if addr not in self._ip_addrs:
-- 
2.26.3

[PATCH 6/6] Devices.Device: modify _ip_add_one method to handle peer addresses

[Jan Tluka]

The addr parameter of the _ip_add_one() method can now take either a string
specifying a device address or a tuple that includes both the device address
and peer address to support point-to-point networks.

Signed-off-by: Jan Tluka <jtluka(a)redhat.com&gt;
---
 lnst/Devices/Device.py | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/lnst/Devices/Device.py b/lnst/Devices/Device.py
index 68b5f1af..b98d9235 100644
--- a/lnst/Devices/Device.py
+++ b/lnst/Devices/Device.py
@@ -569,15 +569,30 @@ class Device(object, metaclass=DeviceMeta):
         self._ip_addrs = []
 
     def _ip_add_one(self, addr):
-        ip = ipaddress(addr)
+        if type(addr) is tuple:
+            ip = ipaddress(addr[0])
+            peer = ipaddress(addr[1])
+        else:
+            ip = ipaddress(addr)
+            peer = None
+
         if ip not in self.ips:
-            self._ipr_wrapper("addr", "add", index=self.ifindex,
-                              address=str(ip), mask=ip.prefixlen)
+            kwargs = dict(
+                index=self.ifindex,
+                local=str(ip),
+                address=str(ip),
+                mask=ip.prefixlen
+            )
+            if peer:
+                kwargs['address'] = str(peer)
+
+            self._ipr_wrapper("addr", "add", **kwargs)
+
         for i in range(5):
             logging.debug("Waiting for ip address to be added {} of
5".format(i))
             time.sleep(1)
             self._if_manager.rescan_devices()
-            if addr in self.ips:
+            if ip in self.ips:
                 break
         else:
             raise DeviceError("Failed to configure ip address
{}".format(str(ip)))
@@ -586,9 +601,9 @@ class Device(object, metaclass=DeviceMeta):
         """add an ip address or a list of ip addresses
 
         Args:
-            addr -- an address accepted by the ipaddress factory method
-                    or a list of addresses accepted by the ipaddress
-                    factory method
+            addr -- an address or a tuple (address, peer_address) accepted
+                    by the ipaddress factory method or a list of the previous
+                    two types
         """
 
         if isinstance(addr, list):
-- 
2.26.3

[Jan Tluka]

Tue, May 04, 2021 at 11:15:25AM CEST, jtluka(a)redhat.com wrote:
> This patch set adds a recipe to perform a simple L2TP tunnel test.
>
> A minor modification to the BaseTunnelRecipe was required to enable
> users to bypass running the packet assert test if it's not required.
>
> The Device.ip_add() method was updated to support the configuration of
> addresses with peer address included. This is a typical use case of the
> L2TP deployments.
>
> I have also found several issues in the LNST code while implementing the
> test, so I'm including them here.
>
> Most important one is handling of RTM_NEWADDR netlink update message
> that contained a bug for the point-to-point network address assignment.
>
> Jan Tluka (6):
>   Recipes.ENRT.BaseTunnelRecipe: handle empty packet assert configs
>   Recipes.ENRT: add L2TPTunnelRecipe
>   RecipeCommon.Ping.Evaluators.RatePingEvaluator: fix the rate_text
>   RatePingEvaluator: handle None result of the Ping test
>   Devices.Device: fix RTM_NEWADDR handling
>   Devices.Device: modify _ip_add_one method to handle peer addresses
>
>  docs/source/l2tp_tunnel_recipe.rst            |   6 +
>  docs/source/specific_scenarios.rst            |   1 +
>  lnst/Devices/Device.py                        |  50 ++++--
>  .../Ping/Evaluators/RatePingEvaluator.py      |   8 +-
>  lnst/Recipes/ENRT/BaseTunnelRecipe.py         |  15 +-
>  lnst/Recipes/ENRT/L2TPTunnelRecipe.py         | 147 ++++++++++++++++++
>  6 files changed, 209 insertions(+), 18 deletions(-)
>  create mode 100644 docs/source/l2tp_tunnel_recipe.rst
>  create mode 100755 lnst/Recipes/ENRT/L2TPTunnelRecipe.py
>
> -- 
> 2.26.3
>  
Scratch this one. The only issue is that the order of patches does not make
logical sense. The patch adding the L2TP recipe requires the update of
Device._ip_add_one() method.

I've already sent another version of the series.

-Jan