Re: [PATCH v3 1/4] dts: add send_packets to test suites and rework packet addressing

[Nicholas Pratte <npratte@iol.unh.edu>]

This is great, I'll be using this in-favor of the boolean solution
that I implemented! Just to bring this to your attention. I am
currently working on some Generic Routing Encapsulation suites the
require multiple IP layers at packet creation; they look something
like:

Ether() / IP() / GRE() / IP() / UDP() / Raw(load='x'*80)

I have to take a deeper look to see how multiple IP layers affect the
declaration of src and dst variables, I'll let you know what I find as
some changes might be needed on this implementation to avoid future
bugs. Once I figure it out, I'll leave a review tag for you.

On Wed, Jul 24, 2024 at 11:07 AM <jspewock@iol.unh.edu> wrote:
>
> From: Jeremy Spewock <jspewock@iol.unh.edu>
>
> Currently the only method provided in the test suite class for sending
> packets sends a single packet and then captures the results. There is,
> in some cases, a need to send multiple packets at once while not really
> needing to capture any traffic received back. The method to do this
> exists in the traffic generator already, but this patch exposes the
> method to test suites.
>
> This patch also updates the _adjust_addresses method of test suites so
> that addresses of packets are only modified if the developer did not
> configure them beforehand. This allows for developers to have more
> control over the content of their packets when sending them through the
> framework.
>
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---
>  dts/framework/test_suite.py            | 74 ++++++++++++++++++--------
>  dts/framework/testbed_model/tg_node.py |  9 ++++
>  2 files changed, 62 insertions(+), 21 deletions(-)
>
> diff --git a/dts/framework/test_suite.py b/dts/framework/test_suite.py
> index 694b2eba65..0b678ed62d 100644
> --- a/dts/framework/test_suite.py
> +++ b/dts/framework/test_suite.py
> @@ -199,7 +199,7 @@ def send_packet_and_capture(
>          Returns:
>              A list of received packets.
>          """
> -        packet = self._adjust_addresses(packet)
> +        packet = self._adjust_addresses([packet])[0]
>          return self.tg_node.send_packet_and_capture(
>              packet,
>              self._tg_port_egress,
> @@ -208,6 +208,18 @@ def send_packet_and_capture(
>              duration,
>          )
>
> +    def send_packets(
> +        self,
> +        packets: list[Packet],
> +    ) -> None:
> +        """Send packets using the traffic generator and do not capture received traffic.
> +
> +        Args:
> +            packets: Packets to send.
> +        """
> +        packets = self._adjust_addresses(packets)
> +        self.tg_node.send_packets(packets, self._tg_port_egress)
> +
>      def get_expected_packet(self, packet: Packet) -> Packet:
>          """Inject the proper L2/L3 addresses into `packet`.
>
> @@ -219,39 +231,59 @@ def get_expected_packet(self, packet: Packet) -> Packet:
>          """
>          return self._adjust_addresses(packet, expected=True)
>
> -    def _adjust_addresses(self, packet: Packet, expected: bool = False) -> Packet:
> +    def _adjust_addresses(self, packets: list[Packet], expected: bool = False) -> list[Packet]:
>          """L2 and L3 address additions in both directions.
>
> +        Only missing addresses are added to packets, existing addressed will not be overridden.
> +
>          Assumptions:
>              Two links between SUT and TG, one link is TG -> SUT, the other SUT -> TG.
>
>          Args:
> -            packet: The packet to modify.
> +            packets: The packets to modify.
>              expected: If :data:`True`, the direction is SUT -> TG,
>                  otherwise the direction is TG -> SUT.
>          """
> -        if expected:
> -            # The packet enters the TG from SUT
> -            # update l2 addresses
> -            packet.src = self._sut_port_egress.mac_address
> -            packet.dst = self._tg_port_ingress.mac_address
> +        ret_packets = []
> +        for packet in packets:
> +            default_pkt_src = type(packet)().src
> +            default_pkt_dst = type(packet)().dst
> +            default_pkt_payload_src = IP().src if hasattr(packet.payload, "src") else None
> +            default_pkt_payload_dst = IP().dst if hasattr(packet.payload, "dst") else None
> +            # If `expected` is :data:`True`, the packet enters the TG from SUT, otherwise the
> +            # packet leaves the TG towards the SUT
>
> -            # The packet is routed from TG egress to TG ingress
> -            # update l3 addresses
> -            packet.payload.src = self._tg_ip_address_egress.ip.exploded
> -            packet.payload.dst = self._tg_ip_address_ingress.ip.exploded
> -        else:
> -            # 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
> +            if packet.src == default_pkt_src:
> +                packet.src = (
> +                    self._sut_port_egress.mac_address
> +                    if expected
> +                    else self._tg_port_egress.mac_address
> +                )
> +            if packet.dst == default_pkt_dst:
> +                packet.dst = (
> +                    self._tg_port_ingress.mac_address
> +                    if expected
> +                    else self._sut_port_ingress.mac_address
> +                )
> +
> +            # The packet is routed from TG egress to TG ingress regardless of if it is expected or
> +            # not.
>
> -            # The packet is routed from TG egress to TG ingress
>              # update l3 addresses
> -            packet.payload.src = self._tg_ip_address_egress.ip.exploded
> -            packet.payload.dst = self._tg_ip_address_ingress.ip.exploded
> -
> -        return Ether(packet.build())
> +            if (
> +                default_pkt_payload_src is not None
> +                and packet.payload.src == default_pkt_payload_src
> +            ):
> +                packet.payload.src = self._tg_ip_address_egress.ip.exploded
> +            if (
> +                default_pkt_payload_dst is not None
> +                and packet.payload.dst == default_pkt_payload_dst
> +            ):
> +                packet.payload.dst = self._tg_ip_address_ingress.ip.exploded
> +            ret_packets.append(Ether(packet.build()))
> +
> +        return ret_packets
>
>      def verify(self, condition: bool, failure_description: str) -> None:
>          """Verify `condition` and handle failures.
> diff --git a/dts/framework/testbed_model/tg_node.py b/dts/framework/testbed_model/tg_node.py
> index 4ee326e99c..758b676258 100644
> --- a/dts/framework/testbed_model/tg_node.py
> +++ b/dts/framework/testbed_model/tg_node.py
> @@ -83,6 +83,15 @@ def send_packet_and_capture(
>              duration,
>          )
>
> +    def send_packets(self, packets: list[Packet], port: Port):
> +        """Send packets without capturing resulting received packets.
> +
> +        Args:
> +            packets: Packets to send.
> +            port: Port to send the packets on.
> +        """
> +        self.traffic_generator.send_packets(packets, port)
> +
>      def close(self) -> None:
>          """Free all resources used by the node.
>
> --
> 2.45.2
>