diff mbox series

[v4,2/5] dts: parameterize what ports the TG sends packets to

Message ID 20240923184235.22582-3-jspewock@iol.unh.edu (mailing list archive)
State New
Delegated to: Paul Szczepanek
Headers
Series dts: add VFs to the framework |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Jeremy Spewock Sept. 23, 2024, 6:42 p.m. UTC 
From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously in the DTS framework the helper methods in the TestSuite
class designated ports as either ingress or egress ports and would wrap
the methods of the traffic generator to allow packets to only flow to
those designated ingress or egress ports. This is undesirable in some
cases, such as when you have virtual functions on top of your port,
where the TG ports can send to more than one SUT port. This patch
solves this problem by creating optional parameters that allow the user
to specify which port to gather the MAC addresses from when sending and
receiving packets.

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/test_suite.py | 48 +++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 10 deletions(-)

Comments

Juraj Linkeš Sept. 25, 2024, 10:58 a.m. UTC | #1 
On 23. 9. 2024 20:42, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> Previously in the DTS framework the helper methods in the TestSuite
> class designated ports as either ingress or egress ports and would wrap
> the methods of the traffic generator to allow packets to only flow to
> those designated ingress or egress ports. This is undesirable in some
> cases, such as when you have virtual functions on top of your port,
> where the TG ports can send to more than one SUT port. This patch
> solves this problem by creating optional parameters that allow the user
> to specify which port to gather the MAC addresses from when sending and
> receiving packets.
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---

I'm not a fan of exposing the functionality in this way. The developers 
needs to fiddle with ports and there are likely better ways to 
accomplish this.

Ideally, the only information the dev would provide that a test case is 
a VF test case and everything else would happen under the hood in the 
TestCase class.

Barring that, we could decorate the whole TestSuite as requiring VFs, 
which would result in automatically creating and removing the VFs in 
setup/teardown (test case marking would be similar, but possibly more 
complicated, especially if we wanted to abide only by test cases 
selected in a given test run). Then the test cases could pass a simple 
vf=True parameter to the send/receive methods.
Luca Vizzarro Nov. 14, 2024, 5:01 p.m. UTC | #2 
Hi there,

once again I am in agreement with Juraj, this approach may risk to 
complicate things further. As he mentioned, we should allow TestSuite to 
be marked for VFs and they should be able to automate the setup behind 
the scenes. If we wanted to distinguish VF and PF test cases, I think 
that's still doable, maybe group VF and PF ones together for easier 
setup and tear down.

I re-iterate the problem with the commit subject being too long and verbose.

Luca
diff mbox series

Patch

diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
index 051509fb86..9b707ca46d 100644
--- a/dts/framework/test_suite.py
+++ b/dts/framework/test_suite.py
@@ -186,6 +186,8 @@  def send_packet_and_capture(
         packet: Packet,
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
+        sut_ingress: Port | None = None,
+        sut_egress: Port | None = None,
     ) -> list[Packet]:
         """Send and receive `packet` using the associated TG.
 
@@ -196,14 +198,16 @@  def send_packet_and_capture(
             packet: The packet to send.
             filter_config: The filter to use when capturing packets.
             duration: Capture traffic for this amount of time after sending `packet`.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`
 
         Returns:
             A list of received packets.
         """
         return self.send_packets_and_capture(
-            [packet],
-            filter_config,
-            duration,
+            [packet], filter_config, duration, sut_ingress, sut_egress
         )
 
     def send_packets_and_capture(
@@ -211,6 +215,8 @@  def send_packets_and_capture(
         packets: list[Packet],
         filter_config: PacketFilteringConfig = PacketFilteringConfig(),
         duration: float = 1,
+        sut_ingress: Port | None = None,
+        sut_egress: Port | None = None,
     ) -> list[Packet]:
         """Send and receive `packets` using the associated TG.
 
@@ -221,11 +227,19 @@  def send_packets_and_capture(
             packets: The packets to send.
             filter_config: The filter to use when capturing packets.
             duration: Capture traffic for this amount of time after sending `packet`.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`
 
         Returns:
             A list of received packets.
         """
-        packets = [self._adjust_addresses(packet) for packet in packets]
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        packets = [self._adjust_addresses(packet, sut_ingress, sut_egress) for packet in packets]
         return self.tg_node.send_packets_and_capture(
             packets,
             self._tg_port_egress,
@@ -234,18 +248,30 @@  def send_packets_and_capture(
             duration,
         )
 
-    def get_expected_packet(self, packet: Packet) -> Packet:
+    def get_expected_packet(
+        self, packet: Packet, sut_ingress: Port | None = None, sut_egress: Port | None = None
+    ) -> Packet:
         """Inject the proper L2/L3 addresses into `packet`.
 
         Args:
             packet: The packet to modify.
+            sut_ingress: Optional port to use as the SUT ingress port. Defaults to
+                `self._sut_port_ingress`.
+            sut_egress: Optional port to use as the SUT egress port. Defaults to
+                `self._sut_port_egress`.
 
         Returns:
             `packet` with injected L2/L3 addresses.
         """
-        return self._adjust_addresses(packet, expected=True)
-
-    def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
+        if sut_ingress is None:
+            sut_ingress = self._sut_port_ingress
+        if sut_egress is None:
+            sut_egress = self._sut_port_egress
+        return self._adjust_addresses(packet, sut_ingress, sut_egress, expected=True)
+
+    def _adjust_addresses(
+        self, packet: Packet, sut_ingress_port: Port, sut_egress_port: Port, expected: bool = False
+    ) -> Packet:
         """L2 and L3 address additions in both directions.
 
         Assumptions:
@@ -255,11 +281,13 @@  def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             packet: The packet to modify.
             expected: If :data:`True`, the direction is SUT -> TG,
                 otherwise the direction is TG -> SUT.
+            sut_ingress_port: The port to use as the Rx port on the SUT.
+            sut_egress_port: The port to use as the Tx port on the SUT.
         """
         if expected:
             # The packet enters the TG from SUT
             # update l2 addresses
-            packet.src = self._sut_port_egress.mac_address
+            packet.src = sut_egress_port.mac_address
             packet.dst = self._tg_port_ingress.mac_address
 
             # The packet is routed from TG egress to TG ingress
@@ -270,7 +298,7 @@  def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
             # The packet leaves TG towards SUT
             # update l2 addresses
             packet.src = self._tg_port_egress.mac_address
-            packet.dst = self._sut_port_ingress.mac_address
+            packet.dst = sut_ingress_port.mac_address
 
             # The packet is routed from TG egress to TG ingress
             # update l3 addresses