[v3,05/12] dts: add support for simpler topologies
Checks
Commit Message
We currently assume there are two links between the SUT and TG nodes,
but that's too strict, even for some of the already existing test cases.
Add support for topologies with less than two links.
For topologies with no links, dummy ports are used. The expectation is
that test suites or cases that don't require any links won't be using
methods that use ports. Any test suites or cases requiring links will be
skipped in topologies with no links, but this feature is not implemented
in this commit.
Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
---
dts/framework/runner.py | 6 +-
dts/framework/test_suite.py | 32 +++----
dts/framework/testbed_model/node.py | 2 +-
dts/framework/testbed_model/port.py | 4 +-
dts/framework/testbed_model/topology.py | 101 ++++++++++++++++++++++
dts/tests/TestSuite_pmd_buffer_scatter.py | 2 +-
6 files changed, 120 insertions(+), 27 deletions(-)
create mode 100644 dts/framework/testbed_model/topology.py
Comments
I just had one question below, otherwise:
Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš
<juraj.linkes@pantheon.tech> wrote:
<snip>
> diff --git a/dts/framework/testbed_model/topology.py b/dts/framework/testbed_model/topology.py
> new file mode 100644
> index 0000000000..19632ee890
> --- /dev/null
> +++ b/dts/framework/testbed_model/topology.py
<snip>
> +
> +
> +class TopologyType(IntEnum):
> + """Supported topology types."""
> +
> + #: A topology with no Traffic Generator.
> + no_link = 0
> + #: A topology with one physical link between the SUT node and the TG node.
> + one_link = 1
> + #: A topology with two physical links between the Sut node and the TG node.
> + two_links = 2
> +
> +
> +class Topology:
> + """Testbed topology.
> +
> + The topology contains ports processed into ingress and egress ports.
> + It's assumed that port0 of the SUT node is connected to port0 of the TG node and so on.
Do we need to make this assumption when you are comparing the port
directly to its peer and matching the addresses? I think you could
specify in conf.yaml that port 0 on the SUT is one of your ports and
its peer is port 1 on the TG and because you do the matching, this
would work fine.
> + If there are no ports on a node, dummy ports (ports with no actual values) are stored.
> + If there is only one link available, the ports of this link are stored
> + as both ingress and egress ports.
> +
> + The dummy ports shouldn't be used. It's up to :class:`~framework.runner.DTSRunner`
> + to ensure no test case or suite requiring actual links is executed
> + when the topology prohibits it and up to the developers to make sure that test cases
> + not requiring any links don't use any ports. Otherwise, the underlying methods
> + using the ports will fail.
> +
> + Attributes:
> + type: The type of the topology.
> + tg_port_egress: The egress port of the TG node.
> + sut_port_ingress: The ingress port of the SUT node.
> + sut_port_egress: The egress port of the SUT node.
> + tg_port_ingress: The ingress port of the TG node.
> + """
> +
> + type: TopologyType
> + tg_port_egress: Port
> + sut_port_ingress: Port
> + sut_port_egress: Port
> + tg_port_ingress: Port
> +
> + def __init__(self, sut_ports: Iterable[Port], tg_ports: Iterable[Port]):
> + """Create the topology from `sut_ports` and `tg_ports`.
> +
> + Args:
> + sut_ports: The SUT node's ports.
> + tg_ports: The TG node's ports.
> + """
> + port_links = []
> + for sut_port in sut_ports:
> + for tg_port in tg_ports:
> + if (sut_port.identifier, sut_port.peer) == (
> + tg_port.peer,
> + tg_port.identifier,
> + ):
> + port_links.append(PortLink(sut_port=sut_port, tg_port=tg_port))
> +
> + self.type = TopologyType(len(port_links))
<snip>
>
On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš <juraj.linkes@pantheon.tech>
wrote:
> We currently assume there are two links between the SUT and TG nodes,
> but that's too strict, even for some of the already existing test cases.
> Add support for topologies with less than two links.
>
> For topologies with no links, dummy ports are used. The expectation is
> that test suites or cases that don't require any links won't be using
> methods that use ports. Any test suites or cases requiring links will be
> skipped in topologies with no links, but this feature is not implemented
> in this commit.
>
> Signed-off-by: Juraj Linkeš <juraj.linkes@pantheon.tech>
>
Reviewed-by: Dean Marx <dmarx@iol.unh.edu>
On 26. 8. 2024 18:54, Jeremy Spewock wrote:
> I just had one question below, otherwise:
>
> Reviewed-by: Jeremy Spewock <jspewock@iol.unh.edu>
>
> On Wed, Aug 21, 2024 at 10:53 AM Juraj Linkeš
> <juraj.linkes@pantheon.tech> wrote:
> <snip>
>> diff --git a/dts/framework/testbed_model/topology.py b/dts/framework/testbed_model/topology.py
>> new file mode 100644
>> index 0000000000..19632ee890
>> --- /dev/null
>> +++ b/dts/framework/testbed_model/topology.py
> <snip>
>> +
>> +
>> +class TopologyType(IntEnum):
>> + """Supported topology types."""
>> +
>> + #: A topology with no Traffic Generator.
>> + no_link = 0
>> + #: A topology with one physical link between the SUT node and the TG node.
>> + one_link = 1
>> + #: A topology with two physical links between the Sut node and the TG node.
>> + two_links = 2
>> +
>> +
>> +class Topology:
>> + """Testbed topology.
>> +
>> + The topology contains ports processed into ingress and egress ports.
>> + It's assumed that port0 of the SUT node is connected to port0 of the TG node and so on.
>
> Do we need to make this assumption when you are comparing the port
> directly to its peer and matching the addresses? I think you could
> specify in conf.yaml that port 0 on the SUT is one of your ports and
> its peer is port 1 on the TG and because you do the matching, this
> would work fine.
>
Yes, the assumption is not adhered to yet. I guess I put this here
because we've been discussing this in the calls, but the actual code
doesn't use this. I'll remove this line.
>> + If there are no ports on a node, dummy ports (ports with no actual values) are stored.
>> + If there is only one link available, the ports of this link are stored
>> + as both ingress and egress ports.
>> +
>> + The dummy ports shouldn't be used. It's up to :class:`~framework.runner.DTSRunner`
>> + to ensure no test case or suite requiring actual links is executed
>> + when the topology prohibits it and up to the developers to make sure that test cases
>> + not requiring any links don't use any ports. Otherwise, the underlying methods
>> + using the ports will fail.
>> +
>> + Attributes:
>> + type: The type of the topology.
>> + tg_port_egress: The egress port of the TG node.
>> + sut_port_ingress: The ingress port of the SUT node.
>> + sut_port_egress: The egress port of the SUT node.
>> + tg_port_ingress: The ingress port of the TG node.
>> + """
>> +
>> + type: TopologyType
>> + tg_port_egress: Port
>> + sut_port_ingress: Port
>> + sut_port_egress: Port
>> + tg_port_ingress: Port
>> +
>> + def __init__(self, sut_ports: Iterable[Port], tg_ports: Iterable[Port]):
>> + """Create the topology from `sut_ports` and `tg_ports`.
>> +
>> + Args:
>> + sut_ports: The SUT node's ports.
>> + tg_ports: The TG node's ports.
>> + """
>> + port_links = []
>> + for sut_port in sut_ports:
>> + for tg_port in tg_ports:
>> + if (sut_port.identifier, sut_port.peer) == (
>> + tg_port.peer,
>> + tg_port.identifier,
>> + ):
>> + port_links.append(PortLink(sut_port=sut_port, tg_port=tg_port))
>> +
>> + self.type = TopologyType(len(port_links))
> <snip>
>>
@@ -53,6 +53,7 @@
TestSuiteWithCases,
)
from .test_suite import TestCase, TestSuite
+from .testbed_model.topology import Topology
class DTSRunner:
@@ -474,6 +475,7 @@ def _run_test_suites(
test_suites_with_cases: The test suites with test cases to run.
"""
end_build_target = False
+ topology = Topology(sut_node.ports, tg_node.ports)
for test_suite_with_cases in test_suites_with_cases:
test_suite_result = build_target_result.add_test_suite(test_suite_with_cases)
try:
@@ -481,6 +483,7 @@ def _run_test_suites(
self._run_test_suite(
sut_node,
tg_node,
+ topology,
test_suite_result,
test_suite_with_cases,
)
@@ -506,6 +509,7 @@ def _run_test_suite(
self,
sut_node: SutNode,
tg_node: TGNode,
+ topology: Topology,
test_suite_result: TestSuiteResult,
test_suite_with_cases: TestSuiteWithCases,
) -> None:
@@ -533,7 +537,7 @@ def _run_test_suite(
self._logger.set_stage(
DtsStage.test_suite_setup, Path(SETTINGS.output_dir, test_suite_name)
)
- test_suite = test_suite_with_cases.test_suite_class(sut_node, tg_node)
+ test_suite = test_suite_with_cases.test_suite_class(sut_node, tg_node, topology)
try:
self._logger.info(f"Starting test suite setup: {test_suite_name}")
test_suite.set_up_suite()
@@ -24,9 +24,10 @@
from scapy.packet import Packet, Padding # type: ignore[import-untyped]
from framework.testbed_model.capability import TestProtocol
-from framework.testbed_model.port import Port, PortLink
+from framework.testbed_model.port import Port
from framework.testbed_model.sut_node import SutNode
from framework.testbed_model.tg_node import TGNode
+from framework.testbed_model.topology import Topology, TopologyType
from framework.testbed_model.traffic_generator.capturing_traffic_generator import (
PacketFilteringConfig,
)
@@ -72,7 +73,7 @@ class TestSuite(TestProtocol):
#: will block the execution of all subsequent test suites in the current build target.
is_blocking: ClassVar[bool] = False
_logger: DTSLogger
- _port_links: list[PortLink]
+ _topology_type: TopologyType
_sut_port_ingress: Port
_sut_port_egress: Port
_sut_ip_address_ingress: Union[IPv4Interface, IPv6Interface]
@@ -86,6 +87,7 @@ def __init__(
self,
sut_node: SutNode,
tg_node: TGNode,
+ topology: Topology,
):
"""Initialize the test suite testbed information and basic configuration.
@@ -95,35 +97,21 @@ def __init__(
Args:
sut_node: The SUT node where the test suite will run.
tg_node: The TG node where the test suite will run.
+ topology: The topology where the test suite will run.
"""
self.sut_node = sut_node
self.tg_node = tg_node
self._logger = get_dts_logger(self.__class__.__name__)
- self._port_links = []
- self._process_links()
- self._sut_port_ingress, self._tg_port_egress = (
- self._port_links[0].sut_port,
- self._port_links[0].tg_port,
- )
- self._sut_port_egress, self._tg_port_ingress = (
- self._port_links[1].sut_port,
- self._port_links[1].tg_port,
- )
+ self._topology_type = topology.type
+ self._tg_port_egress = topology.tg_port_egress
+ self._sut_port_ingress = topology.sut_port_ingress
+ self._sut_port_egress = topology.sut_port_egress
+ self._tg_port_ingress = topology.tg_port_ingress
self._sut_ip_address_ingress = ip_interface("192.168.100.2/24")
self._sut_ip_address_egress = ip_interface("192.168.101.2/24")
self._tg_ip_address_egress = ip_interface("192.168.100.3/24")
self._tg_ip_address_ingress = ip_interface("192.168.101.3/24")
- def _process_links(self) -> None:
- """Construct links between SUT and TG ports."""
- for sut_port in self.sut_node.ports:
- for tg_port in self.tg_node.ports:
- if (sut_port.identifier, sut_port.peer) == (
- tg_port.peer,
- tg_port.identifier,
- ):
- self._port_links.append(PortLink(sut_port=sut_port, tg_port=tg_port))
-
@classmethod
def get_test_cases(
cls, test_case_sublist: Sequence[str] | None = None
@@ -90,7 +90,7 @@ def __init__(self, node_config: NodeConfiguration):
self._init_ports()
def _init_ports(self) -> None:
- self.ports = [Port(self.name, port_config) for port_config in self.config.ports]
+ self.ports = [Port(port_config) for port_config in self.config.ports]
self.main_session.update_ports(self.ports)
for port in self.ports:
self.configure_port_state(port)
@@ -54,7 +54,7 @@ class Port:
mac_address: str = ""
logical_name: str = ""
- def __init__(self, node_name: str, config: PortConfig):
+ def __init__(self, config: PortConfig):
"""Initialize the port from `node_name` and `config`.
Args:
@@ -62,7 +62,7 @@ def __init__(self, node_name: str, config: PortConfig):
config: The test run configuration of the port.
"""
self.identifier = PortIdentifier(
- node=node_name,
+ node=config.node,
pci=config.pci,
)
self.os_driver = config.os_driver
new file mode 100644
@@ -0,0 +1,101 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2024 PANTHEON.tech s.r.o.
+
+"""Testbed topology representation.
+
+A topology of a testbed captures what links are available between the testbed's nodes.
+The link information then implies what type of topology is available.
+"""
+
+from dataclasses import dataclass
+from enum import IntEnum
+from typing import Iterable
+
+from framework.config import PortConfig
+
+from .port import Port
+
+
+class TopologyType(IntEnum):
+ """Supported topology types."""
+
+ #: A topology with no Traffic Generator.
+ no_link = 0
+ #: A topology with one physical link between the SUT node and the TG node.
+ one_link = 1
+ #: A topology with two physical links between the Sut node and the TG node.
+ two_links = 2
+
+
+class Topology:
+ """Testbed topology.
+
+ The topology contains ports processed into ingress and egress ports.
+ It's assumed that port0 of the SUT node is connected to port0 of the TG node and so on.
+ If there are no ports on a node, dummy ports (ports with no actual values) are stored.
+ If there is only one link available, the ports of this link are stored
+ as both ingress and egress ports.
+
+ The dummy ports shouldn't be used. It's up to :class:`~framework.runner.DTSRunner`
+ to ensure no test case or suite requiring actual links is executed
+ when the topology prohibits it and up to the developers to make sure that test cases
+ not requiring any links don't use any ports. Otherwise, the underlying methods
+ using the ports will fail.
+
+ Attributes:
+ type: The type of the topology.
+ tg_port_egress: The egress port of the TG node.
+ sut_port_ingress: The ingress port of the SUT node.
+ sut_port_egress: The egress port of the SUT node.
+ tg_port_ingress: The ingress port of the TG node.
+ """
+
+ type: TopologyType
+ tg_port_egress: Port
+ sut_port_ingress: Port
+ sut_port_egress: Port
+ tg_port_ingress: Port
+
+ def __init__(self, sut_ports: Iterable[Port], tg_ports: Iterable[Port]):
+ """Create the topology from `sut_ports` and `tg_ports`.
+
+ Args:
+ sut_ports: The SUT node's ports.
+ tg_ports: The TG node's ports.
+ """
+ port_links = []
+ for sut_port in sut_ports:
+ for tg_port in tg_ports:
+ if (sut_port.identifier, sut_port.peer) == (
+ tg_port.peer,
+ tg_port.identifier,
+ ):
+ port_links.append(PortLink(sut_port=sut_port, tg_port=tg_port))
+
+ self.type = TopologyType(len(port_links))
+ dummy_port = Port(PortConfig("", "", "", "", "", ""))
+ self.tg_port_egress = dummy_port
+ self.sut_port_ingress = dummy_port
+ self.sut_port_egress = dummy_port
+ self.tg_port_ingress = dummy_port
+ if self.type > TopologyType.no_link:
+ self.tg_port_egress = port_links[0].tg_port
+ self.sut_port_ingress = port_links[0].sut_port
+ self.sut_port_egress = self.sut_port_ingress
+ self.tg_port_ingress = self.tg_port_egress
+ if self.type > TopologyType.one_link:
+ self.sut_port_egress = port_links[1].sut_port
+ self.tg_port_ingress = port_links[1].tg_port
+
+
+@dataclass(slots=True, frozen=True)
+class PortLink:
+ """The physical, cabled connection between the ports.
+
+ Attributes:
+ sut_port: The port on the SUT node connected to `tg_port`.
+ tg_port: The port on the TG node connected to `sut_port`.
+ """
+
+ sut_port: Port
+ tg_port: Port
@@ -58,7 +58,7 @@ def set_up_suite(self) -> None:
to support larger packet sizes.
"""
self.verify(
- len(self._port_links) > 1,
+ self._topology_type > 1,
"There must be at least two port links to run the scatter test suite",
)