[RFC PATCH v1 0/2] dts: replace XML-RPC server

[RFC PATCH v1 0/2] dts: replace XML-RPC server

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

This series implements a new way to handle scapy interactions by using
an interactive shell for managing the session instead of an XML-RPC
server.

One thing to note about these changes while reviewing is that I made the
choice to use a Python interactive terminal and import the Scapy
library instead of using a Scapy interactive shell directly. This choice
was made because of inconsistencies in output that were encountered
while using the Scapy shell due to its use of IPython for its
interactive management. The pseudo-terminal used by paramiko does not
support Cursor Placement Requests (CPR) which are required when using
IPython. This does not limit functionality when compared to our previous
implementation because we were only using the Scapy libraries from
within Python in the previous implementation as well.

Jeremy Spewock (2):
  dts: Add interactive shell for managing Scapy
  dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell

 dts/framework/remote_session/__init__.py      |   1 +
 dts/framework/remote_session/scapy_shell.py   | 175 +++++++++++
 .../testbed_model/traffic_generator/scapy.py  | 284 +-----------------
 dts/framework/utils.py                        |   1 +
 4 files changed, 191 insertions(+), 270 deletions(-)
 create mode 100644 dts/framework/remote_session/scapy_shell.py

-- 
2.45.1

[RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

This shell can be used to remotely run Scapy commands interactively
and provides methods for handling the sending and capturing of packets.

depends-on: series-32014 ("Improve interactive shell output gathering
and logging")

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 dts/framework/remote_session/__init__.py    |   1 +
 dts/framework/remote_session/scapy_shell.py | 175 ++++++++++++++++++++
 dts/framework/utils.py                      |   1 +
 3 files changed, 177 insertions(+)
 create mode 100644 dts/framework/remote_session/scapy_shell.py

diff --git a/dts/framework/remote_session/__init__.py b/dts/framework/remote_session/__init__.py
index 1910c81c3c..ec50265bed 100644
--- a/dts/framework/remote_session/__init__.py
+++ b/dts/framework/remote_session/__init__.py
@@ -21,6 +21,7 @@
 from .interactive_shell import InteractiveShell
 from .python_shell import PythonShell
 from .remote_session import CommandResult, RemoteSession
+from .scapy_shell import ScapyShell
 from .ssh_session import SSHSession
 from .testpmd_shell import TestPmdShell
 
diff --git a/dts/framework/remote_session/scapy_shell.py b/dts/framework/remote_session/scapy_shell.py
new file mode 100644
index 0000000000..fa647dc870
--- /dev/null
+++ b/dts/framework/remote_session/scapy_shell.py
@@ -0,0 +1,175 @@
+# SPDX-License-Identifier: BSD-3-Clause
+# Copyright(c) 2024 University of New Hampshire
+
+"""Scapy interactive shell."""
+
+import re
+import time
+from typing import Callable, ClassVar
+
+from scapy.compat import base64_bytes  # type: ignore[import]
+from scapy.layers.l2 import Ether  # type: ignore[import]
+from scapy.packet import Packet  # type: ignore[import]
+
+from framework.testbed_model.port import Port
+from framework.utils import REGEX_FOR_BASE64_ENCODING
+
+from .python_shell import PythonShell
+
+
+class ScapyShell(PythonShell):
+    """Scapy interactive shell.
+
+    The scapy shell is implemented using a :class:`~.python_shell.PythonShell` and importing
+    everything from the "scapy.all" library. This is done due to formatting issues that occur from
+    the scapy interactive shell attempting to use iPython, which is not compatible with the
+    pseudo-terminal that paramiko creates to manage its channels.
+
+    This class is used as an underlying session for the scapy traffic generator and shouldn't be
+    used directly inside of test suites. If there isn't a method in
+    :class:`framework.testbed_model.traffic_generator.scapy.ScapyTrafficGenerator` to fulfill a
+    need, one should be added there and implemented here.
+    """
+
+    #: Name of sniffer to ensure the same is used in all places
+    _sniffer_name: ClassVar[str] = "sniffer"
+    #: Name of variable that points to the list of packets inside the scapy shell.
+    _send_packet_list_name: ClassVar[str] = "packets"
+    #: Padding to add to the start of a line for python syntax compliance.
+    _padding: ClassVar[str] = " " * 4
+
+    def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None:
+        """Overrides :meth:`~.interactive_shell._start_application`.
+
+        Adds a command that imports everything from the scapy library immediately after starting
+        the shell for usage in later calls to the methods of this class.
+
+        Args:
+            get_privileged_command: A function (but could be any callable) that produces
+                the version of the command with elevated privileges.
+        """
+        super()._start_application(get_privileged_command)
+        self.send_command("from scapy.all import *")
+
+    def _build_send_packet_list(self, packets: list[Packet]) -> None:
+        """Build a list of packets to send later.
+
+        Gets the string that represents the Python command that was used to create each packet in
+        `packets` and sends these commands into the underlying Python session. The purpose behind
+        doing this is to create a list that is identical to `packets` inside the shell. This method
+        should only be called by methods for sending packets immediately prior to sending. The list
+        of packets will continue to exist in the scope of the shell until subsequent calls to this
+        method, so failure to rebuild the list prior to sending packets could lead to undesired
+        "stale" packets to be sent.
+
+        Args:
+            packets: The list of packets to recreate in the shell.
+        """
+        self._logger.info("Building a list of packets to send...")
+        self.send_command(
+            f"{self._send_packet_list_name} = [{', '.join(map(Packet.command, packets))}]"
+        )
+
+    def send_packets(self, packets: list[Packet], send_port: Port) -> None:
+        """Send packets without capturing any received traffic.
+
+        Provides a "fire and forget" method for sending packets for situations when there is no
+        need to collected any received traffic.
+
+        Args:
+            packets: The packets to send.
+            send_port: The port to send the packets from.
+        """
+        self._build_send_packet_list(packets)
+        send_command = [
+            "sendp(",
+            f"{self._send_packet_list_name},",
+            f"iface='{send_port.logical_name}',",
+            "realtime=True,",
+            "verbose=True",
+            ")",
+        ]
+        self.send_command(f"\n{self._padding}".join(send_command))
+
+    def _create_sniffer(
+        self, packets_to_send: list[Packet], send_port: Port, recv_port: Port, filter_config: str
+    ) -> None:
+        """Create an asynchronous sniffer in the shell.
+
+        A list of packets to send is added to the sniffer inside of a callback function so that
+        they are immediately sent at the time sniffing is started.
+
+        Args:
+            packets_to_send: A list of packets to send when sniffing is started.
+            send_port: The port to send the packets on when sniffing is started.
+            recv_port: The port to collect the traffic from.
+            filter_config: An optional BPF format filter to use when sniffing for packets. Omitted
+                when set to an empty string.
+        """
+        self._build_send_packet_list(packets_to_send)
+        sniffer_commands = [
+            f"{self._sniffer_name} = AsyncSniffer(",
+            f"iface='{recv_port.logical_name}',",
+            "store=True,",
+            "started_callback=lambda *args: sendp(",
+            f"{self._padding}{self._send_packet_list_name}, iface='{send_port.logical_name}'),",
+            ")",
+        ]
+        if filter_config:
+            sniffer_commands.insert(-1, f"filter='{filter_config}'")
+
+        self.send_command(f"\n{self._padding}".join(sniffer_commands))
+
+    def _start_and_stop_sniffing(self, duration: float) -> list[Packet]:
+        """Starts asynchronous sniffer, runs for a set `duration`, then collects received packets.
+
+        This method expects that you have first created an asynchronous sniffer inside the shell
+        and will fail if you haven't. Received packets are collected by printing the base64
+        encoding of each packet in the shell and then harvesting these encodings using regex to
+        convert back into packet objects.
+
+        Args:
+            duration: The amount of time in seconds to sniff for received packets.
+
+        Returns:
+            A list of all packets that were received while the sniffer was running.
+        """
+        sniffed_packets_name = "gathered_packets"
+        self.send_command(f"{self._sniffer_name}.start()")
+        time.sleep(duration)
+        self.send_command(f"{sniffed_packets_name} = {self._sniffer_name}.stop(join=True)")
+        # An extra newline is required here due to the nature of interactive Python shells
+        packet_objects = self.send_command(
+            f"for pakt in {sniffed_packets_name}: print(bytes_base64(pakt.build()))\n"
+        )
+        # In the string of bytes "b'XXXX'", we only want the contents ("XXXX")
+        list_of_packets_base64 = re.findall(
+            f"^b'({REGEX_FOR_BASE64_ENCODING})'", packet_objects, re.MULTILINE
+        )
+        return [Ether(base64_bytes(pakt)) for pakt in list_of_packets_base64]
+
+    def send_packets_and_capture(
+        self,
+        packets: list[Packet],
+        send_port: Port,
+        recv_port: Port,
+        filter_config: str,
+        duration: float,
+    ) -> list[Packet]:
+        """Send packets and capture any received traffic.
+
+        The steps required to collect these packets are creating a sniffer that holds the packets to
+        send then starting and stopping the sniffer.
+
+        Args:
+            packets: The packets to send.
+            send_port: The port to send the packets from.
+            recv_port: The port to collect received packets from.
+            filter_config: The filter to use while sniffing for packets.
+            duration: The amount of time in seconds to sniff for received packets.
+
+        Returns:
+            A list of packets received after sending `packets`.
+        """
+        self._create_sniffer(packets, send_port, recv_port, filter_config)
+        return self._start_and_stop_sniffing(duration)
diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index cc5e458cc8..4eea1818ed 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -26,6 +26,7 @@
 from .exception import ConfigurationError
 
 REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
+REGEX_FOR_BASE64_ENCODING: str = "[-a-zA-Z0-9+\\/]*={0,3}"
 
 
 def expand_range(range_str: str) -> list[int]:
-- 
2.45.1

[RFC PATCH v1 2/2] dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously all scapy commands were handled using an XML-RPC server that
ran on the TGNode. This unnecessarily enforces a minimum Python version
of 3.10 on the server that is being used as a traffic generator and
complicates the implementation of scapy methods. This patch removes the
XML-RPC server completely and instead uses a ScapyShell to handle all
Scapy interactions.

Bugzilla ID: 1374
depends-on: series-32014 ("Improve interactive shell output gathering
and logging")

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 .../testbed_model/traffic_generator/scapy.py  | 284 +-----------------
 1 file changed, 14 insertions(+), 270 deletions(-)

diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index 5676235119..2b299ad02f 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -13,20 +13,11 @@
 with a local server proxy from the :mod:`xmlrpc.client` module.
 """
 
-import inspect
-import marshal
-import time
-import types
-import xmlrpc.client
-from xmlrpc.server import SimpleXMLRPCServer
 
-import scapy.all  # type: ignore[import]
-from scapy.layers.l2 import Ether  # type: ignore[import]
 from scapy.packet import Packet  # type: ignore[import]
 
 from framework.config import OS, ScapyTrafficGeneratorConfig
-from framework.remote_session import PythonShell
-from framework.settings import SETTINGS
+from framework.remote_session import ScapyShell
 from framework.testbed_model.node import Node
 from framework.testbed_model.port import Port
 
@@ -36,220 +27,29 @@
     _get_default_capture_name,
 )
 
-"""
-========= BEGIN RPC FUNCTIONS =========
-
-All of the functions in this section are intended to be exported to a python
-shell which runs a scapy RPC server. These functions are made available via that
-RPC server to the packet generator. To add a new function to the RPC server,
-first write the function in this section. Then, if you need any imports, make sure to
-add them to SCAPY_RPC_SERVER_IMPORTS as well. After that, add the function to the list
-in EXPORTED_FUNCTIONS. Note that kwargs (keyword arguments) do not work via xmlrpc,
-so you may need to construct wrapper functions around many scapy types.
-"""
-
-"""
-Add the line needed to import something in a normal python environment
-as an entry to this array. It will be imported before any functions are
-sent to the server.
-"""
-SCAPY_RPC_SERVER_IMPORTS = [
-    "from scapy.all import *",
-    "import xmlrpc",
-    "import sys",
-    "from xmlrpc.server import SimpleXMLRPCServer",
-    "import marshal",
-    "import pickle",
-    "import types",
-    "import time",
-]
-
-
-def scapy_send_packets_and_capture(
-    xmlrpc_packets: list[xmlrpc.client.Binary],
-    send_iface: str,
-    recv_iface: str,
-    duration: float,
-    sniff_filter: str,
-) -> list[bytes]:
-    """The RPC function to send and capture packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-        recv_iface: The logical name of the ingress interface.
-        duration: Capture for this amount of time, in seconds.
-
-    Returns:
-        A list of bytes. Each item in the list represents one packet, which needs
-        to be converted back upon transfer from the remote node.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    sniffer = scapy.all.AsyncSniffer(
-        iface=recv_iface,
-        store=True,
-        started_callback=lambda *args: scapy.all.sendp(scapy_packets, iface=send_iface),
-        filter=sniff_filter,
-    )
-    sniffer.start()
-    time.sleep(duration)
-    return [scapy_packet.build() for scapy_packet in sniffer.stop(join=True)]
-
-
-def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: str) -> None:
-    """The RPC function to send packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-    It only sends `xmlrpc_packets`, without capturing them.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    scapy.all.sendp(scapy_packets, iface=send_iface, realtime=True, verbose=True)
-
-
-"""
-Functions to be exposed by the scapy RPC server.
-"""
-RPC_FUNCTIONS = [
-    scapy_send_packets,
-    scapy_send_packets_and_capture,
-]
-
-"""
-========= END RPC FUNCTIONS =========
-"""
-
-
-class QuittableXMLRPCServer(SimpleXMLRPCServer):
-    r"""Basic XML-RPC server.
-
-    The server may be augmented by functions serializable by the :mod:`marshal` module.
-
-    Example:
-        ::
-
-            def hello_world():
-                # to be sent to the XML-RPC server
-                print("Hello World!")
-
-            # start the XML-RPC server on the remote node
-            # the example assumes you're already connect to a tg_node
-            # this is done by starting a Python shell on the remote node
-            from framework.remote_session import PythonShell
-            session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
-
-            # then importing the modules needed to run the server
-            # and the modules for any functions later added to the server
-            session.send_command("import xmlrpc")
-            session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
-
-            # sending the source code of this class to the Python shell
-            from xmlrpc.server import SimpleXMLRPCServer
-            src = inspect.getsource(QuittableXMLRPCServer)
-            src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
-            spacing = "\n" * 4
-            session.send_command(spacing + src + spacing)
-
-            # then starting the server with:
-            command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
-            session.send_command(command, "XMLRPC OK")
-
-            # now the server is running on the remote node and we can add functions to it
-            # first connect to the server from the execution node
-            import xmlrpc.client
-            server_url = f"http://{tg_node.config.hostname}:8000"
-            rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
-
-            # get the function bytes to send
-            import marshal
-            function_bytes = marshal.dumps(hello_world.__code__)
-            rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
-
-            # now we can execute the function on the server
-            xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
-            print(str(xmlrpc_binary_recv))
-    """
-
-    def __init__(self, *args, **kwargs):
-        """Extend the XML-RPC server initialization.
-
-        Args:
-            args: The positional arguments that will be passed to the superclass's constructor.
-            kwargs: The keyword arguments that will be passed to the superclass's constructor.
-                The `allow_none` argument will be set to :data:`True`.
-        """
-        kwargs["allow_none"] = True
-        super().__init__(*args, **kwargs)
-        self.register_introspection_functions()
-        self.register_function(self.quit)
-        self.register_function(self.add_rpc_function)
-
-    def quit(self) -> None:
-        """Quit the server."""
-        self._BaseServer__shutdown_request = True
-        return None
-
-    def add_rpc_function(self, name: str, function_bytes: xmlrpc.client.Binary) -> None:
-        """Add a function to the server from the local server proxy.
-
-        Args:
-              name: The name of the function.
-              function_bytes: The code of the function.
-        """
-        function_code = marshal.loads(function_bytes.data)
-        function = types.FunctionType(function_code, globals(), name)
-        self.register_function(function)
-
-    def serve_forever(self, poll_interval: float = 0.5) -> None:
-        """Extend the superclass method with an additional print.
-
-        Once executed in the local server proxy, the print gives us a clear string to expect
-        when starting the server. The print means this function was executed on the XML-RPC server.
-        """
-        print("XMLRPC OK")
-        super().serve_forever(poll_interval)
-
 
 class ScapyTrafficGenerator(CapturingTrafficGenerator):
-    """Provides access to scapy functions via an RPC interface.
+    """Provides access to scapy functions on a traffic generator.
 
     This class extends the base with remote execution of scapy functions.
 
-    Any packets sent to the remote server are first converted to bytes. They are received as
-    :class:`~xmlrpc.client.Binary` objects on the server side. When the server sends the packets
-    back, they are also received as :class:`~xmlrpc.client.Binary` objects on the client side, are
-    converted back to :class:`~scapy.packet.Packet` objects and only then returned from the methods.
+    All processing of packets is handled via an instance of a
+    :class:`framework.remote_session.scapy_shell.ScapyShell` that runs on the underlying
+    :class:`framework.testbed_model.tg_node.TGNode`.
 
     Attributes:
         session: The exclusive interactive remote session created by the Scapy
-            traffic generator where the XML-RPC server runs.
-        rpc_server_proxy: The object used by clients to execute functions
-            on the XML-RPC server.
+            traffic generator.
     """
 
-    session: PythonShell
-    rpc_server_proxy: xmlrpc.client.ServerProxy
+    session: ScapyShell
     _config: ScapyTrafficGeneratorConfig
 
     def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
         """Extend the constructor with Scapy TG specifics.
 
-        The traffic generator first starts an XML-RPC on the remote `tg_node`.
-        Then it populates the server with functions which use the Scapy library
-        to send/receive traffic:
-
-            * :func:`scapy_send_packets_and_capture`
-            * :func:`scapy_send_packets`
-
-        To enable verbose logging from the xmlrpc client, use the :option:`--verbose`
-        command line argument or the :envvar:`DTS_VERBOSE` environment variable.
+        The traffic generator starts an underlying session that handles scapy interactions
+        that it will use in its provided methods.
 
         Args:
             tg_node: The node where the traffic generator resides.
@@ -262,50 +62,11 @@ def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
         ), "Linux is the only supported OS for scapy traffic generation"
 
         self.session = self._tg_node.create_interactive_shell(
-            PythonShell, timeout=5, privileged=True, name="ScapyXMLRPCServer"
-        )
-
-        # import libs in remote python console
-        for import_statement in SCAPY_RPC_SERVER_IMPORTS:
-            self.session.send_command(import_statement)
-
-        # start the server
-        xmlrpc_server_listen_port = 8000
-        self._start_xmlrpc_server_in_remote_python(xmlrpc_server_listen_port)
-
-        # connect to the server
-        server_url = f"http://{self._tg_node.config.hostname}:{xmlrpc_server_listen_port}"
-        self.rpc_server_proxy = xmlrpc.client.ServerProxy(
-            server_url, allow_none=True, verbose=SETTINGS.verbose
-        )
-
-        # add functions to the server
-        for function in RPC_FUNCTIONS:
-            # A slightly hacky way to move a function to the remote server.
-            # It is constructed from the name and code on the other side.
-            # Pickle cannot handle functions, nor can any of the other serialization
-            # frameworks aside from the libraries used to generate pyc files, which
-            # are even more messy to work with.
-            function_bytes = marshal.dumps(function.__code__)
-            self.rpc_server_proxy.add_rpc_function(function.__name__, function_bytes)
-
-    def _start_xmlrpc_server_in_remote_python(self, listen_port: int) -> None:
-        # load the source of the function
-        src = inspect.getsource(QuittableXMLRPCServer)
-        # Lines with only whitespace break the repl if in the middle of a function
-        # or class, so strip all lines containing only whitespace
-        src = "\n".join([line for line in src.splitlines() if not line.isspace() and line != ""])
-
-        # execute it in the python terminal
-        self.session.send_command(src + "\n")
-        self.session.send_command(
-            f"server = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));server.serve_forever()",
-            "XMLRPC OK",
+            ScapyShell, timeout=5, privileged=True
         )
 
     def _send_packets(self, packets: list[Packet], port: Port) -> None:
-        packets = [packet.build() for packet in packets]
-        self.rpc_server_proxy.scapy_send_packets(packets, port.logical_name)
+        self.session.send_packets(packets, port)
 
     def _create_packet_filter(self, filter_config: PacketFilteringConfig) -> str:
         """Combines filter settings from `filter_config` into a BPF that scapy can use.
@@ -338,27 +99,10 @@ def _send_packets_and_capture(
         duration: float,
         capture_name: str = _get_default_capture_name(),
     ) -> list[Packet]:
-        binary_packets = [packet.build() for packet in packets]
-
-        xmlrpc_packets: list[
-            xmlrpc.client.Binary
-        ] = self.rpc_server_proxy.scapy_send_packets_and_capture(
-            binary_packets,
-            send_port.logical_name,
-            receive_port.logical_name,
-            duration,
-            self._create_packet_filter(filter_config),
-        )  # type: ignore[assignment]
-
-        scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets]
-        return scapy_packets
+        return self.session.send_packets_and_capture(
+            packets, send_port, receive_port, self._create_packet_filter(filter_config), duration
+        )
 
     def close(self) -> None:
         """Close the traffic generator."""
-        try:
-            self.rpc_server_proxy.quit()
-        except ConnectionRefusedError:
-            # Because the python instance closes, we get no RPC response.
-            # Thus, this error is expected
-            pass
         self.session.close()
-- 
2.45.1

Re: [RFC PATCH v1 2/2] dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell

[Juraj Linkeš]

> diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
> index 5676235119..2b299ad02f 100644
> --- a/dts/framework/testbed_model/traffic_generator/scapy.py
> +++ b/dts/framework/testbed_model/traffic_generator/scapy.py

>   
>   class ScapyTrafficGenerator(CapturingTrafficGenerator):
> -    """Provides access to scapy functions via an RPC interface.
> +    """Provides access to scapy functions on a traffic generator.
>   

traffic generator node

>       This class extends the base with remote execution of scapy functions.
>   
> -    Any packets sent to the remote server are first converted to bytes. They are received as
> -    :class:`~xmlrpc.client.Binary` objects on the server side. When the server sends the packets
> -    back, they are also received as :class:`~xmlrpc.client.Binary` objects on the client side, are
> -    converted back to :class:`~scapy.packet.Packet` objects and only then returned from the methods.
> +    All processing of packets is handled via an instance of a
> +    :class:`framework.remote_session.scapy_shell.ScapyShell` that runs on the underlying
> +    :class:`framework.testbed_model.tg_node.TGNode`.
>   

The module docstring should also be updated.

>       Attributes:
>           session: The exclusive interactive remote session created by the Scapy
> -            traffic generator where the XML-RPC server runs.
> -        rpc_server_proxy: The object used by clients to execute functions
> -            on the XML-RPC server.
> +            traffic generator.
>       """
>   
> -    session: PythonShell
> -    rpc_server_proxy: xmlrpc.client.ServerProxy
> +    session: ScapyShell
>       _config: ScapyTrafficGeneratorConfig
>   
>       def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
>           """Extend the constructor with Scapy TG specifics.
>   
> -        The traffic generator first starts an XML-RPC on the remote `tg_node`.
> -        Then it populates the server with functions which use the Scapy library
> -        to send/receive traffic:
> -
> -            * :func:`scapy_send_packets_and_capture`
> -            * :func:`scapy_send_packets`
> -
> -        To enable verbose logging from the xmlrpc client, use the :option:`--verbose`
> -        command line argument or the :envvar:`DTS_VERBOSE` environment variable.
> +        The traffic generator starts an underlying session that handles scapy interactions
> +        that it will use in its provided methods.
>   

I'm not sure what you're trying to say here - that the methods the tg 
exposes are using the scapy session?

>           Args:
>               tg_node: The node where the traffic generator resides.
> @@ -262,50 +62,11 @@ def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
>           ), "Linux is the only supported OS for scapy traffic generation"
>   
>           self.session = self._tg_node.create_interactive_shell(

Looks like in this specific case, we could do this with multiple 
inheritance instead of composition.

Composition is needed in the other use cases, since we use different 
objects based on the config (e.g. Linux or Windows session). Here, we're 
always going to use the same object (ScapyShell).

The code would need to be refactored to achieve multiple inheritance 
(the __init__ methods would probably have to accept extra kwargs) and 
Luca's testpmd params patch would help a lot, as that looks at least 
somewhat suitable.

I don't know how well would multiple inheritance work, if at all, but 
it's worth trying so that we don't have to basically copy-paste the same 
method signature over and over (e.g. _send_packets and send_packets in 
ScapyTrafficGenerator and ScapyShell).

Re: [RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy

[Juraj Linkeš]

> diff --git a/dts/framework/remote_session/scapy_shell.py b/dts/framework/remote_session/scapy_shell.py
> new file mode 100644
> index 0000000000..fa647dc870
> --- /dev/null
> +++ b/dts/framework/remote_session/scapy_shell.py
> @@ -0,0 +1,175 @@
> +# SPDX-License-Identifier: BSD-3-Clause
> +# Copyright(c) 2024 University of New Hampshire
> +
> +"""Scapy interactive shell."""
> +
> +import re
> +import time
> +from typing import Callable, ClassVar
> +
> +from scapy.compat import base64_bytes  # type: ignore[import]
> +from scapy.layers.l2 import Ether  # type: ignore[import]
> +from scapy.packet import Packet  # type: ignore[import]
> +
> +from framework.testbed_model.port import Port
> +from framework.utils import REGEX_FOR_BASE64_ENCODING
> +
> +from .python_shell import PythonShell
> +
> +
> +class ScapyShell(PythonShell):
> +    """Scapy interactive shell.
> +
> +    The scapy shell is implemented using a :class:`~.python_shell.PythonShell` and importing
> +    everything from the "scapy.all" library. This is done due to formatting issues that occur from
> +    the scapy interactive shell attempting to use iPython, which is not compatible with the
> +    pseudo-terminal that paramiko creates to manage its channels.
> +
> +    This class is used as an underlying session for the scapy traffic generator and shouldn't be
> +    used directly inside of test suites. If there isn't a method in
> +    :class:`framework.testbed_model.traffic_generator.scapy.ScapyTrafficGenerator` to fulfill a
> +    need, one should be added there and implemented here.
> +    """
> +
> +    #: Name of sniffer to ensure the same is used in all places
> +    _sniffer_name: ClassVar[str] = "sniffer"
> +    #: Name of variable that points to the list of packets inside the scapy shell.
> +    _send_packet_list_name: ClassVar[str] = "packets"
> +    #: Padding to add to the start of a line for python syntax compliance.
> +    _padding: ClassVar[str] = " " * 4
> +
> +    def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None:
> +        """Overrides :meth:`~.interactive_shell._start_application`.

This extends the method and in that case we should mention what the 
extension is.

> +
> +        Adds a command that imports everything from the scapy library immediately after starting
> +        the shell for usage in later calls to the methods of this class.
> +
> +        Args:
> +            get_privileged_command: A function (but could be any callable) that produces
> +                the version of the command with elevated privileges.
> +        """
> +        super()._start_application(get_privileged_command)
> +        self.send_command("from scapy.all import *")
> +
> +    def _build_send_packet_list(self, packets: list[Packet]) -> None:

The send in the name evokes that the method sends the packets.

The description in the Args section says "packets to recreate in the 
shell" and I like that so I'd put that in the name: _create_packet_list()

> +        """Build a list of packets to send later.
> +
> +        Gets the string that represents the Python command that was used to create each packet in

Gets the string sounds like that's what the methods returns, as a getter 
method would.

> +        `packets` and sends these commands into the underlying Python session. The purpose behind
> +        doing this is to create a list that is identical to `packets` inside the shell. This method
> +        should only be called by methods for sending packets immediately prior to sending. The list
> +        of packets will continue to exist in the scope of the shell until subsequent calls to this
> +        method, so failure to rebuild the list prior to sending packets could lead to undesired
> +        "stale" packets to be sent.
> +
> +        Args:
> +            packets: The list of packets to recreate in the shell.
> +        """
> +        self._logger.info("Building a list of packets to send...")

The could be just a regular dot instead of the ellipsis (I don't like 
random ellipses as those read as if I was supposed to expect something 
and we don't provide a subsequent log that would continue this ellipsis).

> +        self.send_command(
> +            f"{self._send_packet_list_name} = [{', '.join(map(Packet.command, packets))}]"
> +        )
> +
> +    def send_packets(self, packets: list[Packet], send_port: Port) -> None:
> +        """Send packets without capturing any received traffic.
> +
> +        Provides a "fire and forget" method for sending packets for situations when there is no
> +        need to collected any received traffic.

Typo: collected

> +
> +        Args:
> +            packets: The packets to send.
> +            send_port: The port to send the packets from.
> +        """
> +        self._build_send_packet_list(packets)
> +        send_command = [
> +            "sendp(",
> +            f"{self._send_packet_list_name},",
> +            f"iface='{send_port.logical_name}',",
> +            "realtime=True,",
> +            "verbose=True",
> +            ")",
> +        ]
> +        self.send_command(f"\n{self._padding}".join(send_command))
> +
> +    def _create_sniffer(
> +        self, packets_to_send: list[Packet], send_port: Port, recv_port: Port, filter_config: str
> +    ) -> None:
> +        """Create an asynchronous sniffer in the shell.
> +
> +        A list of packets to send is added to the sniffer inside of a callback function so that
> +        they are immediately sent at the time sniffing is started.
> +
> +        Args:
> +            packets_to_send: A list of packets to send when sniffing is started.
> +            send_port: The port to send the packets on when sniffing is started.
> +            recv_port: The port to collect the traffic from.
> +            filter_config: An optional BPF format filter to use when sniffing for packets. Omitted
> +                when set to an empty string.
> +        """
> +        self._build_send_packet_list(packets_to_send)
> +        sniffer_commands = [
> +            f"{self._sniffer_name} = AsyncSniffer(",
> +            f"iface='{recv_port.logical_name}',",
> +            "store=True,",
> +            "started_callback=lambda *args: sendp(",
> +            f"{self._padding}{self._send_packet_list_name}, iface='{send_port.logical_name}'),",
> +            ")",
> +        ]
> +        if filter_config:
> +            sniffer_commands.insert(-1, f"filter='{filter_config}'")
> +
> +        self.send_command(f"\n{self._padding}".join(sniffer_commands))
> +
> +    def _start_and_stop_sniffing(self, duration: float) -> list[Packet]:
> +        """Starts asynchronous sniffer, runs for a set `duration`, then collects received packets.

This should be in imperative to align with the rest of the docstrings.

> + > +        This method expects that you have first created an 
asynchronous sniffer inside the shell
> +        and will fail if you haven't. Received packets are collected by printing the base64
> +        encoding of each packet in the shell and then harvesting these encodings using regex to
> +        convert back into packet objects.
> +
> +        Args:
> +            duration: The amount of time in seconds to sniff for received packets.
> +
> +        Returns:
> +            A list of all packets that were received while the sniffer was running.
> +        """
> +        sniffed_packets_name = "gathered_packets"
> +        self.send_command(f"{self._sniffer_name}.start()")
> +        time.sleep(duration)
> +        self.send_command(f"{sniffed_packets_name} = {self._sniffer_name}.stop(join=True)")
> +        # An extra newline is required here due to the nature of interactive Python shells
> +        packet_objects = self.send_command(

These are strings, which are objects, but I'd like to be more explicit, 
so maybe packet_strs?

> +            f"for pakt in {sniffed_packets_name}: print(bytes_base64(pakt.build()))\n"
> +        )
> +        # In the string of bytes "b'XXXX'", we only want the contents ("XXXX")
> +        list_of_packets_base64 = re.findall(
> +            f"^b'({REGEX_FOR_BASE64_ENCODING})'", packet_objects, re.MULTILINE
> +        )
> +        return [Ether(base64_bytes(pakt)) for pakt in list_of_packets_base64]
> +
> +    def send_packets_and_capture(
> +        self,
> +        packets: list[Packet],
> +        send_port: Port,
> +        recv_port: Port,
> +        filter_config: str,
> +        duration: float,
> +    ) -> list[Packet]:
> +        """Send packets and capture any received traffic.
> +
> +        The steps required to collect these packets are creating a sniffer that holds the packets to
> +        send then starting and stopping the sniffer.
> +
> +        Args:
> +            packets: The packets to send.
> +            send_port: The port to send the packets from.
> +            recv_port: The port to collect received packets from.
> +            filter_config: The filter to use while sniffing for packets.
> +            duration: The amount of time in seconds to sniff for received packets.
> +
> +        Returns:
> +            A list of packets received after sending `packets`.
> +        """
> +        self._create_sniffer(packets, send_port, recv_port, filter_config)
> +        return self._start_and_stop_sniffing(duration)

Re: [RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy

[Jeremy Spewock]

On Tue, Jun 11, 2024 at 7:12 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
>
> > diff --git a/dts/framework/remote_session/scapy_shell.py b/dts/framework/remote_session/scapy_shell.py
> > new file mode 100644
> > index 0000000000..fa647dc870
> > --- /dev/null
> > +++ b/dts/framework/remote_session/scapy_shell.py
> > @@ -0,0 +1,175 @@
> > +# SPDX-License-Identifier: BSD-3-Clause
> > +# Copyright(c) 2024 University of New Hampshire
> > +
> > +"""Scapy interactive shell."""
> > +
> > +import re
> > +import time
> > +from typing import Callable, ClassVar
> > +
> > +from scapy.compat import base64_bytes  # type: ignore[import]
> > +from scapy.layers.l2 import Ether  # type: ignore[import]
> > +from scapy.packet import Packet  # type: ignore[import]
> > +
> > +from framework.testbed_model.port import Port
> > +from framework.utils import REGEX_FOR_BASE64_ENCODING
> > +
> > +from .python_shell import PythonShell
> > +
> > +
> > +class ScapyShell(PythonShell):
> > +    """Scapy interactive shell.
> > +
> > +    The scapy shell is implemented using a :class:`~.python_shell.PythonShell` and importing
> > +    everything from the "scapy.all" library. This is done due to formatting issues that occur from
> > +    the scapy interactive shell attempting to use iPython, which is not compatible with the
> > +    pseudo-terminal that paramiko creates to manage its channels.
> > +
> > +    This class is used as an underlying session for the scapy traffic generator and shouldn't be
> > +    used directly inside of test suites. If there isn't a method in
> > +    :class:`framework.testbed_model.traffic_generator.scapy.ScapyTrafficGenerator` to fulfill a
> > +    need, one should be added there and implemented here.
> > +    """
> > +
> > +    #: Name of sniffer to ensure the same is used in all places
> > +    _sniffer_name: ClassVar[str] = "sniffer"
> > +    #: Name of variable that points to the list of packets inside the scapy shell.
> > +    _send_packet_list_name: ClassVar[str] = "packets"
> > +    #: Padding to add to the start of a line for python syntax compliance.
> > +    _padding: ClassVar[str] = " " * 4
> > +
> > +    def _start_application(self, get_privileged_command: Callable[[str], str] | None) -> None:
> > +        """Overrides :meth:`~.interactive_shell._start_application`.
>
> This extends the method and in that case we should mention what the
> extension is.

Ack.

>
> > +
> > +        Adds a command that imports everything from the scapy library immediately after starting
> > +        the shell for usage in later calls to the methods of this class.
> > +
> > +        Args:
> > +            get_privileged_command: A function (but could be any callable) that produces
> > +                the version of the command with elevated privileges.
> > +        """
> > +        super()._start_application(get_privileged_command)
> > +        self.send_command("from scapy.all import *")
> > +
> > +    def _build_send_packet_list(self, packets: list[Packet]) -> None:
>
> The send in the name evokes that the method sends the packets.
>
> The description in the Args section says "packets to recreate in the
> shell" and I like that so I'd put that in the name: _create_packet_list()
>

Great point. I was trying too hard to force the idea of building the
"list of packets that will be sent" but on its own this just builds a
list of packets.

> > +        """Build a list of packets to send later.
> > +
> > +        Gets the string that represents the Python command that was used to create each packet in
>
> Gets the string sounds like that's what the methods returns, as a getter
> method would.

Fair enough, I'll edit this.

>
> > +        `packets` and sends these commands into the underlying Python session. The purpose behind
> > +        doing this is to create a list that is identical to `packets` inside the shell. This method
> > +        should only be called by methods for sending packets immediately prior to sending. The list
> > +        of packets will continue to exist in the scope of the shell until subsequent calls to this
> > +        method, so failure to rebuild the list prior to sending packets could lead to undesired
> > +        "stale" packets to be sent.
> > +
> > +        Args:
> > +            packets: The list of packets to recreate in the shell.
> > +        """
> > +        self._logger.info("Building a list of packets to send...")
>
> The could be just a regular dot instead of the ellipsis (I don't like
> random ellipses as those read as if I was supposed to expect something
> and we don't provide a subsequent log that would continue this ellipsis).

Ack.

>
> > +        self.send_command(
> > +            f"{self._send_packet_list_name} = [{', '.join(map(Packet.command, packets))}]"
> > +        )
> > +
> > +    def send_packets(self, packets: list[Packet], send_port: Port) -> None:
> > +        """Send packets without capturing any received traffic.
> > +
> > +        Provides a "fire and forget" method for sending packets for situations when there is no
> > +        need to collected any received traffic.
>
> Typo: collected

Good catch!

>
> > +
> > +        Args:
> > +            packets: The packets to send.
> > +            send_port: The port to send the packets from.
> > +        """
> > +        self._build_send_packet_list(packets)
> > +        send_command = [
> > +            "sendp(",
> > +            f"{self._send_packet_list_name},",
> > +            f"iface='{send_port.logical_name}',",
> > +            "realtime=True,",
> > +            "verbose=True",
> > +            ")",
> > +        ]
> > +        self.send_command(f"\n{self._padding}".join(send_command))
> > +
> > +    def _create_sniffer(
> > +        self, packets_to_send: list[Packet], send_port: Port, recv_port: Port, filter_config: str
> > +    ) -> None:
> > +        """Create an asynchronous sniffer in the shell.
> > +
> > +        A list of packets to send is added to the sniffer inside of a callback function so that
> > +        they are immediately sent at the time sniffing is started.
> > +
> > +        Args:
> > +            packets_to_send: A list of packets to send when sniffing is started.
> > +            send_port: The port to send the packets on when sniffing is started.
> > +            recv_port: The port to collect the traffic from.
> > +            filter_config: An optional BPF format filter to use when sniffing for packets. Omitted
> > +                when set to an empty string.
> > +        """
> > +        self._build_send_packet_list(packets_to_send)
> > +        sniffer_commands = [
> > +            f"{self._sniffer_name} = AsyncSniffer(",
> > +            f"iface='{recv_port.logical_name}',",
> > +            "store=True,",
> > +            "started_callback=lambda *args: sendp(",
> > +            f"{self._padding}{self._send_packet_list_name}, iface='{send_port.logical_name}'),",
> > +            ")",
> > +        ]
> > +        if filter_config:
> > +            sniffer_commands.insert(-1, f"filter='{filter_config}'")
> > +
> > +        self.send_command(f"\n{self._padding}".join(sniffer_commands))
> > +
> > +    def _start_and_stop_sniffing(self, duration: float) -> list[Packet]:
> > +        """Starts asynchronous sniffer, runs for a set `duration`, then collects received packets.
>
> This should be in imperative to align with the rest of the docstrings.

Ack.

>
> > + > +        This method expects that you have first created an
> asynchronous sniffer inside the shell
> > +        and will fail if you haven't. Received packets are collected by printing the base64
> > +        encoding of each packet in the shell and then harvesting these encodings using regex to
> > +        convert back into packet objects.
> > +
> > +        Args:
> > +            duration: The amount of time in seconds to sniff for received packets.
> > +
> > +        Returns:
> > +            A list of all packets that were received while the sniffer was running.
> > +        """
> > +        sniffed_packets_name = "gathered_packets"
> > +        self.send_command(f"{self._sniffer_name}.start()")
> > +        time.sleep(duration)
> > +        self.send_command(f"{sniffed_packets_name} = {self._sniffer_name}.stop(join=True)")
> > +        # An extra newline is required here due to the nature of interactive Python shells
> > +        packet_objects = self.send_command(
>
> These are strings, which are objects, but I'd like to be more explicit,
> so maybe packet_strs?

Good point, that would be more clear.

>
> > +            f"for pakt in {sniffed_packets_name}: print(bytes_base64(pakt.build()))\n"
> > +        )
> > +        # In the string of bytes "b'XXXX'", we only want the contents ("XXXX")
> > +        list_of_packets_base64 = re.findall(
> > +            f"^b'({REGEX_FOR_BASE64_ENCODING})'", packet_objects, re.MULTILINE
> > +        )
> > +        return [Ether(base64_bytes(pakt)) for pakt in list_of_packets_base64]
> > +
> > +    def send_packets_and_capture(
> > +        self,
> > +        packets: list[Packet],
> > +        send_port: Port,
> > +        recv_port: Port,
> > +        filter_config: str,
> > +        duration: float,
> > +    ) -> list[Packet]:
> > +        """Send packets and capture any received traffic.
> > +
> > +        The steps required to collect these packets are creating a sniffer that holds the packets to
> > +        send then starting and stopping the sniffer.
> > +
> > +        Args:
> > +            packets: The packets to send.
> > +            send_port: The port to send the packets from.
> > +            recv_port: The port to collect received packets from.
> > +            filter_config: The filter to use while sniffing for packets.
> > +            duration: The amount of time in seconds to sniff for received packets.
> > +
> > +        Returns:
> > +            A list of packets received after sending `packets`.
> > +        """
> > +        self._create_sniffer(packets, send_port, recv_port, filter_config)
> > +        return self._start_and_stop_sniffing(duration)

Re: [RFC PATCH v1 2/2] dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell

[Jeremy Spewock]

On Tue, Jun 11, 2024 at 6:46 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
> > diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
> > index 5676235119..2b299ad02f 100644
> > --- a/dts/framework/testbed_model/traffic_generator/scapy.py
> > +++ b/dts/framework/testbed_model/traffic_generator/scapy.py
>
> >
> >   class ScapyTrafficGenerator(CapturingTrafficGenerator):
> > -    """Provides access to scapy functions via an RPC interface.
> > +    """Provides access to scapy functions on a traffic generator.
> >
>
> traffic generator node

Ack.

>
> >       This class extends the base with remote execution of scapy functions.
> >
> > -    Any packets sent to the remote server are first converted to bytes. They are received as
> > -    :class:`~xmlrpc.client.Binary` objects on the server side. When the server sends the packets
> > -    back, they are also received as :class:`~xmlrpc.client.Binary` objects on the client side, are
> > -    converted back to :class:`~scapy.packet.Packet` objects and only then returned from the methods.
> > +    All processing of packets is handled via an instance of a
> > +    :class:`framework.remote_session.scapy_shell.ScapyShell` that runs on the underlying
> > +    :class:`framework.testbed_model.tg_node.TGNode`.
> >
>
> The module docstring should also be updated.

Oops, good catch.

>
> >       Attributes:
> >           session: The exclusive interactive remote session created by the Scapy
> > -            traffic generator where the XML-RPC server runs.
> > -        rpc_server_proxy: The object used by clients to execute functions
> > -            on the XML-RPC server.
> > +            traffic generator.
> >       """
> >
> > -    session: PythonShell
> > -    rpc_server_proxy: xmlrpc.client.ServerProxy
> > +    session: ScapyShell
> >       _config: ScapyTrafficGeneratorConfig
> >
> >       def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
> >           """Extend the constructor with Scapy TG specifics.
> >
> > -        The traffic generator first starts an XML-RPC on the remote `tg_node`.
> > -        Then it populates the server with functions which use the Scapy library
> > -        to send/receive traffic:
> > -
> > -            * :func:`scapy_send_packets_and_capture`
> > -            * :func:`scapy_send_packets`
> > -
> > -        To enable verbose logging from the xmlrpc client, use the :option:`--verbose`
> > -        command line argument or the :envvar:`DTS_VERBOSE` environment variable.
> > +        The traffic generator starts an underlying session that handles scapy interactions
> > +        that it will use in its provided methods.
> >
>
> I'm not sure what you're trying to say here - that the methods the tg
> exposes are using the scapy session?

Yeah, that is exactly what I was trying to say, just that the TG
creates a PythonShell and that's what it uses to interact with scapy
when you call it basically.

>
> >           Args:
> >               tg_node: The node where the traffic generator resides.
> > @@ -262,50 +62,11 @@ def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
> >           ), "Linux is the only supported OS for scapy traffic generation"
> >
> >           self.session = self._tg_node.create_interactive_shell(
>
> Looks like in this specific case, we could do this with multiple
> inheritance instead of composition.
>
> Composition is needed in the other use cases, since we use different
> objects based on the config (e.g. Linux or Windows session). Here, we're
> always going to use the same object (ScapyShell).
>
> The code would need to be refactored to achieve multiple inheritance
> (the __init__ methods would probably have to accept extra kwargs) and
> Luca's testpmd params patch would help a lot, as that looks at least
> somewhat suitable.
>
> I don't know how well would multiple inheritance work, if at all, but
> it's worth trying so that we don't have to basically copy-paste the same
> method signature over and over (e.g. _send_packets and send_packets in
> ScapyTrafficGenerator and ScapyShell).

I like this idea. Multiple inheritance is something that I haven't
used much myself but I agree that creating wrapper methods over and
over for the methods that I want to use is very unintuitive. I'll try
it and see how far I can get with it.

>

[PATCH v1 0/1] dts: replace XML-RPC server

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

v1:
 * Documentation fixes based on comments
 * Use multiple inheritance to merge the two commits together, allowing
   the scapy traffic generator to handle the implementation of all of
   the scapy related logic.

Jeremy Spewock (1):
  dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

 .../testbed_model/traffic_generator/scapy.py  | 432 ++++++------------
 dts/framework/utils.py                        |   1 +
 2 files changed, 144 insertions(+), 289 deletions(-)

-- 
2.45.2

[PATCH v1 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously all scapy commands were handled using an XML-RPC server that
ran on the TGNode. This unnecessarily enforces a minimum Python version
of 3.10 on the server that is being used as a traffic generator and
complicates the implementation of scapy methods. This patch removes the
XML-RPC server completely and instead allows the Scapy TG to extend from
the PythonShell to implement the functionality of a traffic generator.
This is done by importing the Scapy library in the PythonShell and
sending commands directly to the interactive session on the TG Node.

Bugzilla ID: 1374
depends-on: series-32242 ("Improve interactive shell output gathering
and logging")

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 .../testbed_model/traffic_generator/scapy.py  | 422 ++++++------------
 dts/framework/utils.py                        |   1 +
 2 files changed, 139 insertions(+), 284 deletions(-)

diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index ca0ea6aca3..0fd7e28522 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -6,309 +6,176 @@
 
 A traffic generator used for functional testing, implemented with
 `the Scapy library <https://scapy.readthedocs.io/en/latest/>`_.
-The traffic generator uses an XML-RPC server to run Scapy on the remote TG node.
+The traffic generator uses an interactive shell to run Scapy on the remote TG node.
 
-The traffic generator uses the :mod:`xmlrpc.server` module to run an XML-RPC server
-in an interactive remote Python SSH session. The communication with the server is facilitated
-with a local server proxy from the :mod:`xmlrpc.client` module.
+The traffic generator extends :class:`framework.remote_session.python_shell.PythonShell` to
+implement the methods for handling packets by sending commands into the interactive shell.
 """
 
-import inspect
-import marshal
+
+import re
 import time
-import types
-import xmlrpc.client
-from xmlrpc.server import SimpleXMLRPCServer
+from typing import ClassVar
 
-import scapy.all  # type: ignore[import-untyped]
+from scapy.compat import base64_bytes  # type: ignore[import-untyped]
 from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
 from scapy.packet import Packet  # type: ignore[import-untyped]
 
 from framework.config import OS, ScapyTrafficGeneratorConfig
 from framework.remote_session.python_shell import PythonShell
-from framework.settings import SETTINGS
 from framework.testbed_model.node import Node
 from framework.testbed_model.port import Port
-
-from .capturing_traffic_generator import (
-    CapturingTrafficGenerator,
+from framework.testbed_model.traffic_generator.capturing_traffic_generator import (
     PacketFilteringConfig,
-    _get_default_capture_name,
 )
+from framework.utils import REGEX_FOR_BASE64_ENCODING
 
-"""
-========= BEGIN RPC FUNCTIONS =========
-
-All of the functions in this section are intended to be exported to a python
-shell which runs a scapy RPC server. These functions are made available via that
-RPC server to the packet generator. To add a new function to the RPC server,
-first write the function in this section. Then, if you need any imports, make sure to
-add them to SCAPY_RPC_SERVER_IMPORTS as well. After that, add the function to the list
-in EXPORTED_FUNCTIONS. Note that kwargs (keyword arguments) do not work via xmlrpc,
-so you may need to construct wrapper functions around many scapy types.
-"""
-
-"""
-Add the line needed to import something in a normal python environment
-as an entry to this array. It will be imported before any functions are
-sent to the server.
-"""
-SCAPY_RPC_SERVER_IMPORTS = [
-    "from scapy.all import *",
-    "import xmlrpc",
-    "import sys",
-    "from xmlrpc.server import SimpleXMLRPCServer",
-    "import marshal",
-    "import pickle",
-    "import types",
-    "import time",
-]
-
-
-def scapy_send_packets_and_capture(
-    xmlrpc_packets: list[xmlrpc.client.Binary],
-    send_iface: str,
-    recv_iface: str,
-    duration: float,
-    sniff_filter: str,
-) -> list[bytes]:
-    """The RPC function to send and capture packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-        recv_iface: The logical name of the ingress interface.
-        duration: Capture for this amount of time, in seconds.
-
-    Returns:
-        A list of bytes. Each item in the list represents one packet, which needs
-        to be converted back upon transfer from the remote node.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    sniffer = scapy.all.AsyncSniffer(
-        iface=recv_iface,
-        store=True,
-        started_callback=lambda *args: scapy.all.sendp(scapy_packets, iface=send_iface),
-        filter=sniff_filter,
-    )
-    sniffer.start()
-    time.sleep(duration)
-    return [scapy_packet.build() for scapy_packet in sniffer.stop(join=True)]
-
-
-def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: str) -> None:
-    """The RPC function to send packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-    It only sends `xmlrpc_packets`, without capturing them.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    scapy.all.sendp(scapy_packets, iface=send_iface, realtime=True, verbose=True)
-
-
-"""
-Functions to be exposed by the scapy RPC server.
-"""
-RPC_FUNCTIONS = [
-    scapy_send_packets,
-    scapy_send_packets_and_capture,
-]
-
-"""
-========= END RPC FUNCTIONS =========
-"""
-
+from .capturing_traffic_generator import CapturingTrafficGenerator
 
-class QuittableXMLRPCServer(SimpleXMLRPCServer):
-    r"""Basic XML-RPC server.
 
-    The server may be augmented by functions serializable by the :mod:`marshal` module.
+class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
+    """Provides access to scapy functions on a traffic generator node.
 
-    Example:
-        ::
+    This class extends the base with remote execution of scapy functions. All methods for
+    processing packets are implemented using an underlying
+    :class:`framework.remote_session.python_shell.PythonShell` which imports the Scapy library.
 
-            def hello_world():
-                # to be sent to the XML-RPC server
-                print("Hello World!")
-
-            # start the XML-RPC server on the remote node
-            # this is done by starting a Python shell on the remote node
-            from framework.remote_session import PythonShell
-            # the example assumes you're already connected to a tg_node
-            session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
-
-            # then importing the modules needed to run the server
-            # and the modules for any functions later added to the server
-            session.send_command("import xmlrpc")
-            session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
-
-            # sending the source code of this class to the Python shell
-            from xmlrpc.server import SimpleXMLRPCServer
-            src = inspect.getsource(QuittableXMLRPCServer)
-            src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
-            spacing = "\n" * 4
-            session.send_command(spacing + src + spacing)
-
-            # then starting the server with:
-            command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
-            session.send_command(command, "XMLRPC OK")
-
-            # now the server is running on the remote node and we can add functions to it
-            # first connect to the server from the execution node
-            import xmlrpc.client
-            server_url = f"http://{tg_node.config.hostname}:8000"
-            rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
-
-            # get the function bytes to send
-            import marshal
-            function_bytes = marshal.dumps(hello_world.__code__)
-            rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
-
-            # now we can execute the function on the server
-            xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
-            print(str(xmlrpc_binary_recv))
+    Note that the order of inheritance is important for this class. In order to instantiate this
+    class, the abstract methods of :class:`~.capturing_traffic_generator.CapturingTrafficGenerator`
+    must be implemented. Since some of these methods are implemented in the underlying interactive
+    shell, according to Python's Method Resolution Order (MRO), the interactive shell must come
+    first.
     """
 
-    def __init__(self, *args, **kwargs):
-        """Extend the XML-RPC server initialization.
-
-        Args:
-            args: The positional arguments that will be passed to the superclass's constructor.
-            kwargs: The keyword arguments that will be passed to the superclass's constructor.
-                The `allow_none` argument will be set to :data:`True`.
-        """
-        kwargs["allow_none"] = True
-        super().__init__(*args, **kwargs)
-        self.register_introspection_functions()
-        self.register_function(self.quit)
-        self.register_function(self.add_rpc_function)
-
-    def quit(self) -> None:
-        """Quit the server."""
-        self._BaseServer__shutdown_request = True
-        return None
-
-    def add_rpc_function(self, name: str, function_bytes: xmlrpc.client.Binary) -> None:
-        """Add a function to the server from the local server proxy.
-
-        Args:
-              name: The name of the function.
-              function_bytes: The code of the function.
-        """
-        function_code = marshal.loads(function_bytes.data)
-        function = types.FunctionType(function_code, globals(), name)
-        self.register_function(function)
-
-    def serve_forever(self, poll_interval: float = 0.5) -> None:
-        """Extend the superclass method with an additional print.
-
-        Once executed in the local server proxy, the print gives us a clear string to expect
-        when starting the server. The print means this function was executed on the XML-RPC server.
-        """
-        print("XMLRPC OK")
-        super().serve_forever(poll_interval)
-
-
-class ScapyTrafficGenerator(CapturingTrafficGenerator):
-    """Provides access to scapy functions via an RPC interface.
-
-    This class extends the base with remote execution of scapy functions.
-
-    Any packets sent to the remote server are first converted to bytes. They are received as
-    :class:`~xmlrpc.client.Binary` objects on the server side. When the server sends the packets
-    back, they are also received as :class:`~xmlrpc.client.Binary` objects on the client side, are
-    converted back to :class:`~scapy.packet.Packet` objects and only then returned from the methods.
-
-    Attributes:
-        session: The exclusive interactive remote session created by the Scapy
-            traffic generator where the XML-RPC server runs.
-        rpc_server_proxy: The object used by clients to execute functions
-            on the XML-RPC server.
-    """
-
-    session: PythonShell
-    rpc_server_proxy: xmlrpc.client.ServerProxy
     _config: ScapyTrafficGeneratorConfig
 
+    #: Name of sniffer to ensure the same is used in all places
+    _sniffer_name: ClassVar[str] = "sniffer"
+    #: Name of variable that points to the list of packets inside the scapy shell.
+    _send_packet_list_name: ClassVar[str] = "packets"
+    #: Padding to add to the start of a line for python syntax compliance.
+    _padding: ClassVar[str] = " " * 4
+
     def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
         """Extend the constructor with Scapy TG specifics.
 
-        The traffic generator first starts an XML-RPC on the remote `tg_node`.
-        Then it populates the server with functions which use the Scapy library
-        to send/receive traffic:
-
-            * :func:`scapy_send_packets_and_capture`
-            * :func:`scapy_send_packets`
-
-        To enable verbose logging from the xmlrpc client, use the :option:`--verbose`
-        command line argument or the :envvar:`DTS_VERBOSE` environment variable.
+        Initializes both a traffic generator and an interactive shell to handle Scapy functions.
+        The interactive shell will be started on `tg_node`.
 
         Args:
             tg_node: The node where the traffic generator resides.
             config: The traffic generator's test run configuration.
         """
-        super().__init__(tg_node, config)
+        CapturingTrafficGenerator.__init__(self, tg_node, config)
+        PythonShell.__init__(self, tg_node, privileged=True)
 
         assert (
             self._tg_node.config.os == OS.linux
         ), "Linux is the only supported OS for scapy traffic generation"
 
-        self.session = PythonShell(
-            self._tg_node, timeout=5, privileged=True, name="ScapyXMLRPCServer"
-        )
+    def start_application(self) -> None:
+        """Extends :meth:`framework.remote_session.interactive_shell.start_application`.
 
-        # import libs in remote python console
-        for import_statement in SCAPY_RPC_SERVER_IMPORTS:
-            self.session.send_command(import_statement)
+        Adds a command that imports everything from the scapy library immediately after starting
+        the shell for usage in later calls to the methods of this class.
+        """
+        super().start_application()
+        self.send_command("from scapy.all import *")
 
-        # start the server
-        xmlrpc_server_listen_port = 8000
-        self._start_xmlrpc_server_in_remote_python(xmlrpc_server_listen_port)
+    def _create_packet_list(self, packets: list[Packet]) -> None:
+        """Build a list of packets to send later.
 
-        # connect to the server
-        server_url = f"http://{self._tg_node.config.hostname}:{xmlrpc_server_listen_port}"
-        self.rpc_server_proxy = xmlrpc.client.ServerProxy(
-            server_url, allow_none=True, verbose=SETTINGS.verbose
-        )
+        Sends the string that represents the Python command that was used to create each packet in
+        `packets` into the underlying Python session. The purpose behind doing this is to create a
+        list that is identical to `packets` inside the shell. This method should only be called by
+        methods for sending packets immediately prior to sending. The list of packets will continue
+        to exist in the scope of the shell until subsequent calls to this method, so failure to
+        rebuild the list prior to sending packets could lead to undesired "stale" packets to be
+        sent.
 
-        # add functions to the server
-        for function in RPC_FUNCTIONS:
-            # A slightly hacky way to move a function to the remote server.
-            # It is constructed from the name and code on the other side.
-            # Pickle cannot handle functions, nor can any of the other serialization
-            # frameworks aside from the libraries used to generate pyc files, which
-            # are even more messy to work with.
-            function_bytes = marshal.dumps(function.__code__)
-            self.rpc_server_proxy.add_rpc_function(function.__name__, function_bytes)
-
-    def _start_xmlrpc_server_in_remote_python(self, listen_port: int) -> None:
-        # load the source of the function
-        src = inspect.getsource(QuittableXMLRPCServer)
-        # Lines with only whitespace break the repl if in the middle of a function
-        # or class, so strip all lines containing only whitespace
-        src = "\n".join([line for line in src.splitlines() if not line.isspace() and line != ""])
-
-        # execute it in the python terminal
-        self.session.send_command(src + "\n")
-        self.session.send_command(
-            f"server = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));server.serve_forever()",
-            "XMLRPC OK",
+        Args:
+            packets: The list of packets to recreate in the shell.
+        """
+        self._logger.info("Building a list of packets to send.")
+        self.send_command(
+            f"{self._send_packet_list_name} = [{', '.join(map(Packet.command, packets))}]"
         )
 
     def _send_packets(self, packets: list[Packet], port: Port) -> None:
-        packets = [packet.build() for packet in packets]
-        self.rpc_server_proxy.scapy_send_packets(packets, port.logical_name)
+        """Implementation for sending packets without capturing any received traffic.
+
+        Provides a "fire and forget" method of sending packets.
+        """
+        self._create_packet_list(packets)
+        send_command = [
+            "sendp(",
+            f"{self._send_packet_list_name},",
+            f"iface='{port.logical_name}',",
+            "realtime=True,",
+            "verbose=True",
+            ")",
+        ]
+        self.send_command(f"\n{self._padding}".join(send_command))
+
+    def _create_sniffer(
+        self, packets_to_send: list[Packet], send_port: Port, recv_port: Port, filter_config: str
+    ) -> None:
+        """Create an asynchronous sniffer in the shell.
+
+        A list of packets to send is added to the sniffer inside of a callback function so that
+        they are immediately sent at the time sniffing is started.
+
+        Args:
+            packets_to_send: A list of packets to send when sniffing is started.
+            send_port: The port to send the packets on when sniffing is started.
+            recv_port: The port to collect the traffic from.
+            filter_config: An optional BPF format filter to use when sniffing for packets. Omitted
+                when set to an empty string.
+        """
+        self._create_packet_list(packets_to_send)
+        sniffer_commands = [
+            f"{self._sniffer_name} = AsyncSniffer(",
+            f"iface='{recv_port.logical_name}',",
+            "store=True,",
+            "started_callback=lambda *args: sendp(",
+            f"{self._padding}{self._send_packet_list_name}, iface='{send_port.logical_name}'),",
+            ")",
+        ]
+        if filter_config:
+            sniffer_commands.insert(-1, f"filter='{filter_config}'")
+
+        self.send_command(f"\n{self._padding}".join(sniffer_commands))
+
+    def _start_and_stop_sniffing(self, duration: float) -> list[Packet]:
+        """Start asynchronous sniffer, run for a set `duration`, then collect received packets.
+
+        This method expects that you have first created an asynchronous sniffer inside the shell
+        and will fail if you haven't. Received packets are collected by printing the base64
+        encoding of each packet in the shell and then harvesting these encodings using regex to
+        convert back into packet objects.
+
+        Args:
+            duration: The amount of time in seconds to sniff for received packets.
+
+        Returns:
+            A list of all packets that were received while the sniffer was running.
+        """
+        sniffed_packets_name = "gathered_packets"
+        self.send_command(f"{self._sniffer_name}.start()")
+        time.sleep(duration)
+        self.send_command(f"{sniffed_packets_name} = {self._sniffer_name}.stop(join=True)")
+        # An extra newline is required here due to the nature of interactive Python shells
+        packet_strs = self.send_command(
+            f"for pakt in {sniffed_packets_name}: print(bytes_base64(pakt.build()))\n"
+        )
+        # In the string of bytes "b'XXXX'", we only want the contents ("XXXX")
+        list_of_packets_base64 = re.findall(
+            f"^b'({REGEX_FOR_BASE64_ENCODING})'", packet_strs, re.MULTILINE
+        )
+        return [Ether(base64_bytes(pakt)) for pakt in list_of_packets_base64]
 
     def _create_packet_filter(self, filter_config: PacketFilteringConfig) -> str:
-        """Combines filter settings from `filter_config` into a BPF that scapy can use.
+        """Combine filter settings from `filter_config` into a BPF that scapy can use.
 
         Scapy allows for the use of Berkeley Packet Filters (BPFs) to filter what packets are
         collected based on various attributes of the packet.
@@ -333,32 +200,19 @@ def _send_packets_and_capture(
         self,
         packets: list[Packet],
         send_port: Port,
-        receive_port: Port,
+        recv_port: Port,
         filter_config: PacketFilteringConfig,
         duration: float,
-        capture_name: str = _get_default_capture_name(),
     ) -> list[Packet]:
-        binary_packets = [packet.build() for packet in packets]
-
-        xmlrpc_packets: list[
-            xmlrpc.client.Binary
-        ] = self.rpc_server_proxy.scapy_send_packets_and_capture(
-            binary_packets,
-            send_port.logical_name,
-            receive_port.logical_name,
-            duration,
-            self._create_packet_filter(filter_config),
-        )  # type: ignore[assignment]
-
-        scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets]
-        return scapy_packets
-
-    def close(self) -> None:
-        """Close the traffic generator."""
-        try:
-            self.rpc_server_proxy.quit()
-        except ConnectionRefusedError:
-            # Because the python instance closes, we get no RPC response.
-            # Thus, this error is expected
-            pass
-        self.session.close()
+        """Implementation for sending packets and capturing any received traffic.
+
+        This method first creates an asynchronous sniffer that holds the packets to send, then
+        starts and stops and starts said sniffer.
+
+        Returns:
+            A list of packets received after sending `packets`.
+        """
+        self._create_sniffer(
+            packets, send_port, recv_port, self._create_packet_filter(filter_config)
+        )
+        return self._start_and_stop_sniffing(duration)
diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index 6b5d5a805f..aa213c188f 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -27,6 +27,7 @@
 from .exception import ConfigurationError
 
 REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
+REGEX_FOR_BASE64_ENCODING: str = "[-a-zA-Z0-9+\\/]*={0,3}"
 
 
 def expand_range(range_str: str) -> list[int]:
-- 
2.45.2

Re: [PATCH v1 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[Juraj Linkeš]

> +    #: Padding to add to the start of a line for python syntax compliance.
> +    _padding: ClassVar[str] = " " * 4

We use padding in the context of packets so let's use something else 
here, such as _python_indentation.

> +
>       def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
>           """Extend the constructor with Scapy TG specifics.
>   
> -        The traffic generator first starts an XML-RPC on the remote `tg_node`.
> -        Then it populates the server with functions which use the Scapy library
> -        to send/receive traffic:
> -
> -            * :func:`scapy_send_packets_and_capture`
> -            * :func:`scapy_send_packets`
> -
> -        To enable verbose logging from the xmlrpc client, use the :option:`--verbose`
> -        command line argument or the :envvar:`DTS_VERBOSE` environment variable.
> +        Initializes both a traffic generator and an interactive shell to handle Scapy functions.

Should these be with the definite article (instead of a and an)?

> +        The interactive shell will be started on `tg_node`.
>   
>           Args:
>               tg_node: The node where the traffic generator resides.
>               config: The traffic generator's test run configuration.
>           """
> -        super().__init__(tg_node, config)
> +        CapturingTrafficGenerator.__init__(self, tg_node, config)
> +        PythonShell.__init__(self, tg_node, privileged=True)
>   

This should probably mirror the superclass order from which the class is 
subclassed - PythonShell first, then CapturingTrafficGenerator.	

I'm also thinking of using super() here, but it's a bit convoluted. We'd 
need to do something like this:

class Common:
     def __init__(self, *args, **kwargs):
         super().__init__()

class TrafficGenerator(Common):
     def __init__(self, tg_node, config, **kwargs):
         super().__init__(tg_node, **kwargs)

class InteractiveShell(Common):
     def __init__(self, node, privileged = False, timeout = 15, 
start_on_init = True, params = "", **kwargs):
         super().__init__(node, **kwargs)

This works for both mro's. The common class basically makes the two 
subclasses eligible for multiple interitance while still working with 
normal inheritance (e.g. when making an instance of regular PythonShell) 
and the super() calls in the subclasses define what they have in common.

Since we only have this one use case, it may be overkill to use super() 
instead of calling both __init__()s directly. But with super(), we 
should be able to use any traffic generator with any interactive shell. 
I don't know whether we'll need that (we can use T-Rex without an 
interactive shell, but maybe using one is superior), but it could be useful.

>           assert (
>               self._tg_node.config.os == OS.linux
>           ), "Linux is the only supported OS for scapy traffic generation"
>   
> -        self.session = PythonShell(
> -            self._tg_node, timeout=5, privileged=True, name="ScapyXMLRPCServer"
> -        )
> +    def start_application(self) -> None:
> +        """Extends :meth:`framework.remote_session.interactive_shell.start_application`.
>   
> -        # import libs in remote python console
> -        for import_statement in SCAPY_RPC_SERVER_IMPORTS:
> -            self.session.send_command(import_statement)
> +        Adds a command that imports everything from the scapy library immediately after starting

This sound as if we were adding the command to some sort of internal 
list for processing later. We can probably just say "Import everything 
... in the remote shell.". I think it would be valuable to explicitly 
say where the import happens.

> +        the shell for usage in later calls to the methods of this class.
> +        """
> +        super().start_application()
> +        self.send_command("from scapy.all import *")
>   

It's possible we should check that the import was successful or do some 
other check that Scapy is indeed present and throw an exception if it isn't.

Actually, we probably should add a smoke test that would do that - some 
basic verification of traffic generators.

Re: [PATCH v1 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[Jeremy Spewock]

On Fri, Jun 21, 2024 at 10:14 AM Juraj Linkeš
<juraj.linkes@pantheon.tech> wrote:
>
>
> > +    #: Padding to add to the start of a line for python syntax compliance.
> > +    _padding: ClassVar[str] = " " * 4
>
> We use padding in the context of packets so let's use something else
> here, such as _python_indentation.

I didn't think of that, good call.

>
> > +
> >       def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
> >           """Extend the constructor with Scapy TG specifics.
> >
> > -        The traffic generator first starts an XML-RPC on the remote `tg_node`.
> > -        Then it populates the server with functions which use the Scapy library
> > -        to send/receive traffic:
> > -
> > -            * :func:`scapy_send_packets_and_capture`
> > -            * :func:`scapy_send_packets`
> > -
> > -        To enable verbose logging from the xmlrpc client, use the :option:`--verbose`
> > -        command line argument or the :envvar:`DTS_VERBOSE` environment variable.
> > +        Initializes both a traffic generator and an interactive shell to handle Scapy functions.
>
> Should these be with the definite article (instead of a and an)?

Yeah, I can see how that would make sense. I'll update this.

>
> > +        The interactive shell will be started on `tg_node`.
> >
> >           Args:
> >               tg_node: The node where the traffic generator resides.
> >               config: The traffic generator's test run configuration.
> >           """
> > -        super().__init__(tg_node, config)
> > +        CapturingTrafficGenerator.__init__(self, tg_node, config)
> > +        PythonShell.__init__(self, tg_node, privileged=True)
> >
>
> This should probably mirror the superclass order from which the class is
> subclassed - PythonShell first, then CapturingTrafficGenerator.

Ack.

>
> I'm also thinking of using super() here, but it's a bit convoluted. We'd
> need to do something like this:
>
> class Common:
>      def __init__(self, *args, **kwargs):
>          super().__init__()
>
> class TrafficGenerator(Common):
>      def __init__(self, tg_node, config, **kwargs):
>          super().__init__(tg_node, **kwargs)
>
> class InteractiveShell(Common):
>      def __init__(self, node, privileged = False, timeout = 15,
> start_on_init = True, params = "", **kwargs):
>          super().__init__(node, **kwargs)
>
> This works for both mro's. The common class basically makes the two
> subclasses eligible for multiple interitance while still working with
> normal inheritance (e.g. when making an instance of regular PythonShell)
> and the super() calls in the subclasses define what they have in common.
>
> Since we only have this one use case, it may be overkill to use super()
> instead of calling both __init__()s directly. But with super(), we
> should be able to use any traffic generator with any interactive shell.
> I don't know whether we'll need that (we can use T-Rex without an
> interactive shell, but maybe using one is superior), but it could be useful.

It could be useful, but it does increase the complexity a little bit.
I could put it together and see how it looks. It definitely would be
better to use super so we don't have to unnecessarily override the
init method in the future, the only thing that would deter me slightly
is the seemingly random `**kwargs` that don't get used inside the
interactive shell class. Of course this ambiguity goes away if it's
documented, so this shouldn't really be a problem.

>
> >           assert (
> >               self._tg_node.config.os == OS.linux
> >           ), "Linux is the only supported OS for scapy traffic generation"
> >
> > -        self.session = PythonShell(
> > -            self._tg_node, timeout=5, privileged=True, name="ScapyXMLRPCServer"
> > -        )
> > +    def start_application(self) -> None:
> > +        """Extends :meth:`framework.remote_session.interactive_shell.start_application`.
> >
> > -        # import libs in remote python console
> > -        for import_statement in SCAPY_RPC_SERVER_IMPORTS:
> > -            self.session.send_command(import_statement)
> > +        Adds a command that imports everything from the scapy library immediately after starting
>
> This sound as if we were adding the command to some sort of internal
> list for processing later. We can probably just say "Import everything
> ... in the remote shell.". I think it would be valuable to explicitly
> say where the import happens.

That makes sense, I'll make the change.

>
> > +        the shell for usage in later calls to the methods of this class.
> > +        """
> > +        super().start_application()
> > +        self.send_command("from scapy.all import *")
> >
>
> It's possible we should check that the import was successful or do some
> other check that Scapy is indeed present and throw an exception if it isn't.
>

That's not a bad idea, I could try and just use something from the
scapy library and check the output.

> Actually, we probably should add a smoke test that would do that - some
> basic verification of traffic generators.

This idea I like a lot! We've had the traffic generator itself
implemented for a while now, a smoke test that ensures traffic can
flow in general would definitely be beneficial.

>

[PATCH v2 0/1] dts: replace XML-RPC server

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

v2:
 * Address comments left on the previous version about naming and
   documentation
 * Change the multiple-inheritance in the scapy traffic generator to use
   super instead of calling the init methods of the classes directly.

Jeremy Spewock (1):
  dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

 .../remote_session/interactive_shell.py       |  11 +-
 .../traffic_generator/__init__.py             |   2 +-
 .../testbed_model/traffic_generator/scapy.py  | 425 ++++++------------
 .../traffic_generator/traffic_generator.py    |  15 +-
 dts/framework/utils.py                        |  15 +
 5 files changed, 177 insertions(+), 291 deletions(-)

-- 
2.45.2

[PATCH v2 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously all scapy commands were handled using an XML-RPC server that
ran on the TGNode. This unnecessarily enforces a minimum Python version
of 3.10 on the server that is being used as a traffic generator and
complicates the implementation of scapy methods. This patch removes the
XML-RPC server completely and instead allows the Scapy TG to extend from
the PythonShell to implement the functionality of a traffic generator.
This is done by importing the Scapy library in the PythonShell and
sending commands directly to the interactive session on the TG Node.

Bugzilla ID: 1374
depends-on: series-32242 ("Improve interactive shell output gathering
and logging")

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
Something I would like to note about this patch is the use of **kwargs
without proper typing is unintuitive. Something that could be done to
solve this could be using Unpack (like is done in other places) to
create a more useful type for the parameters that are allowed/expected,
but I couldn't find a good way to do this without refacotring the
parameters of interactive shells to be denoted using TypedDicts, which I
felt would be a little out-of-scope for this patch and what it is trying
to do.

However, I think using TypedDicts and an Unpack for
InteractiveShells could be an interesting approach that might allow us
to save some copy-pasting when sub-classing the shell. Instead of
restating all of the existing parameters and then adding more to the
subclasses, you could simply subclass a parameters dict and add more as
needed. Then, the method signatures for interactive shells would just
become `**kwargs: Unpack[InteractiveShellParams]` and the signature for
multiple inheritance using the interactive shells would be much more
useful since you could unpack the same TypedDict of parameters.

Additionally, I believe that the TestPmdShell is incompatible with
this approach to multiple-inheritance due to how it uses key-word
arguments.

 .../remote_session/interactive_shell.py       |  11 +-
 .../traffic_generator/__init__.py             |   2 +-
 .../testbed_model/traffic_generator/scapy.py  | 425 ++++++------------
 .../traffic_generator/traffic_generator.py    |  15 +-
 dts/framework/utils.py                        |  15 +
 5 files changed, 177 insertions(+), 291 deletions(-)

diff --git a/dts/framework/remote_session/interactive_shell.py b/dts/framework/remote_session/interactive_shell.py
index c92fdbfcdf..b723a50b48 100644
--- a/dts/framework/remote_session/interactive_shell.py
+++ b/dts/framework/remote_session/interactive_shell.py
@@ -29,16 +29,18 @@
 from framework.params import Params
 from framework.settings import SETTINGS
 from framework.testbed_model.node import Node
+from framework.utils import MultiInheritanceBaseClass
 
 
-class InteractiveShell(ABC):
+class InteractiveShell(MultiInheritanceBaseClass, ABC):
     """The base class for managing interactive shells.
 
     This class shouldn't be instantiated directly, but instead be extended. It contains
     methods for starting interactive shells as well as sending commands to these shells
     and collecting input until reaching a certain prompt. All interactive applications
     will use the same SSH connection, but each will create their own channel on that
-    session.
+    session. This class also extends from :class:`framework.utils.MultiInheritanceBaseClass` to
+    allow for both single- and multiple-inheritance.
     """
 
     _node: Node
@@ -74,9 +76,13 @@ def __init__(
         start_on_init: bool = True,
         app_params: Params = Params(),
         name: str | None = None,
+        **kwargs,
     ) -> None:
         """Create an SSH channel during initialization.
 
+        Additional key-word arguments can be passed through `kwargs` is needed to fulfill other
+        constructors in the case of multiple-inheritance.
+
         Args:
             node: The node on which to run start the interactive shell.
             privileged: Enables the shell to run as superuser.
@@ -100,6 +106,7 @@ def __init__(
 
         if start_on_init:
             self.start_application()
+        super().__init__(node, **kwargs)
 
     def _setup_ssh_channel(self):
         self._ssh_channel = self._node.main_session.interactive_session.session.invoke_shell()
diff --git a/dts/framework/testbed_model/traffic_generator/__init__.py b/dts/framework/testbed_model/traffic_generator/__init__.py
index 6dac86a224..a319fa5320 100644
--- a/dts/framework/testbed_model/traffic_generator/__init__.py
+++ b/dts/framework/testbed_model/traffic_generator/__init__.py
@@ -36,7 +36,7 @@ def create_traffic_generator(
     """
     match traffic_generator_config:
         case ScapyTrafficGeneratorConfig():
-            return ScapyTrafficGenerator(tg_node, traffic_generator_config)
+            return ScapyTrafficGenerator(tg_node, traffic_generator_config, privileged=True)
         case _:
             raise ConfigurationError(
                 f"Unknown traffic generator: {traffic_generator_config.traffic_generator_type}"
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index ca0ea6aca3..b3a8b188a1 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -6,309 +6,179 @@
 
 A traffic generator used for functional testing, implemented with
 `the Scapy library <https://scapy.readthedocs.io/en/latest/>`_.
-The traffic generator uses an XML-RPC server to run Scapy on the remote TG node.
+The traffic generator uses an interactive shell to run Scapy on the remote TG node.
 
-The traffic generator uses the :mod:`xmlrpc.server` module to run an XML-RPC server
-in an interactive remote Python SSH session. The communication with the server is facilitated
-with a local server proxy from the :mod:`xmlrpc.client` module.
+The traffic generator extends :class:`framework.remote_session.python_shell.PythonShell` to
+implement the methods for handling packets by sending commands into the interactive shell.
 """
 
-import inspect
-import marshal
+
+import re
 import time
-import types
-import xmlrpc.client
-from xmlrpc.server import SimpleXMLRPCServer
+from typing import ClassVar
 
-import scapy.all  # type: ignore[import-untyped]
+from scapy.compat import base64_bytes  # type: ignore[import-untyped]
 from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
 from scapy.packet import Packet  # type: ignore[import-untyped]
 
 from framework.config import OS, ScapyTrafficGeneratorConfig
 from framework.remote_session.python_shell import PythonShell
-from framework.settings import SETTINGS
 from framework.testbed_model.node import Node
 from framework.testbed_model.port import Port
-
-from .capturing_traffic_generator import (
-    CapturingTrafficGenerator,
+from framework.testbed_model.traffic_generator.capturing_traffic_generator import (
     PacketFilteringConfig,
-    _get_default_capture_name,
 )
+from framework.utils import REGEX_FOR_BASE64_ENCODING
 
-"""
-========= BEGIN RPC FUNCTIONS =========
-
-All of the functions in this section are intended to be exported to a python
-shell which runs a scapy RPC server. These functions are made available via that
-RPC server to the packet generator. To add a new function to the RPC server,
-first write the function in this section. Then, if you need any imports, make sure to
-add them to SCAPY_RPC_SERVER_IMPORTS as well. After that, add the function to the list
-in EXPORTED_FUNCTIONS. Note that kwargs (keyword arguments) do not work via xmlrpc,
-so you may need to construct wrapper functions around many scapy types.
-"""
-
-"""
-Add the line needed to import something in a normal python environment
-as an entry to this array. It will be imported before any functions are
-sent to the server.
-"""
-SCAPY_RPC_SERVER_IMPORTS = [
-    "from scapy.all import *",
-    "import xmlrpc",
-    "import sys",
-    "from xmlrpc.server import SimpleXMLRPCServer",
-    "import marshal",
-    "import pickle",
-    "import types",
-    "import time",
-]
-
-
-def scapy_send_packets_and_capture(
-    xmlrpc_packets: list[xmlrpc.client.Binary],
-    send_iface: str,
-    recv_iface: str,
-    duration: float,
-    sniff_filter: str,
-) -> list[bytes]:
-    """The RPC function to send and capture packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-        recv_iface: The logical name of the ingress interface.
-        duration: Capture for this amount of time, in seconds.
-
-    Returns:
-        A list of bytes. Each item in the list represents one packet, which needs
-        to be converted back upon transfer from the remote node.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    sniffer = scapy.all.AsyncSniffer(
-        iface=recv_iface,
-        store=True,
-        started_callback=lambda *args: scapy.all.sendp(scapy_packets, iface=send_iface),
-        filter=sniff_filter,
-    )
-    sniffer.start()
-    time.sleep(duration)
-    return [scapy_packet.build() for scapy_packet in sniffer.stop(join=True)]
-
-
-def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: str) -> None:
-    """The RPC function to send packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-    It only sends `xmlrpc_packets`, without capturing them.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    scapy.all.sendp(scapy_packets, iface=send_iface, realtime=True, verbose=True)
-
-
-"""
-Functions to be exposed by the scapy RPC server.
-"""
-RPC_FUNCTIONS = [
-    scapy_send_packets,
-    scapy_send_packets_and_capture,
-]
-
-"""
-========= END RPC FUNCTIONS =========
-"""
-
-
-class QuittableXMLRPCServer(SimpleXMLRPCServer):
-    r"""Basic XML-RPC server.
+from .capturing_traffic_generator import CapturingTrafficGenerator
 
-    The server may be augmented by functions serializable by the :mod:`marshal` module.
 
-    Example:
-        ::
+class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
+    """Provides access to scapy functions on a traffic generator node.
 
-            def hello_world():
-                # to be sent to the XML-RPC server
-                print("Hello World!")
+    This class extends the base with remote execution of scapy functions. All methods for
+    processing packets are implemented using an underlying
+    :class:`framework.remote_session.python_shell.PythonShell` which imports the Scapy library.
 
-            # start the XML-RPC server on the remote node
-            # this is done by starting a Python shell on the remote node
-            from framework.remote_session import PythonShell
-            # the example assumes you're already connected to a tg_node
-            session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
+    Note that the order of inheritance is important for this class. In order to instantiate this
+    class, the abstract methods of :class:`~.capturing_traffic_generator.CapturingTrafficGenerator`
+    must be implemented. Since some of these methods are implemented in the underlying interactive
+    shell, according to Python's Method Resolution Order (MRO), the interactive shell must come
+    first.
+    """
 
-            # then importing the modules needed to run the server
-            # and the modules for any functions later added to the server
-            session.send_command("import xmlrpc")
-            session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
+    _config: ScapyTrafficGeneratorConfig
 
-            # sending the source code of this class to the Python shell
-            from xmlrpc.server import SimpleXMLRPCServer
-            src = inspect.getsource(QuittableXMLRPCServer)
-            src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
-            spacing = "\n" * 4
-            session.send_command(spacing + src + spacing)
+    #: Name of sniffer to ensure the same is used in all places
+    _sniffer_name: ClassVar[str] = "sniffer"
+    #: Name of variable that points to the list of packets inside the scapy shell.
+    _send_packet_list_name: ClassVar[str] = "packets"
+    #: Padding to add to the start of a line for python syntax compliance.
+    _python_indentation: ClassVar[str] = " " * 4
 
-            # then starting the server with:
-            command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
-            session.send_command(command, "XMLRPC OK")
+    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig, **kwargs):
+        """Extend the constructor with Scapy TG specifics.
 
-            # now the server is running on the remote node and we can add functions to it
-            # first connect to the server from the execution node
-            import xmlrpc.client
-            server_url = f"http://{tg_node.config.hostname}:8000"
-            rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
+        Initializes both the traffic generator and the interactive shell used to handle Scapy
+        functions. The interactive shell will be started on `tg_node`. The additional key-word
+        arguments in `kwargs` are used to pass into the constructor for the interactive shell.
 
-            # get the function bytes to send
-            import marshal
-            function_bytes = marshal.dumps(hello_world.__code__)
-            rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
+        Args:
+            tg_node: The node where the traffic generator resides.
+            config: The traffic generator's test run configuration.
+        """
+        assert (
+            tg_node.config.os == OS.linux
+        ), "Linux is the only supported OS for scapy traffic generation"
 
-            # now we can execute the function on the server
-            xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
-            print(str(xmlrpc_binary_recv))
-    """
+        super().__init__(tg_node, config=config, **kwargs)
 
-    def __init__(self, *args, **kwargs):
-        """Extend the XML-RPC server initialization.
+    def start_application(self) -> None:
+        """Extends :meth:`framework.remote_session.interactive_shell.start_application`.
 
-        Args:
-            args: The positional arguments that will be passed to the superclass's constructor.
-            kwargs: The keyword arguments that will be passed to the superclass's constructor.
-                The `allow_none` argument will be set to :data:`True`.
+        Adds a command that imports everything from the scapy library immediately after starting
+        the shell for usage in later calls to the methods of this class.
         """
-        kwargs["allow_none"] = True
-        super().__init__(*args, **kwargs)
-        self.register_introspection_functions()
-        self.register_function(self.quit)
-        self.register_function(self.add_rpc_function)
+        super().start_application()
+        self.send_command("from scapy.all import *")
 
-    def quit(self) -> None:
-        """Quit the server."""
-        self._BaseServer__shutdown_request = True
-        return None
+    def _create_packet_list(self, packets: list[Packet]) -> None:
+        """Build a list of packets to send later.
 
-    def add_rpc_function(self, name: str, function_bytes: xmlrpc.client.Binary) -> None:
-        """Add a function to the server from the local server proxy.
+        Sends the string that represents the Python command that was used to create each packet in
+        `packets` into the underlying Python session. The purpose behind doing this is to create a
+        list that is identical to `packets` inside the shell. This method should only be called by
+        methods for sending packets immediately prior to sending. The list of packets will continue
+        to exist in the scope of the shell until subsequent calls to this method, so failure to
+        rebuild the list prior to sending packets could lead to undesired "stale" packets to be
+        sent.
 
         Args:
-              name: The name of the function.
-              function_bytes: The code of the function.
+            packets: The list of packets to recreate in the shell.
         """
-        function_code = marshal.loads(function_bytes.data)
-        function = types.FunctionType(function_code, globals(), name)
-        self.register_function(function)
+        self._logger.info("Building a list of packets to send.")
+        self.send_command(
+            f"{self._send_packet_list_name} = [{', '.join(map(Packet.command, packets))}]"
+        )
 
-    def serve_forever(self, poll_interval: float = 0.5) -> None:
-        """Extend the superclass method with an additional print.
+    def _send_packets(self, packets: list[Packet], port: Port) -> None:
+        """Implementation for sending packets without capturing any received traffic.
 
-        Once executed in the local server proxy, the print gives us a clear string to expect
-        when starting the server. The print means this function was executed on the XML-RPC server.
+        Provides a "fire and forget" method of sending packets.
         """
-        print("XMLRPC OK")
-        super().serve_forever(poll_interval)
-
-
-class ScapyTrafficGenerator(CapturingTrafficGenerator):
-    """Provides access to scapy functions via an RPC interface.
-
-    This class extends the base with remote execution of scapy functions.
-
-    Any packets sent to the remote server are first converted to bytes. They are received as
-    :class:`~xmlrpc.client.Binary` objects on the server side. When the server sends the packets
-    back, they are also received as :class:`~xmlrpc.client.Binary` objects on the client side, are
-    converted back to :class:`~scapy.packet.Packet` objects and only then returned from the methods.
-
-    Attributes:
-        session: The exclusive interactive remote session created by the Scapy
-            traffic generator where the XML-RPC server runs.
-        rpc_server_proxy: The object used by clients to execute functions
-            on the XML-RPC server.
-    """
-
-    session: PythonShell
-    rpc_server_proxy: xmlrpc.client.ServerProxy
-    _config: ScapyTrafficGeneratorConfig
-
-    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
-        """Extend the constructor with Scapy TG specifics.
-
-        The traffic generator first starts an XML-RPC on the remote `tg_node`.
-        Then it populates the server with functions which use the Scapy library
-        to send/receive traffic:
-
-            * :func:`scapy_send_packets_and_capture`
-            * :func:`scapy_send_packets`
-
-        To enable verbose logging from the xmlrpc client, use the :option:`--verbose`
-        command line argument or the :envvar:`DTS_VERBOSE` environment variable.
+        self._create_packet_list(packets)
+        send_command = [
+            "sendp(",
+            f"{self._send_packet_list_name},",
+            f"iface='{port.logical_name}',",
+            "realtime=True,",
+            "verbose=True",
+            ")",
+        ]
+        self.send_command(f"\n{self._python_indentation}".join(send_command))
+
+    def _create_sniffer(
+        self, packets_to_send: list[Packet], send_port: Port, recv_port: Port, filter_config: str
+    ) -> None:
+        """Create an asynchronous sniffer in the shell.
+
+        A list of packets to send is added to the sniffer inside of a callback function so that
+        they are immediately sent at the time sniffing is started.
 
         Args:
-            tg_node: The node where the traffic generator resides.
-            config: The traffic generator's test run configuration.
+            packets_to_send: A list of packets to send when sniffing is started.
+            send_port: The port to send the packets on when sniffing is started.
+            recv_port: The port to collect the traffic from.
+            filter_config: An optional BPF format filter to use when sniffing for packets. Omitted
+                when set to an empty string.
         """
-        super().__init__(tg_node, config)
+        self._create_packet_list(packets_to_send)
+        sniffer_commands = [
+            f"{self._sniffer_name} = AsyncSniffer(",
+            f"iface='{recv_port.logical_name}',",
+            "store=True,",
+            "started_callback=lambda *args: sendp(",
+            (
+                f"{self._python_indentation}{self._send_packet_list_name},"
+                f" iface='{send_port.logical_name}'),"
+            ),
+            ")",
+        ]
+        if filter_config:
+            sniffer_commands.insert(-1, f"filter='{filter_config}'")
+
+        self.send_command(f"\n{self._python_indentation}".join(sniffer_commands))
+
+    def _start_and_stop_sniffing(self, duration: float) -> list[Packet]:
+        """Start asynchronous sniffer, run for a set `duration`, then collect received packets.
+
+        This method expects that you have first created an asynchronous sniffer inside the shell
+        and will fail if you haven't. Received packets are collected by printing the base64
+        encoding of each packet in the shell and then harvesting these encodings using regex to
+        convert back into packet objects.
 
-        assert (
-            self._tg_node.config.os == OS.linux
-        ), "Linux is the only supported OS for scapy traffic generation"
-
-        self.session = PythonShell(
-            self._tg_node, timeout=5, privileged=True, name="ScapyXMLRPCServer"
-        )
-
-        # import libs in remote python console
-        for import_statement in SCAPY_RPC_SERVER_IMPORTS:
-            self.session.send_command(import_statement)
-
-        # start the server
-        xmlrpc_server_listen_port = 8000
-        self._start_xmlrpc_server_in_remote_python(xmlrpc_server_listen_port)
+        Args:
+            duration: The amount of time in seconds to sniff for received packets.
 
-        # connect to the server
-        server_url = f"http://{self._tg_node.config.hostname}:{xmlrpc_server_listen_port}"
-        self.rpc_server_proxy = xmlrpc.client.ServerProxy(
-            server_url, allow_none=True, verbose=SETTINGS.verbose
+        Returns:
+            A list of all packets that were received while the sniffer was running.
+        """
+        sniffed_packets_name = "gathered_packets"
+        self.send_command(f"{self._sniffer_name}.start()")
+        time.sleep(duration)
+        self.send_command(f"{sniffed_packets_name} = {self._sniffer_name}.stop(join=True)")
+        # An extra newline is required here due to the nature of interactive Python shells
+        packet_strs = self.send_command(
+            f"for pakt in {sniffed_packets_name}: print(bytes_base64(pakt.build()))\n"
         )
-
-        # add functions to the server
-        for function in RPC_FUNCTIONS:
-            # A slightly hacky way to move a function to the remote server.
-            # It is constructed from the name and code on the other side.
-            # Pickle cannot handle functions, nor can any of the other serialization
-            # frameworks aside from the libraries used to generate pyc files, which
-            # are even more messy to work with.
-            function_bytes = marshal.dumps(function.__code__)
-            self.rpc_server_proxy.add_rpc_function(function.__name__, function_bytes)
-
-    def _start_xmlrpc_server_in_remote_python(self, listen_port: int) -> None:
-        # load the source of the function
-        src = inspect.getsource(QuittableXMLRPCServer)
-        # Lines with only whitespace break the repl if in the middle of a function
-        # or class, so strip all lines containing only whitespace
-        src = "\n".join([line for line in src.splitlines() if not line.isspace() and line != ""])
-
-        # execute it in the python terminal
-        self.session.send_command(src + "\n")
-        self.session.send_command(
-            f"server = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));server.serve_forever()",
-            "XMLRPC OK",
+        # In the string of bytes "b'XXXX'", we only want the contents ("XXXX")
+        list_of_packets_base64 = re.findall(
+            f"^b'({REGEX_FOR_BASE64_ENCODING})'", packet_strs, re.MULTILINE
         )
-
-    def _send_packets(self, packets: list[Packet], port: Port) -> None:
-        packets = [packet.build() for packet in packets]
-        self.rpc_server_proxy.scapy_send_packets(packets, port.logical_name)
+        return [Ether(base64_bytes(pakt)) for pakt in list_of_packets_base64]
 
     def _create_packet_filter(self, filter_config: PacketFilteringConfig) -> str:
-        """Combines filter settings from `filter_config` into a BPF that scapy can use.
+        """Combine filter settings from `filter_config` into a BPF that scapy can use.
 
         Scapy allows for the use of Berkeley Packet Filters (BPFs) to filter what packets are
         collected based on various attributes of the packet.
@@ -333,32 +203,19 @@ def _send_packets_and_capture(
         self,
         packets: list[Packet],
         send_port: Port,
-        receive_port: Port,
+        recv_port: Port,
         filter_config: PacketFilteringConfig,
         duration: float,
-        capture_name: str = _get_default_capture_name(),
     ) -> list[Packet]:
-        binary_packets = [packet.build() for packet in packets]
-
-        xmlrpc_packets: list[
-            xmlrpc.client.Binary
-        ] = self.rpc_server_proxy.scapy_send_packets_and_capture(
-            binary_packets,
-            send_port.logical_name,
-            receive_port.logical_name,
-            duration,
-            self._create_packet_filter(filter_config),
-        )  # type: ignore[assignment]
-
-        scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets]
-        return scapy_packets
-
-    def close(self) -> None:
-        """Close the traffic generator."""
-        try:
-            self.rpc_server_proxy.quit()
-        except ConnectionRefusedError:
-            # Because the python instance closes, we get no RPC response.
-            # Thus, this error is expected
-            pass
-        self.session.close()
+        """Implementation for sending packets and capturing any received traffic.
+
+        This method first creates an asynchronous sniffer that holds the packets to send, then
+        starts and stops and starts said sniffer.
+
+        Returns:
+            A list of packets received after sending `packets`.
+        """
+        self._create_sniffer(
+            packets, send_port, recv_port, self._create_packet_filter(filter_config)
+        )
+        return self._start_and_stop_sniffing(duration)
diff --git a/dts/framework/testbed_model/traffic_generator/traffic_generator.py b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
index 4ce1148706..176d5e9065 100644
--- a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
+++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
@@ -16,23 +16,29 @@
 from framework.logger import DTSLogger, get_dts_logger
 from framework.testbed_model.node import Node
 from framework.testbed_model.port import Port
-from framework.utils import get_packet_summaries
+from framework.utils import MultiInheritanceBaseClass, get_packet_summaries
 
 
-class TrafficGenerator(ABC):
+class TrafficGenerator(MultiInheritanceBaseClass, ABC):
     """The base traffic generator.
 
     Exposes the common public methods of all traffic generators and defines private methods
-    that must implement the traffic generation logic in subclasses.
+    that must implement the traffic generation logic in subclasses. This class also extends from
+    :class:`framework.utils.MultiInheritanceBaseClass` to allow subclasses the ability to inherit
+    from multiple classes to fulfil the traffic generating functionality without breaking
+    single-inheritance.
     """
 
     _config: TrafficGeneratorConfig
     _tg_node: Node
     _logger: DTSLogger
 
-    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
+    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig, **kwargs):
         """Initialize the traffic generator.
 
+        Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other
+        constructors in the case of multiple-inheritance.
+
         Args:
             tg_node: The traffic generator node where the created traffic generator will be running.
             config: The traffic generator's test run configuration.
@@ -40,6 +46,7 @@ def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
         self._config = config
         self._tg_node = tg_node
         self._logger = get_dts_logger(f"{self._tg_node.name} {self._config.traffic_generator_type}")
+        super().__init__(tg_node, **kwargs)
 
     def send_packet(self, packet: Packet, port: Port) -> None:
         """Send `packet` and block until it is fully sent.
diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index 6b5d5a805f..1370ca1fe5 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -27,6 +27,7 @@
 from .exception import ConfigurationError
 
 REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
+REGEX_FOR_BASE64_ENCODING: str = "[-a-zA-Z0-9+\\/]*={0,3}"
 
 
 def expand_range(range_str: str) -> list[int]:
@@ -244,3 +245,17 @@ def _delete_tarball(self) -> None:
     def __fspath__(self) -> str:
         """The os.PathLike protocol implementation."""
         return str(self._tarball_path)
+
+
+class MultiInheritanceBaseClass:
+    """A base class for classes utilizing multiple-inheritance.
+
+    This class enables it's subclasses to support both single and multiple inheritance by acting as
+    a stopping point in the tree of calls to the constructors of super-classes. This class is able
+    to exist at the end of the Method Resolution Order (MRO) so that sub-classes can call super
+    without repercussion.
+    """
+
+    def __init__(self, *args, **kwargs) -> None:
+        """Call the init method of :class:`object`."""
+        super().__init__()
-- 
2.45.2

Re: [PATCH v2 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[Patrick Robb]

Just noting in attempting to test the latest version of this patch I
see it does not cleanly apply.

[PATCH v3 0/1] dts: replace XML-RPC server

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

v3:
  * rebase on next-dts branch

Jeremy Spewock (1):
  dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

 .../single_active_interactive_shell.py        |   7 +-
 .../traffic_generator/__init__.py             |   2 +-
 .../testbed_model/traffic_generator/scapy.py  | 428 ++++++------------
 .../traffic_generator/traffic_generator.py    |  15 +-
 dts/framework/utils.py                        |  15 +
 5 files changed, 175 insertions(+), 292 deletions(-)

-- 
2.46.0


-- 



*Let's Connect!*

...  *October Webinars*

Ask Us Anything: IOL Services 
Open Q&A 
<https://unh.zoom.us/webinar/register/9017265932716/WN_OUo5S7iQRLmKKY7CsmwZhw#/registration>Your 
questions. Our answers. Let's get started.


Oct 3rd


Live Tour of INTACT(R) 
for IPv6 Testing and Validation 
<https://unh.zoom.us/webinar/register/7117231236474/WN_I2zfyi_2S2yEiXkxBRi8sA#/registration>
Open tour. Open Q&A. See why we think you'll love INTACT.

Oct 9th


How to 
Prep for Our NVMe(R) Plugfest #21 
<https://unh.zoom.us/webinar/register/4017266809553/WN_X1iA2SZ8QhmcGboF2DImNg#/registration>
Checklists. Conversation. Let's get ready to plugin! 
Oct 15th


... * 
Newsletter*

*
*
Get the IOL Connector 
<https://www.iol.unh.edu/news/email-newsletters> for our latest news and 
event info.



.

[PATCH v3 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously all scapy commands were handled using an XML-RPC server that
ran on the TGNode. This unnecessarily enforces a minimum Python version
of 3.10 on the server that is being used as a traffic generator and
complicates the implementation of scapy methods. This patch removes the
XML-RPC server completely and instead allows the Scapy TG to extend from
the PythonShell to implement the functionality of a traffic generator.
This is done by importing the Scapy library in the PythonShell and
sending commands directly to the interactive session on the TG Node.

Bugzilla ID: 1374

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 .../single_active_interactive_shell.py        |   7 +-
 .../traffic_generator/__init__.py             |   2 +-
 .../testbed_model/traffic_generator/scapy.py  | 428 ++++++------------
 .../traffic_generator/traffic_generator.py    |  15 +-
 dts/framework/utils.py                        |  15 +
 5 files changed, 175 insertions(+), 292 deletions(-)

diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py
index 77a4dcefdf..4f1d7fc27f 100644
--- a/dts/framework/remote_session/single_active_interactive_shell.py
+++ b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -36,9 +36,10 @@
 from framework.params import Params
 from framework.settings import SETTINGS
 from framework.testbed_model.node import Node
+from framework.utils import MultiInheritanceBaseClass
 
 
-class SingleActiveInteractiveShell(ABC):
+class SingleActiveInteractiveShell(MultiInheritanceBaseClass, ABC):
     """The base class for managing interactive shells.
 
     This class shouldn't be instantiated directly, but instead be extended. It contains
@@ -93,9 +94,13 @@ def __init__(
         timeout: float = SETTINGS.timeout,
         app_params: Params = Params(),
         name: str | None = None,
+        **kwargs,
     ) -> None:
         """Create an SSH channel during initialization.
 
+        Additional key-word arguments can be passed through `kwargs` is needed to fulfill other
+        constructors in the case of multiple-inheritance.
+
         Args:
             node: The node on which to run start the interactive shell.
             privileged: Enables the shell to run as superuser.
diff --git a/dts/framework/testbed_model/traffic_generator/__init__.py b/dts/framework/testbed_model/traffic_generator/__init__.py
index 6dac86a224..a319fa5320 100644
--- a/dts/framework/testbed_model/traffic_generator/__init__.py
+++ b/dts/framework/testbed_model/traffic_generator/__init__.py
@@ -36,7 +36,7 @@ def create_traffic_generator(
     """
     match traffic_generator_config:
         case ScapyTrafficGeneratorConfig():
-            return ScapyTrafficGenerator(tg_node, traffic_generator_config)
+            return ScapyTrafficGenerator(tg_node, traffic_generator_config, privileged=True)
         case _:
             raise ConfigurationError(
                 f"Unknown traffic generator: {traffic_generator_config.traffic_generator_type}"
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index 13fc1107aa..86fa54b627 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -6,311 +6,180 @@
 
 A traffic generator used for functional testing, implemented with
 `the Scapy library <https://scapy.readthedocs.io/en/latest/>`_.
-The traffic generator uses an XML-RPC server to run Scapy on the remote TG node.
+The traffic generator uses an interactive shell to run Scapy on the remote TG node.
 
-The traffic generator uses the :mod:`xmlrpc.server` module to run an XML-RPC server
-in an interactive remote Python SSH session. The communication with the server is facilitated
-with a local server proxy from the :mod:`xmlrpc.client` module.
+The traffic generator extends :class:`framework.remote_session.python_shell.PythonShell` to
+implement the methods for handling packets by sending commands into the interactive shell.
 """
 
-import inspect
-import marshal
+
+import re
 import time
-import types
-import xmlrpc.client
-from xmlrpc.server import SimpleXMLRPCServer
+from typing import ClassVar
 
-import scapy.all  # type: ignore[import-untyped]
+from scapy.compat import base64_bytes  # type: ignore[import-untyped]
 from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
 from scapy.packet import Packet  # type: ignore[import-untyped]
 
 from framework.config import OS, ScapyTrafficGeneratorConfig
 from framework.remote_session.python_shell import PythonShell
-from framework.settings import SETTINGS
 from framework.testbed_model.node import Node
 from framework.testbed_model.port import Port
-
-from .capturing_traffic_generator import (
-    CapturingTrafficGenerator,
+from framework.testbed_model.traffic_generator.capturing_traffic_generator import (
     PacketFilteringConfig,
-    _get_default_capture_name,
 )
+from framework.utils import REGEX_FOR_BASE64_ENCODING
 
-"""
-========= BEGIN RPC FUNCTIONS =========
-
-All of the functions in this section are intended to be exported to a python
-shell which runs a scapy RPC server. These functions are made available via that
-RPC server to the packet generator. To add a new function to the RPC server,
-first write the function in this section. Then, if you need any imports, make sure to
-add them to SCAPY_RPC_SERVER_IMPORTS as well. After that, add the function to the list
-in EXPORTED_FUNCTIONS. Note that kwargs (keyword arguments) do not work via xmlrpc,
-so you may need to construct wrapper functions around many scapy types.
-"""
-
-"""
-Add the line needed to import something in a normal python environment
-as an entry to this array. It will be imported before any functions are
-sent to the server.
-"""
-SCAPY_RPC_SERVER_IMPORTS = [
-    "from scapy.all import *",
-    "import xmlrpc",
-    "import sys",
-    "from xmlrpc.server import SimpleXMLRPCServer",
-    "import marshal",
-    "import pickle",
-    "import types",
-    "import time",
-]
-
-
-def scapy_send_packets_and_capture(
-    xmlrpc_packets: list[xmlrpc.client.Binary],
-    send_iface: str,
-    recv_iface: str,
-    duration: float,
-    sniff_filter: str,
-) -> list[bytes]:
-    """The RPC function to send and capture packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-        recv_iface: The logical name of the ingress interface.
-        duration: Capture for this amount of time, in seconds.
-
-    Returns:
-        A list of bytes. Each item in the list represents one packet, which needs
-        to be converted back upon transfer from the remote node.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    sniffer = scapy.all.AsyncSniffer(
-        iface=recv_iface,
-        store=True,
-        started_callback=lambda *args: scapy.all.sendp(scapy_packets, iface=send_iface),
-        filter=sniff_filter,
-    )
-    sniffer.start()
-    time.sleep(duration)
-    return [scapy_packet.build() for scapy_packet in sniffer.stop(join=True)]
-
-
-def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: str) -> None:
-    """The RPC function to send packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-    It only sends `xmlrpc_packets`, without capturing them.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    scapy.all.sendp(scapy_packets, iface=send_iface, realtime=True, verbose=True)
-
-
-"""
-Functions to be exposed by the scapy RPC server.
-"""
-RPC_FUNCTIONS = [
-    scapy_send_packets,
-    scapy_send_packets_and_capture,
-]
-
-"""
-========= END RPC FUNCTIONS =========
-"""
-
-
-class QuittableXMLRPCServer(SimpleXMLRPCServer):
-    r"""Basic XML-RPC server.
+from .capturing_traffic_generator import CapturingTrafficGenerator
 
-    The server may be augmented by functions serializable by the :mod:`marshal` module.
 
-    Example:
-        ::
+class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
+    """Provides access to scapy functions on a traffic generator node.
 
-            def hello_world():
-                # to be sent to the XML-RPC server
-                print("Hello World!")
+    This class extends the base with remote execution of scapy functions. All methods for
+    processing packets are implemented using an underlying
+    :class:`framework.remote_session.python_shell.PythonShell` which imports the Scapy library.
 
-            # start the XML-RPC server on the remote node
-            # this is done by starting a Python shell on the remote node
-            from framework.remote_session import PythonShell
-            # the example assumes you're already connected to a tg_node
-            session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
+    Note that the order of inheritance is important for this class. In order to instantiate this
+    class, the abstract methods of :class:`~.capturing_traffic_generator.CapturingTrafficGenerator`
+    must be implemented. Since some of these methods are implemented in the underlying interactive
+    shell, according to Python's Method Resolution Order (MRO), the interactive shell must come
+    first.
+    """
 
-            # then importing the modules needed to run the server
-            # and the modules for any functions later added to the server
-            session.send_command("import xmlrpc")
-            session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
+    _config: ScapyTrafficGeneratorConfig
 
-            # sending the source code of this class to the Python shell
-            from xmlrpc.server import SimpleXMLRPCServer
-            src = inspect.getsource(QuittableXMLRPCServer)
-            src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
-            spacing = "\n" * 4
-            session.send_command(spacing + src + spacing)
+    #: Name of sniffer to ensure the same is used in all places
+    _sniffer_name: ClassVar[str] = "sniffer"
+    #: Name of variable that points to the list of packets inside the scapy shell.
+    _send_packet_list_name: ClassVar[str] = "packets"
+    #: Padding to add to the start of a line for python syntax compliance.
+    _python_indentation: ClassVar[str] = " " * 4
 
-            # then starting the server with:
-            command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
-            session.send_command(command, "XMLRPC OK")
+    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig, **kwargs):
+        """Extend the constructor with Scapy TG specifics.
 
-            # now the server is running on the remote node and we can add functions to it
-            # first connect to the server from the execution node
-            import xmlrpc.client
-            server_url = f"http://{tg_node.config.hostname}:8000"
-            rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
+        Initializes both the traffic generator and the interactive shell used to handle Scapy
+        functions. The interactive shell will be started on `tg_node`. The additional key-word
+        arguments in `kwargs` are used to pass into the constructor for the interactive shell.
 
-            # get the function bytes to send
-            import marshal
-            function_bytes = marshal.dumps(hello_world.__code__)
-            rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
+        Args:
+            tg_node: The node where the traffic generator resides.
+            config: The traffic generator's test run configuration.
+        """
+        assert (
+            tg_node.config.os == OS.linux
+        ), "Linux is the only supported OS for scapy traffic generation"
 
-            # now we can execute the function on the server
-            xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
-            print(str(xmlrpc_binary_recv))
-    """
+        super().__init__(tg_node, config=config, **kwargs)
+        self.start_application()
 
-    def __init__(self, *args, **kwargs):
-        """Extend the XML-RPC server initialization.
+    def start_application(self) -> None:
+        """Extends :meth:`framework.remote_session.interactive_shell.start_application`.
 
-        Args:
-            args: The positional arguments that will be passed to the superclass's constructor.
-            kwargs: The keyword arguments that will be passed to the superclass's constructor.
-                The `allow_none` argument will be set to :data:`True`.
+        Adds a command that imports everything from the scapy library immediately after starting
+        the shell for usage in later calls to the methods of this class.
         """
-        kwargs["allow_none"] = True
-        super().__init__(*args, **kwargs)
-        self.register_introspection_functions()
-        self.register_function(self.quit)
-        self.register_function(self.add_rpc_function)
+        super().start_application()
+        self.send_command("from scapy.all import *")
 
-    def quit(self) -> None:
-        """Quit the server."""
-        self._BaseServer__shutdown_request = True
-        return None
+    def _create_packet_list(self, packets: list[Packet]) -> None:
+        """Build a list of packets to send later.
 
-    def add_rpc_function(self, name: str, function_bytes: xmlrpc.client.Binary) -> None:
-        """Add a function to the server from the local server proxy.
+        Sends the string that represents the Python command that was used to create each packet in
+        `packets` into the underlying Python session. The purpose behind doing this is to create a
+        list that is identical to `packets` inside the shell. This method should only be called by
+        methods for sending packets immediately prior to sending. The list of packets will continue
+        to exist in the scope of the shell until subsequent calls to this method, so failure to
+        rebuild the list prior to sending packets could lead to undesired "stale" packets to be
+        sent.
 
         Args:
-              name: The name of the function.
-              function_bytes: The code of the function.
+            packets: The list of packets to recreate in the shell.
         """
-        function_code = marshal.loads(function_bytes.data)
-        function = types.FunctionType(function_code, globals(), name)
-        self.register_function(function)
+        self._logger.info("Building a list of packets to send.")
+        self.send_command(
+            f"{self._send_packet_list_name} = [{', '.join(map(Packet.command, packets))}]"
+        )
 
-    def serve_forever(self, poll_interval: float = 0.5) -> None:
-        """Extend the superclass method with an additional print.
+    def _send_packets(self, packets: list[Packet], port: Port) -> None:
+        """Implementation for sending packets without capturing any received traffic.
 
-        Once executed in the local server proxy, the print gives us a clear string to expect
-        when starting the server. The print means this function was executed on the XML-RPC server.
+        Provides a "fire and forget" method of sending packets.
         """
-        print("XMLRPC OK")
-        super().serve_forever(poll_interval)
-
-
-class ScapyTrafficGenerator(CapturingTrafficGenerator):
-    """Provides access to scapy functions via an RPC interface.
-
-    This class extends the base with remote execution of scapy functions.
-
-    Any packets sent to the remote server are first converted to bytes. They are received as
-    :class:`~xmlrpc.client.Binary` objects on the server side. When the server sends the packets
-    back, they are also received as :class:`~xmlrpc.client.Binary` objects on the client side, are
-    converted back to :class:`~scapy.packet.Packet` objects and only then returned from the methods.
-
-    Attributes:
-        session: The exclusive interactive remote session created by the Scapy
-            traffic generator where the XML-RPC server runs.
-        rpc_server_proxy: The object used by clients to execute functions
-            on the XML-RPC server.
-    """
-
-    session: PythonShell
-    rpc_server_proxy: xmlrpc.client.ServerProxy
-    _config: ScapyTrafficGeneratorConfig
-
-    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
-        """Extend the constructor with Scapy TG specifics.
-
-        The traffic generator first starts an XML-RPC on the remote `tg_node`.
-        Then it populates the server with functions which use the Scapy library
-        to send/receive traffic:
-
-            * :func:`scapy_send_packets_and_capture`
-            * :func:`scapy_send_packets`
-
-        To enable verbose logging from the xmlrpc client, use the :option:`--verbose`
-        command line argument or the :envvar:`DTS_VERBOSE` environment variable.
+        self._create_packet_list(packets)
+        send_command = [
+            "sendp(",
+            f"{self._send_packet_list_name},",
+            f"iface='{port.logical_name}',",
+            "realtime=True,",
+            "verbose=True",
+            ")",
+        ]
+        self.send_command(f"\n{self._python_indentation}".join(send_command))
+
+    def _create_sniffer(
+        self, packets_to_send: list[Packet], send_port: Port, recv_port: Port, filter_config: str
+    ) -> None:
+        """Create an asynchronous sniffer in the shell.
+
+        A list of packets to send is added to the sniffer inside of a callback function so that
+        they are immediately sent at the time sniffing is started.
 
         Args:
-            tg_node: The node where the traffic generator resides.
-            config: The traffic generator's test run configuration.
+            packets_to_send: A list of packets to send when sniffing is started.
+            send_port: The port to send the packets on when sniffing is started.
+            recv_port: The port to collect the traffic from.
+            filter_config: An optional BPF format filter to use when sniffing for packets. Omitted
+                when set to an empty string.
         """
-        super().__init__(tg_node, config)
-
-        assert (
-            self._tg_node.config.os == OS.linux
-        ), "Linux is the only supported OS for scapy traffic generation"
-
-        self.session = PythonShell(
-            self._tg_node, timeout=5, privileged=True, name="ScapyXMLRPCServer"
-        )
-
-        self.session.start_application()
+        self._create_packet_list(packets_to_send)
+        sniffer_commands = [
+            f"{self._sniffer_name} = AsyncSniffer(",
+            f"iface='{recv_port.logical_name}',",
+            "store=True,",
+            "started_callback=lambda *args: sendp(",
+            (
+                f"{self._python_indentation}{self._send_packet_list_name},"
+                f" iface='{send_port.logical_name}'),"
+            ),
+            ")",
+        ]
+        if filter_config:
+            sniffer_commands.insert(-1, f"filter='{filter_config}'")
+
+        self.send_command(f"\n{self._python_indentation}".join(sniffer_commands))
+
+    def _start_and_stop_sniffing(self, duration: float) -> list[Packet]:
+        """Start asynchronous sniffer, run for a set `duration`, then collect received packets.
+
+        This method expects that you have first created an asynchronous sniffer inside the shell
+        and will fail if you haven't. Received packets are collected by printing the base64
+        encoding of each packet in the shell and then harvesting these encodings using regex to
+        convert back into packet objects.
 
-        # import libs in remote python console
-        for import_statement in SCAPY_RPC_SERVER_IMPORTS:
-            self.session.send_command(import_statement)
-
-        # start the server
-        xmlrpc_server_listen_port = 8000
-        self._start_xmlrpc_server_in_remote_python(xmlrpc_server_listen_port)
+        Args:
+            duration: The amount of time in seconds to sniff for received packets.
 
-        # connect to the server
-        server_url = f"http://{self._tg_node.config.hostname}:{xmlrpc_server_listen_port}"
-        self.rpc_server_proxy = xmlrpc.client.ServerProxy(
-            server_url, allow_none=True, verbose=SETTINGS.verbose
+        Returns:
+            A list of all packets that were received while the sniffer was running.
+        """
+        sniffed_packets_name = "gathered_packets"
+        self.send_command(f"{self._sniffer_name}.start()")
+        time.sleep(duration)
+        self.send_command(f"{sniffed_packets_name} = {self._sniffer_name}.stop(join=True)")
+        # An extra newline is required here due to the nature of interactive Python shells
+        packet_strs = self.send_command(
+            f"for pakt in {sniffed_packets_name}: print(bytes_base64(pakt.build()))\n"
         )
-
-        # add functions to the server
-        for function in RPC_FUNCTIONS:
-            # A slightly hacky way to move a function to the remote server.
-            # It is constructed from the name and code on the other side.
-            # Pickle cannot handle functions, nor can any of the other serialization
-            # frameworks aside from the libraries used to generate pyc files, which
-            # are even more messy to work with.
-            function_bytes = marshal.dumps(function.__code__)
-            self.rpc_server_proxy.add_rpc_function(function.__name__, function_bytes)
-
-    def _start_xmlrpc_server_in_remote_python(self, listen_port: int) -> None:
-        # load the source of the function
-        src = inspect.getsource(QuittableXMLRPCServer)
-        # Lines with only whitespace break the repl if in the middle of a function
-        # or class, so strip all lines containing only whitespace
-        src = "\n".join([line for line in src.splitlines() if not line.isspace() and line != ""])
-
-        # execute it in the python terminal
-        self.session.send_command(src + "\n")
-        self.session.send_command(
-            f"server = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));server.serve_forever()",
-            "XMLRPC OK",
+        # In the string of bytes "b'XXXX'", we only want the contents ("XXXX")
+        list_of_packets_base64 = re.findall(
+            f"^b'({REGEX_FOR_BASE64_ENCODING})'", packet_strs, re.MULTILINE
         )
-
-    def _send_packets(self, packets: list[Packet], port: Port) -> None:
-        packets = [packet.build() for packet in packets]
-        self.rpc_server_proxy.scapy_send_packets(packets, port.logical_name)
+        return [Ether(base64_bytes(pakt)) for pakt in list_of_packets_base64]
 
     def _create_packet_filter(self, filter_config: PacketFilteringConfig) -> str:
-        """Combines filter settings from `filter_config` into a BPF that scapy can use.
+        """Combine filter settings from `filter_config` into a BPF that scapy can use.
 
         Scapy allows for the use of Berkeley Packet Filters (BPFs) to filter what packets are
         collected based on various attributes of the packet.
@@ -335,32 +204,19 @@ def _send_packets_and_capture(
         self,
         packets: list[Packet],
         send_port: Port,
-        receive_port: Port,
+        recv_port: Port,
         filter_config: PacketFilteringConfig,
         duration: float,
-        capture_name: str = _get_default_capture_name(),
     ) -> list[Packet]:
-        binary_packets = [packet.build() for packet in packets]
-
-        xmlrpc_packets: list[
-            xmlrpc.client.Binary
-        ] = self.rpc_server_proxy.scapy_send_packets_and_capture(
-            binary_packets,
-            send_port.logical_name,
-            receive_port.logical_name,
-            duration,
-            self._create_packet_filter(filter_config),
-        )  # type: ignore[assignment]
-
-        scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets]
-        return scapy_packets
-
-    def close(self) -> None:
-        """Close the traffic generator."""
-        try:
-            self.rpc_server_proxy.quit()
-        except ConnectionRefusedError:
-            # Because the python instance closes, we get no RPC response.
-            # Thus, this error is expected
-            pass
-        self.session.close()
+        """Implementation for sending packets and capturing any received traffic.
+
+        This method first creates an asynchronous sniffer that holds the packets to send, then
+        starts and stops and starts said sniffer.
+
+        Returns:
+            A list of packets received after sending `packets`.
+        """
+        self._create_sniffer(
+            packets, send_port, recv_port, self._create_packet_filter(filter_config)
+        )
+        return self._start_and_stop_sniffing(duration)
diff --git a/dts/framework/testbed_model/traffic_generator/traffic_generator.py b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
index 4ce1148706..176d5e9065 100644
--- a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
+++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
@@ -16,23 +16,29 @@
 from framework.logger import DTSLogger, get_dts_logger
 from framework.testbed_model.node import Node
 from framework.testbed_model.port import Port
-from framework.utils import get_packet_summaries
+from framework.utils import MultiInheritanceBaseClass, get_packet_summaries
 
 
-class TrafficGenerator(ABC):
+class TrafficGenerator(MultiInheritanceBaseClass, ABC):
     """The base traffic generator.
 
     Exposes the common public methods of all traffic generators and defines private methods
-    that must implement the traffic generation logic in subclasses.
+    that must implement the traffic generation logic in subclasses. This class also extends from
+    :class:`framework.utils.MultiInheritanceBaseClass` to allow subclasses the ability to inherit
+    from multiple classes to fulfil the traffic generating functionality without breaking
+    single-inheritance.
     """
 
     _config: TrafficGeneratorConfig
     _tg_node: Node
     _logger: DTSLogger
 
-    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
+    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig, **kwargs):
         """Initialize the traffic generator.
 
+        Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other
+        constructors in the case of multiple-inheritance.
+
         Args:
             tg_node: The traffic generator node where the created traffic generator will be running.
             config: The traffic generator's test run configuration.
@@ -40,6 +46,7 @@ def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
         self._config = config
         self._tg_node = tg_node
         self._logger = get_dts_logger(f"{self._tg_node.name} {self._config.traffic_generator_type}")
+        super().__init__(tg_node, **kwargs)
 
     def send_packet(self, packet: Packet, port: Port) -> None:
         """Send `packet` and block until it is fully sent.
diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index c768dd0c99..c68fc44517 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -29,6 +29,7 @@
 from .exception import ConfigurationError, InternalError
 
 REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
+REGEX_FOR_BASE64_ENCODING: str = "[-a-zA-Z0-9+\\/]*={0,3}"
 
 
 def expand_range(range_str: str) -> list[int]:
@@ -319,3 +320,17 @@ def _make_packet() -> Packet:
         return packet / random.randbytes(usable_payload_size)
 
     return [_make_packet() for _ in range(number_of)]
+
+
+class MultiInheritanceBaseClass:
+    """A base class for classes utilizing multiple-inheritance.
+
+    This class enables it's subclasses to support both single and multiple inheritance by acting as
+    a stopping point in the tree of calls to the constructors of super-classes. This class is able
+    to exist at the end of the Method Resolution Order (MRO) so that sub-classes can call super
+    without repercussion.
+    """
+
+    def __init__(self, *args, **kwargs) -> None:
+        """Call the init method of :class:`object`."""
+        super().__init__()
-- 
2.46.0


-- 



*Let's Connect!*

...  *October Webinars*

Ask Us Anything: IOL Services 
Open Q&A 
<https://unh.zoom.us/webinar/register/9017265932716/WN_OUo5S7iQRLmKKY7CsmwZhw#/registration>Your 
questions. Our answers. Let's get started.


Oct 3rd


Live Tour of INTACT(R) 
for IPv6 Testing and Validation 
<https://unh.zoom.us/webinar/register/7117231236474/WN_I2zfyi_2S2yEiXkxBRi8sA#/registration>
Open tour. Open Q&A. See why we think you'll love INTACT.

Oct 9th


How to 
Prep for Our NVMe(R) Plugfest #21 
<https://unh.zoom.us/webinar/register/4017266809553/WN_X1iA2SZ8QhmcGboF2DImNg#/registration>
Checklists. Conversation. Let's get ready to plugin! 
Oct 15th


... * 
Newsletter*

*
*
Get the IOL Connector 
<https://www.iol.unh.edu/news/email-newsletters> for our latest news and 
event info.



.

Re: [PATCH v3 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[Juraj Linkeš]

I like how this looks. I have a number of minor comments (mainly wording 
and naming), but overall it looks very good.

On 19. 9. 2024 21:02, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> Previously all scapy commands were handled using an XML-RPC server that
> ran on the TGNode. This unnecessarily enforces a minimum Python version
> of 3.10 on the server that is being used as a traffic generator and
> complicates the implementation of scapy methods.

What is the TG's minimum Python version now? 
https://bugs.dpdk.org/show_bug.cgi?id=1388 says this will become a moot 
point, but we're still using Python on the remote node.


> diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py

> @@ -93,9 +94,13 @@ def __init__(
>           timeout: float = SETTINGS.timeout,
>           app_params: Params = Params(),
>           name: str | None = None,
> +        **kwargs,
>       ) -> None:
>           """Create an SSH channel during initialization.
>   
> +        Additional key-word arguments can be passed through `kwargs` is needed to fulfill other

Typo: is -> if

And key-word should be just keyword.

Also, should we add super().__init__(), seeing as we also added it to 
TrafficGenerator?


> diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py

> +class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):

> +    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig, **kwargs):

The usage of privileged=True when creating the instance confused me for 
a bit, because it's not this class's argument, but rather 
SingleActiveInteractiveShell's. I thing we should document the arguments 
of SingleActiveInteractiveShell in the Keyword Args section. We probably 
need just a link to SingleActiveInteractiveShell.


> +    def _create_packet_list(self, packets: list[Packet]) -> None:

Maybe we could apply some of the ideas from the local/remote naming 
scheme I talked about in the tg devbind script patch here. Whatever 
happens on the TG could be prefixed with remote and whatever is 
happening locally would be without the prefix (or maybe whatever is 
happnening on the TG shouldn't be prefixed (or a different prefix - 
shell)? Makes sense, but then we'd need a good prefix for what's 
happening on the execution environment. Maybe this also needs to be in a 
different patch, after it's been though through with everything else in 
mind.). That would make this _create_remote_packet_list, but we're just 
setting a variable (the passed packets have already been built), so 
maybe _set_remote_packet_list?

Or maybe all of the remote methods could start with remote, making it 
_remote_set_packet_list (or _shell_set_packet_list? Doesn't sound bad.). 
Not sure which is better, maybe after renaming more of these it's going 
to be clearer.

Whatever we go with, the naming scheme should be explained in the 
class's dosctring.


> +    def _create_sniffer(
> +        self, packets_to_send: list[Packet], send_port: Port, recv_port: Port, filter_config: str
> +    ) -> None:
> +        """Create an asynchronous sniffer in the shell.
> +
> +        A list of packets to send is added to the sniffer inside of a callback function so that
> +        they are immediately sent at the time sniffing is started.

This is still a bit confusing (where are the packets added and what is 
inside the function?); We may not need the Scapy implementation details. 
We could just say the packets are sent immediately at the time sniffing 
is started. Or maybe:
A list of packets is passed to the sniffer's callback function so that 
they are immediately sent at the time sniffing is started.

It's a private method, so maybe the implementation detail could be 
valuable, even though it's not fully clear why the implication is true - 
you still need some knowledge of how the sniffer works to put everything 
together.


> +        sniffer_commands = [
> +            f"{self._sniffer_name} = AsyncSniffer(",
> +            f"iface='{recv_port.logical_name}',",
> +            "store=True,",
> +            "started_callback=lambda *args: sendp(",

As far as I can tell, we're not using the args, so we can just use 
"lambda: sendp()"

> +            (
> +                f"{self._python_indentation}{self._send_packet_list_name},"

Is the indentation needed here?


> @@ -335,32 +204,19 @@ def _send_packets_and_capture(

I think we can improve the order of methods in the class. I'd put 
_send_packets() and _send_packets_and_capture() after the public 
methods. These two methods are the most important ones (implementing the 
abstract methods). The other methods should come after that in the order 
they're used in _send_packets() and _send_packets_and_capture().

>           self,
>           packets: list[Packet],
>           send_port: Port,
> -        receive_port: Port,
> +        recv_port: Port,
>           filter_config: PacketFilteringConfig,
>           duration: float,
> -        capture_name: str = _get_default_capture_name(),
>       ) -> list[Packet]:

> +        """Implementation for sending packets and capturing any received traffic.
> +
> +        This method first creates an asynchronous sniffer that holds the packets to send, then
> +        starts and stops and starts said sniffer.

This looks like a typo: starts and stops and starts.

Maybe we could add that we collect the received packets from the sniffer 
once we've stopped it.


> diff --git a/dts/framework/testbed_model/traffic_generator/traffic_generator.py b/dts/framework/testbed_model/traffic_generator/traffic_generator.py

> @@ -16,23 +16,29 @@

> +class TrafficGenerator(MultiInheritanceBaseClass, ABC):

> +    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig, **kwargs):
>           """Initialize the traffic generator.
>   
> +        Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other
> +        constructors in the case of multiple-inheritance.
> +

The wording in this sentence is slightly different that the one in 
SingleActiveInteractiveShell. Let's unify them.

Re: [PATCH v3 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[Jeremy Spewock]

On Tue, Sep 24, 2024 at 6:55 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
> I like how this looks. I have a number of minor comments (mainly wording
> and naming), but overall it looks very good.
>
> On 19. 9. 2024 21:02, jspewock@iol.unh.edu wrote:
> > From: Jeremy Spewock <jspewock@iol.unh.edu>
> >
> > Previously all scapy commands were handled using an XML-RPC server that
> > ran on the TGNode. This unnecessarily enforces a minimum Python version
> > of 3.10 on the server that is being used as a traffic generator and
> > complicates the implementation of scapy methods.
>
> What is the TG's minimum Python version now?
> https://bugs.dpdk.org/show_bug.cgi?id=1388 says this will become a moot
> point, but we're still using Python on the remote node.

Right, there is still some dependency there. I'm not sure the exact
versions, but doing some looking around I believe one of the newest
things scapy tools we use in the framework is the AsyncSniffer and the
scapy documentation [1] says that it has been available since version
2.4.3, and then when I looked at that version of the scapy package [2]
it looks like it claims to support python 2.7 and python 3.4-7. Poking
around in the documentation/code from scapy version 2.4.3 it also
looks like the syntax is very similar, so I believe it would work, but
I'm not sure I have any hosts that I could run on which have python
3.4. Maybe that does make the dependency essentially a moot point
considering these are fairly old python versions.

[1] https://github.com/secdev/scapy/blob/master/doc/scapy/usage.rst
[2] https://pypi.org/project/scapy/2.4.3/

>
>
> > diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py
>
> > @@ -93,9 +94,13 @@ def __init__(
> >           timeout: float = SETTINGS.timeout,
> >           app_params: Params = Params(),
> >           name: str | None = None,
> > +        **kwargs,
> >       ) -> None:
> >           """Create an SSH channel during initialization.
> >
> > +        Additional key-word arguments can be passed through `kwargs` is needed to fulfill other
>
> Typo: is -> if
>
> And key-word should be just keyword.
>

Ack.

> Also, should we add super().__init__(), seeing as we also added it to
> TrafficGenerator?
>

We probably should so that it doesn't matter which order you specify
the two in and they still work, this is probably something that I
missed in the rebase.

>
> > diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
>
> > +class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
>
> > +    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig, **kwargs):
>
> The usage of privileged=True when creating the instance confused me for
> a bit, because it's not this class's argument, but rather
> SingleActiveInteractiveShell's. I thing we should document the arguments
> of SingleActiveInteractiveShell in the Keyword Args section. We probably
> need just a link to SingleActiveInteractiveShell.

Sure, I can add some kind of reference. I agree that it's confusing, I
wish there was a simple way we could unpack the parameters of the
interactive shell into kwargs, but when I was searching I couldn't
really find anything great. Other than doing something like making
TypedDict classes for interactive shell parameters, but I'm not sure
that's super sleek either.

>
>
> > +    def _create_packet_list(self, packets: list[Packet]) -> None:
>
> Maybe we could apply some of the ideas from the local/remote naming
> scheme I talked about in the tg devbind script patch here. Whatever
> happens on the TG could be prefixed with remote and whatever is
> happening locally would be without the prefix (or maybe whatever is
> happnening on the TG shouldn't be prefixed (or a different prefix -
> shell)? Makes sense, but then we'd need a good prefix for what's
> happening on the execution environment. Maybe this also needs to be in a
> different patch, after it's been though through with everything else in

I'll still write something here that makes the distinction and that
other patch could reformat if the author thought something else was
better.

> mind.). That would make this _create_remote_packet_list, but we're just
> setting a variable (the passed packets have already been built), so
> maybe _set_remote_packet_list?
>
> Or maybe all of the remote methods could start with remote, making it
> _remote_set_packet_list (or _shell_set_packet_list? Doesn't sound bad.).
> Not sure which is better, maybe after renaming more of these it's going
> to be clearer.

This is a good idea. I like remote more initially, but I'll try it out
on some of the methods and see if shell fits better. Regardless, I
think one of those two would be a good option as well.

>
> Whatever we go with, the naming scheme should be explained in the
> class's dosctring.

Ack.

>
>
> > +    def _create_sniffer(
> > +        self, packets_to_send: list[Packet], send_port: Port, recv_port: Port, filter_config: str
> > +    ) -> None:
> > +        """Create an asynchronous sniffer in the shell.
> > +
> > +        A list of packets to send is added to the sniffer inside of a callback function so that
> > +        they are immediately sent at the time sniffing is started.
>
> This is still a bit confusing (where are the packets added and what is
> inside the function?); We may not need the Scapy implementation details.
> We could just say the packets are sent immediately at the time sniffing
> is started. Or maybe:
> A list of packets is passed to the sniffer's callback function so that
> they are immediately sent at the time sniffing is started.

Sure, I think this wording makes sense. That's a good point that this
doc-string really doesn't need that much information about the
implementation and it could be confusing.

>
> It's a private method, so maybe the implementation detail could be
> valuable, even though it's not fully clear why the implication is true -
> you still need some knowledge of how the sniffer works to put everything
> together.

I think it helps especially because the send method just creates a
sniffer, then it starts and stops sniffing so I wanted to add more
context as to how that actually leads to packets being sent. I think
your wording above though gives enough information to understand that
in a more elegant way.

>
>
> > +        sniffer_commands = [
> > +            f"{self._sniffer_name} = AsyncSniffer(",
> > +            f"iface='{recv_port.logical_name}',",
> > +            "store=True,",
> > +            "started_callback=lambda *args: sendp(",
>
> As far as I can tell, we're not using the args, so we can just use
> "lambda: sendp()"

We don't use the argument, but there are positional arguments passed
into this function by scapy which is why we have to catch and ignore
them.

>
> > +            (
> > +                f"{self._python_indentation}{self._send_packet_list_name},"
>
> Is the indentation needed here?

It's not required, but I think it makes the logs more readable.

>
>
> > @@ -335,32 +204,19 @@ def _send_packets_and_capture(
>
> I think we can improve the order of methods in the class. I'd put
> _send_packets() and _send_packets_and_capture() after the public
> methods. These two methods are the most important ones (implementing the
> abstract methods). The other methods should come after that in the order
> they're used in _send_packets() and _send_packets_and_capture().

Sure, I can do that.

>
> >           self,
> >           packets: list[Packet],
> >           send_port: Port,
> > -        receive_port: Port,
> > +        recv_port: Port,
> >           filter_config: PacketFilteringConfig,
> >           duration: float,
> > -        capture_name: str = _get_default_capture_name(),
> >       ) -> list[Packet]:
>
> > +        """Implementation for sending packets and capturing any received traffic.
> > +
> > +        This method first creates an asynchronous sniffer that holds the packets to send, then
> > +        starts and stops and starts said sniffer.
>
> This looks like a typo: starts and stops and starts.
>

Yup, good catch.

> Maybe we could add that we collect the received packets from the sniffer
> once we've stopped it.
>

Sure, that could be useful information.

>
> > diff --git a/dts/framework/testbed_model/traffic_generator/traffic_generator.py b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
>
> > @@ -16,23 +16,29 @@
>
> > +class TrafficGenerator(MultiInheritanceBaseClass, ABC):
>
> > +    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig, **kwargs):
> >           """Initialize the traffic generator.
> >
> > +        Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other
> > +        constructors in the case of multiple-inheritance.
> > +
>
> The wording in this sentence is slightly different that the one in
> SingleActiveInteractiveShell. Let's unify them.

Ack.

>

Re: [PATCH v3 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[Juraj Linkeš]

On 24. 9. 2024 18:34, Jeremy Spewock wrote:
> On Tue, Sep 24, 2024 at 6:55 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>>
>> I like how this looks. I have a number of minor comments (mainly wording
>> and naming), but overall it looks very good.
>>
>> On 19. 9. 2024 21:02, jspewock@iol.unh.edu wrote:
>>> From: Jeremy Spewock <jspewock@iol.unh.edu>
>>>
>>> Previously all scapy commands were handled using an XML-RPC server that
>>> ran on the TGNode. This unnecessarily enforces a minimum Python version
>>> of 3.10 on the server that is being used as a traffic generator and
>>> complicates the implementation of scapy methods.
>>
>> What is the TG's minimum Python version now?
>> https://bugs.dpdk.org/show_bug.cgi?id=1388 says this will become a moot
>> point, but we're still using Python on the remote node.
> 
> Right, there is still some dependency there. I'm not sure the exact
> versions, but doing some looking around I believe one of the newest
> things scapy tools we use in the framework is the AsyncSniffer and the
> scapy documentation [1] says that it has been available since version
> 2.4.3, and then when I looked at that version of the scapy package [2]
> it looks like it claims to support python 2.7 and python 3.4-7. Poking
> around in the documentation/code from scapy version 2.4.3 it also
> looks like the syntax is very similar, so I believe it would work, but
> I'm not sure I have any hosts that I could run on which have python
> 3.4. Maybe that does make the dependency essentially a moot point
> considering these are fairly old python versions.
> 
> [1] https://github.com/secdev/scapy/blob/master/doc/scapy/usage.rst
> [2] https://pypi.org/project/scapy/2.4.3/
> 

Great. We should still document that the TG needs Python. I've updated 
the ticket. [0]

[0] https://bugs.dpdk.org/show_bug.cgi?id=1388


>>> +    def _create_packet_list(self, packets: list[Packet]) -> None:
>>
>> Maybe we could apply some of the ideas from the local/remote naming
>> scheme I talked about in the tg devbind script patch here. Whatever
>> happens on the TG could be prefixed with remote and whatever is
>> happening locally would be without the prefix (or maybe whatever is
>> happnening on the TG shouldn't be prefixed (or a different prefix -
>> shell)? Makes sense, but then we'd need a good prefix for what's
>> happening on the execution environment. Maybe this also needs to be in a
>> different patch, after it's been though through with everything else in
> 
> I'll still write something here that makes the distinction and that
> other patch could reformat if the author thought something else was
> better.
> 

Great.


>>> +        sniffer_commands = [
>>> +            f"{self._sniffer_name} = AsyncSniffer(",
>>> +            f"iface='{recv_port.logical_name}',",
>>> +            "store=True,",
>>> +            "started_callback=lambda *args: sendp(",
>>
>> As far as I can tell, we're not using the args, so we can just use
>> "lambda: sendp()"
> 
> We don't use the argument, but there are positional arguments passed
> into this function by scapy which is why we have to catch and ignore
> them.
> 

Makes sense now, thanks for the explanation. Maybe we could put 
somewhere in here a little comment pointing this out?

>>
>>> +            (
>>> +                f"{self._python_indentation}{self._send_packet_list_name},"
>>
>> Is the indentation needed here?
> 
> It's not required, but I think it makes the logs more readable.
> 

That's a worthy use, let's keep it. Maybe also add an explanatory 
comment here?

[PATCH v4 0/1] dts: replace XML-RPC server

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

v4:
 * update naming scheme of methods in the scapy traffic generator and
   reorganized them
 * update doc-strings to add consistency and fix typos
 * add supoer().__init__() call to single_active_interactive_shell so
   that it and the traffic generator can be given in any order
 * added comments for additional clarity where needed

Jeremy Spewock (1):
  dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

 .../single_active_interactive_shell.py        |   8 +-
 .../traffic_generator/__init__.py             |   2 +-
 .../testbed_model/traffic_generator/scapy.py  | 454 +++++++-----------
 .../traffic_generator/traffic_generator.py    |  15 +-
 dts/framework/utils.py                        |  15 +
 5 files changed, 195 insertions(+), 299 deletions(-)

-- 
2.46.0

[PATCH v4 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously all scapy commands were handled using an XML-RPC server that
ran on the TGNode. This unnecessarily enforces a minimum Python version
of 3.10 on the server that is being used as a traffic generator and
complicates the implementation of scapy methods. This patch removes the
XML-RPC server completely and instead allows the Scapy TG to extend from
the PythonShell to implement the functionality of a traffic generator.
This is done by importing the Scapy library in the PythonShell and
sending commands directly to the interactive session on the TG Node.

Bugzilla ID: 1374

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 .../single_active_interactive_shell.py        |   8 +-
 .../traffic_generator/__init__.py             |   2 +-
 .../testbed_model/traffic_generator/scapy.py  | 454 +++++++-----------
 .../traffic_generator/traffic_generator.py    |  15 +-
 dts/framework/utils.py                        |  15 +
 5 files changed, 195 insertions(+), 299 deletions(-)

diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py
index 77a4dcefdf..f41d729655 100644
--- a/dts/framework/remote_session/single_active_interactive_shell.py
+++ b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -36,9 +36,10 @@
 from framework.params import Params
 from framework.settings import SETTINGS
 from framework.testbed_model.node import Node
+from framework.utils import MultiInheritanceBaseClass
 
 
-class SingleActiveInteractiveShell(ABC):
+class SingleActiveInteractiveShell(MultiInheritanceBaseClass, ABC):
     """The base class for managing interactive shells.
 
     This class shouldn't be instantiated directly, but instead be extended. It contains
@@ -93,9 +94,13 @@ def __init__(
         timeout: float = SETTINGS.timeout,
         app_params: Params = Params(),
         name: str | None = None,
+        **kwargs,
     ) -> None:
         """Create an SSH channel during initialization.
 
+        Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other
+        constructors in the case of multiple-inheritance.
+
         Args:
             node: The node on which to run start the interactive shell.
             privileged: Enables the shell to run as superuser.
@@ -115,6 +120,7 @@ def __init__(
         self._timeout = timeout
         # Ensure path is properly formatted for the host
         self._update_real_path(self.path)
+        super().__init__(node, **kwargs)
 
     def _setup_ssh_channel(self):
         self._ssh_channel = self._node.main_session.interactive_session.session.invoke_shell()
diff --git a/dts/framework/testbed_model/traffic_generator/__init__.py b/dts/framework/testbed_model/traffic_generator/__init__.py
index 6dac86a224..a319fa5320 100644
--- a/dts/framework/testbed_model/traffic_generator/__init__.py
+++ b/dts/framework/testbed_model/traffic_generator/__init__.py
@@ -36,7 +36,7 @@ def create_traffic_generator(
     """
     match traffic_generator_config:
         case ScapyTrafficGeneratorConfig():
-            return ScapyTrafficGenerator(tg_node, traffic_generator_config)
+            return ScapyTrafficGenerator(tg_node, traffic_generator_config, privileged=True)
         case _:
             raise ConfigurationError(
                 f"Unknown traffic generator: {traffic_generator_config.traffic_generator_type}"
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index 13fc1107aa..07f11b7f78 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -6,311 +6,150 @@
 
 A traffic generator used for functional testing, implemented with
 `the Scapy library <https://scapy.readthedocs.io/en/latest/>`_.
-The traffic generator uses an XML-RPC server to run Scapy on the remote TG node.
+The traffic generator uses an interactive shell to run Scapy on the remote TG node.
 
-The traffic generator uses the :mod:`xmlrpc.server` module to run an XML-RPC server
-in an interactive remote Python SSH session. The communication with the server is facilitated
-with a local server proxy from the :mod:`xmlrpc.client` module.
+The traffic generator extends :class:`framework.remote_session.python_shell.PythonShell` to
+implement the methods for handling packets by sending commands into the interactive shell.
 """
 
-import inspect
-import marshal
+
+import re
 import time
-import types
-import xmlrpc.client
-from xmlrpc.server import SimpleXMLRPCServer
+from typing import ClassVar
 
-import scapy.all  # type: ignore[import-untyped]
+from scapy.compat import base64_bytes  # type: ignore[import-untyped]
 from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
 from scapy.packet import Packet  # type: ignore[import-untyped]
 
 from framework.config import OS, ScapyTrafficGeneratorConfig
 from framework.remote_session.python_shell import PythonShell
-from framework.settings import SETTINGS
 from framework.testbed_model.node import Node
 from framework.testbed_model.port import Port
-
-from .capturing_traffic_generator import (
-    CapturingTrafficGenerator,
+from framework.testbed_model.traffic_generator.capturing_traffic_generator import (
     PacketFilteringConfig,
-    _get_default_capture_name,
 )
+from framework.utils import REGEX_FOR_BASE64_ENCODING
 
-"""
-========= BEGIN RPC FUNCTIONS =========
-
-All of the functions in this section are intended to be exported to a python
-shell which runs a scapy RPC server. These functions are made available via that
-RPC server to the packet generator. To add a new function to the RPC server,
-first write the function in this section. Then, if you need any imports, make sure to
-add them to SCAPY_RPC_SERVER_IMPORTS as well. After that, add the function to the list
-in EXPORTED_FUNCTIONS. Note that kwargs (keyword arguments) do not work via xmlrpc,
-so you may need to construct wrapper functions around many scapy types.
-"""
+from .capturing_traffic_generator import CapturingTrafficGenerator
 
-"""
-Add the line needed to import something in a normal python environment
-as an entry to this array. It will be imported before any functions are
-sent to the server.
-"""
-SCAPY_RPC_SERVER_IMPORTS = [
-    "from scapy.all import *",
-    "import xmlrpc",
-    "import sys",
-    "from xmlrpc.server import SimpleXMLRPCServer",
-    "import marshal",
-    "import pickle",
-    "import types",
-    "import time",
-]
-
-
-def scapy_send_packets_and_capture(
-    xmlrpc_packets: list[xmlrpc.client.Binary],
-    send_iface: str,
-    recv_iface: str,
-    duration: float,
-    sniff_filter: str,
-) -> list[bytes]:
-    """The RPC function to send and capture packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-        recv_iface: The logical name of the ingress interface.
-        duration: Capture for this amount of time, in seconds.
-
-    Returns:
-        A list of bytes. Each item in the list represents one packet, which needs
-        to be converted back upon transfer from the remote node.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    sniffer = scapy.all.AsyncSniffer(
-        iface=recv_iface,
-        store=True,
-        started_callback=lambda *args: scapy.all.sendp(scapy_packets, iface=send_iface),
-        filter=sniff_filter,
-    )
-    sniffer.start()
-    time.sleep(duration)
-    return [scapy_packet.build() for scapy_packet in sniffer.stop(join=True)]
-
-
-def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: str) -> None:
-    """The RPC function to send packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-    It only sends `xmlrpc_packets`, without capturing them.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    scapy.all.sendp(scapy_packets, iface=send_iface, realtime=True, verbose=True)
 
+class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
+    """Provides access to scapy functions on a traffic generator node.
 
-"""
-Functions to be exposed by the scapy RPC server.
-"""
-RPC_FUNCTIONS = [
-    scapy_send_packets,
-    scapy_send_packets_and_capture,
-]
+    This class extends the base with remote execution of scapy functions. All methods for
+    processing packets are implemented using an underlying
+    :class:`framework.remote_session.python_shell.PythonShell` which imports the Scapy library.
 
-"""
-========= END RPC FUNCTIONS =========
-"""
-
-
-class QuittableXMLRPCServer(SimpleXMLRPCServer):
-    r"""Basic XML-RPC server.
-
-    The server may be augmented by functions serializable by the :mod:`marshal` module.
-
-    Example:
-        ::
+    Because of the dual-inheritance, this class has both methods that wrap scapy commands sent into
+    the shell and methods that run locally to fulfill traffic generation needs. To help make a
+    clear distinction between the two, the names of the methods that wrap the logic of the
+    underlying shell should be prepended with "shell".
 
-            def hello_world():
-                # to be sent to the XML-RPC server
-                print("Hello World!")
-
-            # start the XML-RPC server on the remote node
-            # this is done by starting a Python shell on the remote node
-            from framework.remote_session import PythonShell
-            # the example assumes you're already connected to a tg_node
-            session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
-
-            # then importing the modules needed to run the server
-            # and the modules for any functions later added to the server
-            session.send_command("import xmlrpc")
-            session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
-
-            # sending the source code of this class to the Python shell
-            from xmlrpc.server import SimpleXMLRPCServer
-            src = inspect.getsource(QuittableXMLRPCServer)
-            src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
-            spacing = "\n" * 4
-            session.send_command(spacing + src + spacing)
-
-            # then starting the server with:
-            command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
-            session.send_command(command, "XMLRPC OK")
+    Note that the order of inheritance is important for this class. In order to instantiate this
+    class, the abstract methods of :class:`~.capturing_traffic_generator.CapturingTrafficGenerator`
+    must be implemented. Since some of these methods are implemented in the underlying interactive
+    shell, according to Python's Method Resolution Order (MRO), the interactive shell must come
+    first.
+    """
 
-            # now the server is running on the remote node and we can add functions to it
-            # first connect to the server from the execution node
-            import xmlrpc.client
-            server_url = f"http://{tg_node.config.hostname}:8000"
-            rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
+    _config: ScapyTrafficGeneratorConfig
 
-            # get the function bytes to send
-            import marshal
-            function_bytes = marshal.dumps(hello_world.__code__)
-            rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
+    #: Name of sniffer to ensure the same is used in all places
+    _sniffer_name: ClassVar[str] = "sniffer"
+    #: Name of variable that points to the list of packets inside the scapy shell.
+    _send_packet_list_name: ClassVar[str] = "packets"
+    #: Padding to add to the start of a line for python syntax compliance.
+    _python_indentation: ClassVar[str] = " " * 4
 
-            # now we can execute the function on the server
-            xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
-            print(str(xmlrpc_binary_recv))
-    """
+    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig, **kwargs):
+        """Extend the constructor with Scapy TG specifics.
 
-    def __init__(self, *args, **kwargs):
-        """Extend the XML-RPC server initialization.
+        Initializes both the traffic generator and the interactive shell used to handle Scapy
+        functions. The interactive shell will be started on `tg_node`. The additional key-word
+        arguments in `kwargs` are used to pass into the constructor for the interactive shell.
 
         Args:
-            args: The positional arguments that will be passed to the superclass's constructor.
-            kwargs: The keyword arguments that will be passed to the superclass's constructor.
-                The `allow_none` argument will be set to :data:`True`.
+            tg_node: The node where the traffic generator resides.
+            config: The traffic generator's test run configuration.
+            kwargs: Keyword arguments all traffic generators must support in order to allow for
+                multiple-inheritance. Values passed into this parameter should correspond to the
+                parameters of :meth:`PythonShell.__init__` in this case.
         """
-        kwargs["allow_none"] = True
-        super().__init__(*args, **kwargs)
-        self.register_introspection_functions()
-        self.register_function(self.quit)
-        self.register_function(self.add_rpc_function)
+        assert (
+            tg_node.config.os == OS.linux
+        ), "Linux is the only supported OS for scapy traffic generation"
 
-    def quit(self) -> None:
-        """Quit the server."""
-        self._BaseServer__shutdown_request = True
-        return None
+        super().__init__(tg_node, config=config, **kwargs)
+        self.start_application()
 
-    def add_rpc_function(self, name: str, function_bytes: xmlrpc.client.Binary) -> None:
-        """Add a function to the server from the local server proxy.
+    def start_application(self) -> None:
+        """Extends :meth:`framework.remote_session.interactive_shell.start_application`.
 
-        Args:
-              name: The name of the function.
-              function_bytes: The code of the function.
+        Adds a command that imports everything from the scapy library immediately after starting
+        the shell for usage in later calls to the methods of this class.
         """
-        function_code = marshal.loads(function_bytes.data)
-        function = types.FunctionType(function_code, globals(), name)
-        self.register_function(function)
+        super().start_application()
+        self.send_command("from scapy.all import *")
 
-    def serve_forever(self, poll_interval: float = 0.5) -> None:
-        """Extend the superclass method with an additional print.
+    def _send_packets(self, packets: list[Packet], port: Port) -> None:
+        """Implementation for sending packets without capturing any received traffic.
 
-        Once executed in the local server proxy, the print gives us a clear string to expect
-        when starting the server. The print means this function was executed on the XML-RPC server.
+        Provides a "fire and forget" method of sending packets.
         """
-        print("XMLRPC OK")
-        super().serve_forever(poll_interval)
-
-
-class ScapyTrafficGenerator(CapturingTrafficGenerator):
-    """Provides access to scapy functions via an RPC interface.
-
-    This class extends the base with remote execution of scapy functions.
-
-    Any packets sent to the remote server are first converted to bytes. They are received as
-    :class:`~xmlrpc.client.Binary` objects on the server side. When the server sends the packets
-    back, they are also received as :class:`~xmlrpc.client.Binary` objects on the client side, are
-    converted back to :class:`~scapy.packet.Packet` objects and only then returned from the methods.
-
-    Attributes:
-        session: The exclusive interactive remote session created by the Scapy
-            traffic generator where the XML-RPC server runs.
-        rpc_server_proxy: The object used by clients to execute functions
-            on the XML-RPC server.
-    """
-
-    session: PythonShell
-    rpc_server_proxy: xmlrpc.client.ServerProxy
-    _config: ScapyTrafficGeneratorConfig
-
-    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
-        """Extend the constructor with Scapy TG specifics.
-
-        The traffic generator first starts an XML-RPC on the remote `tg_node`.
-        Then it populates the server with functions which use the Scapy library
-        to send/receive traffic:
+        self._shell_set_packet_list(packets)
+        send_command = [
+            "sendp(",
+            f"{self._send_packet_list_name},",
+            f"iface='{port.logical_name}',",
+            "realtime=True,",
+            "verbose=True",
+            ")",
+        ]
+        self.send_command(f"\n{self._python_indentation}".join(send_command))
 
-            * :func:`scapy_send_packets_and_capture`
-            * :func:`scapy_send_packets`
+    def _send_packets_and_capture(
+        self,
+        packets: list[Packet],
+        send_port: Port,
+        recv_port: Port,
+        filter_config: PacketFilteringConfig,
+        duration: float,
+    ) -> list[Packet]:
+        """Implementation for sending packets and capturing any received traffic.
 
-        To enable verbose logging from the xmlrpc client, use the :option:`--verbose`
-        command line argument or the :envvar:`DTS_VERBOSE` environment variable.
+        This method first creates an asynchronous sniffer that holds the packets to send, then
+        starts and stops said sniffer, collecting any packets that it had received while it was
+        running.
 
-        Args:
-            tg_node: The node where the traffic generator resides.
-            config: The traffic generator's test run configuration.
+        Returns:
+            A list of packets received after sending `packets`.
         """
-        super().__init__(tg_node, config)
-
-        assert (
-            self._tg_node.config.os == OS.linux
-        ), "Linux is the only supported OS for scapy traffic generation"
-
-        self.session = PythonShell(
-            self._tg_node, timeout=5, privileged=True, name="ScapyXMLRPCServer"
+        self._shell_create_sniffer(
+            packets, send_port, recv_port, self._create_packet_filter(filter_config)
         )
+        return self._shell_start_and_stop_sniffing(duration)
 
-        self.session.start_application()
+    def _shell_set_packet_list(self, packets: list[Packet]) -> None:
+        """Build a list of packets to send later.
 
-        # import libs in remote python console
-        for import_statement in SCAPY_RPC_SERVER_IMPORTS:
-            self.session.send_command(import_statement)
+        Sends the string that represents the Python command that was used to create each packet in
+        `packets` into the underlying Python session. The purpose behind doing this is to create a
+        list that is identical to `packets` inside the shell. This method should only be called by
+        methods for sending packets immediately prior to sending. The list of packets will continue
+        to exist in the scope of the shell until subsequent calls to this method, so failure to
+        rebuild the list prior to sending packets could lead to undesired "stale" packets to be
+        sent.
 
-        # start the server
-        xmlrpc_server_listen_port = 8000
-        self._start_xmlrpc_server_in_remote_python(xmlrpc_server_listen_port)
-
-        # connect to the server
-        server_url = f"http://{self._tg_node.config.hostname}:{xmlrpc_server_listen_port}"
-        self.rpc_server_proxy = xmlrpc.client.ServerProxy(
-            server_url, allow_none=True, verbose=SETTINGS.verbose
-        )
-
-        # add functions to the server
-        for function in RPC_FUNCTIONS:
-            # A slightly hacky way to move a function to the remote server.
-            # It is constructed from the name and code on the other side.
-            # Pickle cannot handle functions, nor can any of the other serialization
-            # frameworks aside from the libraries used to generate pyc files, which
-            # are even more messy to work with.
-            function_bytes = marshal.dumps(function.__code__)
-            self.rpc_server_proxy.add_rpc_function(function.__name__, function_bytes)
-
-    def _start_xmlrpc_server_in_remote_python(self, listen_port: int) -> None:
-        # load the source of the function
-        src = inspect.getsource(QuittableXMLRPCServer)
-        # Lines with only whitespace break the repl if in the middle of a function
-        # or class, so strip all lines containing only whitespace
-        src = "\n".join([line for line in src.splitlines() if not line.isspace() and line != ""])
-
-        # execute it in the python terminal
-        self.session.send_command(src + "\n")
-        self.session.send_command(
-            f"server = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));server.serve_forever()",
-            "XMLRPC OK",
+        Args:
+            packets: The list of packets to recreate in the shell.
+        """
+        self._logger.info("Building a list of packets to send.")
+        self.send_command(
+            f"{self._send_packet_list_name} = [{', '.join(map(Packet.command, packets))}]"
         )
 
-    def _send_packets(self, packets: list[Packet], port: Port) -> None:
-        packets = [packet.build() for packet in packets]
-        self.rpc_server_proxy.scapy_send_packets(packets, port.logical_name)
-
     def _create_packet_filter(self, filter_config: PacketFilteringConfig) -> str:
-        """Combines filter settings from `filter_config` into a BPF that scapy can use.
+        """Combine filter settings from `filter_config` into a BPF that scapy can use.
 
         Scapy allows for the use of Berkeley Packet Filters (BPFs) to filter what packets are
         collected based on various attributes of the packet.
@@ -331,36 +170,65 @@ def _create_packet_filter(self, filter_config: PacketFilteringConfig) -> str:
             bpf_filter.append("ether[12:2] != 0x88cc")
         return " && ".join(bpf_filter)
 
-    def _send_packets_and_capture(
-        self,
-        packets: list[Packet],
-        send_port: Port,
-        receive_port: Port,
-        filter_config: PacketFilteringConfig,
-        duration: float,
-        capture_name: str = _get_default_capture_name(),
-    ) -> list[Packet]:
-        binary_packets = [packet.build() for packet in packets]
-
-        xmlrpc_packets: list[
-            xmlrpc.client.Binary
-        ] = self.rpc_server_proxy.scapy_send_packets_and_capture(
-            binary_packets,
-            send_port.logical_name,
-            receive_port.logical_name,
-            duration,
-            self._create_packet_filter(filter_config),
-        )  # type: ignore[assignment]
-
-        scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets]
-        return scapy_packets
-
-    def close(self) -> None:
-        """Close the traffic generator."""
-        try:
-            self.rpc_server_proxy.quit()
-        except ConnectionRefusedError:
-            # Because the python instance closes, we get no RPC response.
-            # Thus, this error is expected
-            pass
-        self.session.close()
+    def _shell_create_sniffer(
+        self, packets_to_send: list[Packet], send_port: Port, recv_port: Port, filter_config: str
+    ) -> None:
+        """Create an asynchronous sniffer in the shell.
+
+        A list of packets is passed to the sniffer's callback function so that they are immediately
+        sent at the time sniffing is started.
+
+        Args:
+            packets_to_send: A list of packets to send when sniffing is started.
+            send_port: The port to send the packets on when sniffing is started.
+            recv_port: The port to collect the traffic from.
+            filter_config: An optional BPF format filter to use when sniffing for packets. Omitted
+                when set to an empty string.
+        """
+        self._shell_set_packet_list(packets_to_send)
+        sniffer_commands = [
+            f"{self._sniffer_name} = AsyncSniffer(",
+            f"iface='{recv_port.logical_name}',",
+            "store=True,",
+            # *args is used in the arguments of the lambda since Scapy sends parameters to the
+            # callback function which we do not need for our purposes.
+            "started_callback=lambda *args: sendp(",
+            (
+                # Additional indentation is added to this line only for readability of the logs.
+                f"{self._python_indentation}{self._send_packet_list_name},"
+                f" iface='{send_port.logical_name}'),"
+            ),
+            ")",
+        ]
+        if filter_config:
+            sniffer_commands.insert(-1, f"filter='{filter_config}'")
+
+        self.send_command(f"\n{self._python_indentation}".join(sniffer_commands))
+
+    def _shell_start_and_stop_sniffing(self, duration: float) -> list[Packet]:
+        """Start asynchronous sniffer, run for a set `duration`, then collect received packets.
+
+        This method expects that you have first created an asynchronous sniffer inside the shell
+        and will fail if you haven't. Received packets are collected by printing the base64
+        encoding of each packet in the shell and then harvesting these encodings using regex to
+        convert back into packet objects.
+
+        Args:
+            duration: The amount of time in seconds to sniff for received packets.
+
+        Returns:
+            A list of all packets that were received while the sniffer was running.
+        """
+        sniffed_packets_name = "gathered_packets"
+        self.send_command(f"{self._sniffer_name}.start()")
+        time.sleep(duration)
+        self.send_command(f"{sniffed_packets_name} = {self._sniffer_name}.stop(join=True)")
+        # An extra newline is required here due to the nature of interactive Python shells
+        packet_strs = self.send_command(
+            f"for pakt in {sniffed_packets_name}: print(bytes_base64(pakt.build()))\n"
+        )
+        # In the string of bytes "b'XXXX'", we only want the contents ("XXXX")
+        list_of_packets_base64 = re.findall(
+            f"^b'({REGEX_FOR_BASE64_ENCODING})'", packet_strs, re.MULTILINE
+        )
+        return [Ether(base64_bytes(pakt)) for pakt in list_of_packets_base64]
diff --git a/dts/framework/testbed_model/traffic_generator/traffic_generator.py b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
index 4ce1148706..176d5e9065 100644
--- a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
+++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
@@ -16,23 +16,29 @@
 from framework.logger import DTSLogger, get_dts_logger
 from framework.testbed_model.node import Node
 from framework.testbed_model.port import Port
-from framework.utils import get_packet_summaries
+from framework.utils import MultiInheritanceBaseClass, get_packet_summaries
 
 
-class TrafficGenerator(ABC):
+class TrafficGenerator(MultiInheritanceBaseClass, ABC):
     """The base traffic generator.
 
     Exposes the common public methods of all traffic generators and defines private methods
-    that must implement the traffic generation logic in subclasses.
+    that must implement the traffic generation logic in subclasses. This class also extends from
+    :class:`framework.utils.MultiInheritanceBaseClass` to allow subclasses the ability to inherit
+    from multiple classes to fulfil the traffic generating functionality without breaking
+    single-inheritance.
     """
 
     _config: TrafficGeneratorConfig
     _tg_node: Node
     _logger: DTSLogger
 
-    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
+    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig, **kwargs):
         """Initialize the traffic generator.
 
+        Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other
+        constructors in the case of multiple-inheritance.
+
         Args:
             tg_node: The traffic generator node where the created traffic generator will be running.
             config: The traffic generator's test run configuration.
@@ -40,6 +46,7 @@ def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
         self._config = config
         self._tg_node = tg_node
         self._logger = get_dts_logger(f"{self._tg_node.name} {self._config.traffic_generator_type}")
+        super().__init__(tg_node, **kwargs)
 
     def send_packet(self, packet: Packet, port: Port) -> None:
         """Send `packet` and block until it is fully sent.
diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index c768dd0c99..c68fc44517 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -29,6 +29,7 @@
 from .exception import ConfigurationError, InternalError
 
 REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
+REGEX_FOR_BASE64_ENCODING: str = "[-a-zA-Z0-9+\\/]*={0,3}"
 
 
 def expand_range(range_str: str) -> list[int]:
@@ -319,3 +320,17 @@ def _make_packet() -> Packet:
         return packet / random.randbytes(usable_payload_size)
 
     return [_make_packet() for _ in range(number_of)]
+
+
+class MultiInheritanceBaseClass:
+    """A base class for classes utilizing multiple-inheritance.
+
+    This class enables it's subclasses to support both single and multiple inheritance by acting as
+    a stopping point in the tree of calls to the constructors of super-classes. This class is able
+    to exist at the end of the Method Resolution Order (MRO) so that sub-classes can call super
+    without repercussion.
+    """
+
+    def __init__(self, *args, **kwargs) -> None:
+        """Call the init method of :class:`object`."""
+        super().__init__()
-- 
2.46.0

Re: [PATCH v4 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[Juraj Linkeš]

The subject line should start with a lowercase character and it's also 
over the character limit. This fits and basically says the same:
use PythonShell for Scapy instead of XML-RPC

Scapy TG didn't fit but I think the above still works.

> diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py

> @@ -93,9 +94,13 @@ def __init__(
>           timeout: float = SETTINGS.timeout,
>           app_params: Params = Params(),
>           name: str | None = None,
> +        **kwargs,
>       ) -> None:
>           """Create an SSH channel during initialization.
>   
> +        Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other

key-word -> keyword

> +        constructors in the case of multiple-inheritance.

I also didn't notice this hyphen, it shouldn't be there.

These hyphens are in a lot of places, I don't if I caught them all.

> diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py

> @@ -6,311 +6,150 @@
>   
>   A traffic generator used for functional testing, implemented with
>   `the Scapy library <https://scapy.readthedocs.io/en/latest/>`_.
> -The traffic generator uses an XML-RPC server to run Scapy on the remote TG node.
> +The traffic generator uses an interactive shell to run Scapy on the remote TG node.
>   
> -The traffic generator uses the :mod:`xmlrpc.server` module to run an XML-RPC server
> -in an interactive remote Python SSH session. The communication with the server is facilitated
> -with a local server proxy from the :mod:`xmlrpc.client` module.
> +The traffic generator extends :class:`framework.remote_session.python_shell.PythonShell` to
> +implement the methods for handling packets by sending commands into the interactive shell.

Since this only mentions PythonShell, people could understand that's the 
only thing the tg extends. We should either add a qualifier such as 
additionally extends or just explicitly say it also extends the 
capturing TG (which is also useful information as it tells us which kind 
of TG it is).


> +class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
> +    """Provides access to scapy functions on a traffic generator node.

> +    This class extends the base with remote execution of scapy functions. All methods for
> +    processing packets are implemented using an underlying
> +    :class:`framework.remote_session.python_shell.PythonShell` which imports the Scapy library.

> +    Because of the dual-inheritance, this class has both methods that wrap scapy commands sent into

The hyphen doesn't sound right. And maybe double would be a better term. 
I understand dual to mean two of the same and that doesn't fit for me as 
well.

> +    the shell and methods that run locally to fulfill traffic generation needs. To help make a
> +    clear distinction between the two, the names of the methods that wrap the logic of the
> +    underlying shell should be prepended with "shell".

I think there would be value in explicitly saying that the shell runs on 
the TG node.


> +    Note that the order of inheritance is important for this class. In order to instantiate this
> +    class, the abstract methods of :class:`~.capturing_traffic_generator.CapturingTrafficGenerator`
> +    must be implemented. Since some of these methods are implemented in the underlying interactive
> +    shell, according to Python's Method Resolution Order (MRO), the interactive shell must come
> +    first.

I didn't notice this before. Is this because of the close() method? Do 
we need to add any special steps to close the TG? Closing the 
interactive session should be enough, but I wanted to check with you.


> +    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig, **kwargs):
> +        """Extend the constructor with Scapy TG specifics.
>   
> -    def __init__(self, *args, **kwargs):
> -        """Extend the XML-RPC server initialization.
> +        Initializes both the traffic generator and the interactive shell used to handle Scapy
> +        functions. The interactive shell will be started on `tg_node`. The additional key-word
> +        arguments in `kwargs` are used to pass into the constructor for the interactive shell.
>   
>           Args:
> -            args: The positional arguments that will be passed to the superclass's constructor.
> -            kwargs: The keyword arguments that will be passed to the superclass's constructor.
> -                The `allow_none` argument will be set to :data:`True`.
> +            tg_node: The node where the traffic generator resides.
> +            config: The traffic generator's test run configuration.
> +            kwargs: Keyword arguments all traffic generators must support in order to allow for
> +                multiple-inheritance.

I'm not sure why all and why they must support them. We can just say the 
supported keyword arguments corresdpond to the parameters of 
:meth:`PythonShell.__init__`.

Also, multiple-inheritance -> multiple inheritance


> diff --git a/dts/framework/testbed_model/traffic_generator/traffic_generator.py b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
> index 4ce1148706..176d5e9065 100644
> --- a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
> +++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
> @@ -16,23 +16,29 @@
>   from framework.logger import DTSLogger, get_dts_logger
>   from framework.testbed_model.node import Node
>   from framework.testbed_model.port import Port
> -from framework.utils import get_packet_summaries
> +from framework.utils import MultiInheritanceBaseClass, get_packet_summaries
>   
>   
> -class TrafficGenerator(ABC):
> +class TrafficGenerator(MultiInheritanceBaseClass, ABC):
>       """The base traffic generator.
>   
>       Exposes the common public methods of all traffic generators and defines private methods
> -    that must implement the traffic generation logic in subclasses.
> +    that must implement the traffic generation logic in subclasses. This class also extends from
> +    :class:`framework.utils.MultiInheritanceBaseClass` to allow subclasses the ability to inherit
> +    from multiple classes to fulfil the traffic generating functionality without breaking
> +    single-inheritance.

Hyphen again.

>       """
>   
>       _config: TrafficGeneratorConfig
>       _tg_node: Node
>       _logger: DTSLogger
>   
> -    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
> +    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig, **kwargs):
>           """Initialize the traffic generator.
>   
> +        Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other
> +        constructors in the case of multiple-inheritance.
> +

keyword, multiple inheritance

Re: [PATCH v4 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[Jeremy Spewock]

On Thu, Sep 26, 2024 at 5:12 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
> The subject line should start with a lowercase character and it's also
> over the character limit. This fits and basically says the same:
> use PythonShell for Scapy instead of XML-RPC
>
> Scapy TG didn't fit but I think the above still works.
>
> > diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py
>
> > @@ -93,9 +94,13 @@ def __init__(
> >           timeout: float = SETTINGS.timeout,
> >           app_params: Params = Params(),
> >           name: str | None = None,
> > +        **kwargs,
> >       ) -> None:
> >           """Create an SSH channel during initialization.
> >
> > +        Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other
>
> key-word -> keyword

Ack.

>
> > +        constructors in the case of multiple-inheritance.
>
> I also didn't notice this hyphen, it shouldn't be there.
>
> These hyphens are in a lot of places, I don't if I caught them all.

I'm not sure why I put so many, haha. I'll look through and try to
find the rest.

>
> > diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
>
> > @@ -6,311 +6,150 @@
> >
> >   A traffic generator used for functional testing, implemented with
> >   `the Scapy library <https://scapy.readthedocs.io/en/latest/>`_.
> > -The traffic generator uses an XML-RPC server to run Scapy on the remote TG node.
> > +The traffic generator uses an interactive shell to run Scapy on the remote TG node.
> >
> > -The traffic generator uses the :mod:`xmlrpc.server` module to run an XML-RPC server
> > -in an interactive remote Python SSH session. The communication with the server is facilitated
> > -with a local server proxy from the :mod:`xmlrpc.client` module.
> > +The traffic generator extends :class:`framework.remote_session.python_shell.PythonShell` to
> > +implement the methods for handling packets by sending commands into the interactive shell.
>
> Since this only mentions PythonShell, people could understand that's the
> only thing the tg extends. We should either add a qualifier such as
> additionally extends or just explicitly say it also extends the
> capturing TG (which is also useful information as it tells us which kind
> of TG it is).

Good idea, I'll add this.

>
>
> > +class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
> > +    """Provides access to scapy functions on a traffic generator node.
>
> > +    This class extends the base with remote execution of scapy functions. All methods for
> > +    processing packets are implemented using an underlying
> > +    :class:`framework.remote_session.python_shell.PythonShell` which imports the Scapy library.
>
> > +    Because of the dual-inheritance, this class has both methods that wrap scapy commands sent into
>
> The hyphen doesn't sound right. And maybe double would be a better term.
> I understand dual to mean two of the same and that doesn't fit for me as
> well.

Ack.

>
> > +    the shell and methods that run locally to fulfill traffic generation needs. To help make a
> > +    clear distinction between the two, the names of the methods that wrap the logic of the
> > +    underlying shell should be prepended with "shell".
>
> I think there would be value in explicitly saying that the shell runs on
> the TG node.

Ack.

>
>
> > +    Note that the order of inheritance is important for this class. In order to instantiate this
> > +    class, the abstract methods of :class:`~.capturing_traffic_generator.CapturingTrafficGenerator`
> > +    must be implemented. Since some of these methods are implemented in the underlying interactive
> > +    shell, according to Python's Method Resolution Order (MRO), the interactive shell must come
> > +    first.
>
> I didn't notice this before. Is this because of the close() method? Do
> we need to add any special steps to close the TG? Closing the
> interactive session should be enough, but I wanted to check with you.

Yes it is because of the close method in the traffic generator. I
think closing the shell should be all we need to do really, there
isn't anything else this traffic generator is really using on the
host.

>
>
> > +    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig, **kwargs):
> > +        """Extend the constructor with Scapy TG specifics.
> >
> > -    def __init__(self, *args, **kwargs):
> > -        """Extend the XML-RPC server initialization.
> > +        Initializes both the traffic generator and the interactive shell used to handle Scapy
> > +        functions. The interactive shell will be started on `tg_node`. The additional key-word
> > +        arguments in `kwargs` are used to pass into the constructor for the interactive shell.
> >
> >           Args:
> > -            args: The positional arguments that will be passed to the superclass's constructor.
> > -            kwargs: The keyword arguments that will be passed to the superclass's constructor.
> > -                The `allow_none` argument will be set to :data:`True`.
> > +            tg_node: The node where the traffic generator resides.
> > +            config: The traffic generator's test run configuration.
> > +            kwargs: Keyword arguments all traffic generators must support in order to allow for
> > +                multiple-inheritance.
>
> I'm not sure why all and why they must support them. We can just say the

I guess other leaf subclasses of traffic generators which don't
inherit from two things wouldn't actually need them, you're right. I
was just thinking that they had to be supported since the base class
supports them to allow for the multiple inheritance, but thinking more
on it now not all of them have to.

> supported keyword arguments corresdpond to the parameters of
> :meth:`PythonShell.__init__`.

Sure, I like this wording.

>
> Also, multiple-inheritance -> multiple inheritance

Clearly I need to do some reading about when I should actually be
using hyphens :).

>
>
> > diff --git a/dts/framework/testbed_model/traffic_generator/traffic_generator.py b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
> > index 4ce1148706..176d5e9065 100644
> > --- a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
> > +++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
> > @@ -16,23 +16,29 @@
> >   from framework.logger import DTSLogger, get_dts_logger
> >   from framework.testbed_model.node import Node
> >   from framework.testbed_model.port import Port
> > -from framework.utils import get_packet_summaries
> > +from framework.utils import MultiInheritanceBaseClass, get_packet_summaries
> >
> >
> > -class TrafficGenerator(ABC):
> > +class TrafficGenerator(MultiInheritanceBaseClass, ABC):
> >       """The base traffic generator.
> >
> >       Exposes the common public methods of all traffic generators and defines private methods
> > -    that must implement the traffic generation logic in subclasses.
> > +    that must implement the traffic generation logic in subclasses. This class also extends from
> > +    :class:`framework.utils.MultiInheritanceBaseClass` to allow subclasses the ability to inherit
> > +    from multiple classes to fulfil the traffic generating functionality without breaking
> > +    single-inheritance.
>
> Hyphen again.

Ack.

>
> >       """
> >
> >       _config: TrafficGeneratorConfig
> >       _tg_node: Node
> >       _logger: DTSLogger
> >
> > -    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
> > +    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig, **kwargs):
> >           """Initialize the traffic generator.
> >
> > +        Additional key-word arguments can be passed through `kwargs` if needed for fulfilling other
> > +        constructors in the case of multiple-inheritance.
> > +
>
> keyword, multiple inheritance

Ack.

>
>

Re: [PATCH v4 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[Jeremy Spewock]

On Thu, Sep 26, 2024 at 5:12 AM Juraj Linkeš <juraj.linkes@pantheon.tech> wrote:
>
> The subject line should start with a lowercase character and it's also
> over the character limit. This fits and basically says the same:
> use PythonShell for Scapy instead of XML-RPC
>
> Scapy TG didn't fit but I think the above still works.
>

I forgot to respond to this too, but these are good points that I
overlooked and I like that wording so I'll update this as well.

<snip>
>

[PATCH v5 0/1] dts: replace XML-RPC server

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

v5:
 * renamed commit subject to match contribution guidelines
 * reformatted doc-strings to add more context and clarity
 * removed unnecessary hyphens

Jeremy Spewock (1):
  dts: use PythonShell for Scapy instead of XML-RPC

 .../single_active_interactive_shell.py        |   8 +-
 .../traffic_generator/__init__.py             |   2 +-
 .../testbed_model/traffic_generator/scapy.py  | 455 +++++++-----------
 .../traffic_generator/traffic_generator.py    |  15 +-
 dts/framework/utils.py                        |  15 +
 5 files changed, 196 insertions(+), 299 deletions(-)

-- 
2.46.0

[PATCH v5 1/1] dts: use PythonShell for Scapy instead of XML-RPC

[jspewock]

From: Jeremy Spewock <jspewock@iol.unh.edu>

Previously all scapy commands were handled using an XML-RPC server that
ran on the TGNode. This unnecessarily enforces a minimum Python version
of 3.10 on the server that is being used as a traffic generator and
complicates the implementation of scapy methods. This patch removes the
XML-RPC server completely and instead allows the Scapy TG to extend from
the PythonShell to implement the functionality of a traffic generator.
This is done by importing the Scapy library in the PythonShell and
sending commands directly to the interactive session on the TG Node.

Bugzilla ID: 1374

Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
---
 .../single_active_interactive_shell.py        |   8 +-
 .../traffic_generator/__init__.py             |   2 +-
 .../testbed_model/traffic_generator/scapy.py  | 455 +++++++-----------
 .../traffic_generator/traffic_generator.py    |  15 +-
 dts/framework/utils.py                        |  15 +
 5 files changed, 196 insertions(+), 299 deletions(-)

diff --git a/dts/framework/remote_session/single_active_interactive_shell.py b/dts/framework/remote_session/single_active_interactive_shell.py
index 77a4dcefdf..e3f6424e97 100644
--- a/dts/framework/remote_session/single_active_interactive_shell.py
+++ b/dts/framework/remote_session/single_active_interactive_shell.py
@@ -36,9 +36,10 @@
 from framework.params import Params
 from framework.settings import SETTINGS
 from framework.testbed_model.node import Node
+from framework.utils import MultiInheritanceBaseClass
 
 
-class SingleActiveInteractiveShell(ABC):
+class SingleActiveInteractiveShell(MultiInheritanceBaseClass, ABC):
     """The base class for managing interactive shells.
 
     This class shouldn't be instantiated directly, but instead be extended. It contains
@@ -93,9 +94,13 @@ def __init__(
         timeout: float = SETTINGS.timeout,
         app_params: Params = Params(),
         name: str | None = None,
+        **kwargs,
     ) -> None:
         """Create an SSH channel during initialization.
 
+        Additional keyword arguments can be passed through `kwargs` if needed for fulfilling other
+        constructors in the case of multiple inheritance.
+
         Args:
             node: The node on which to run start the interactive shell.
             privileged: Enables the shell to run as superuser.
@@ -115,6 +120,7 @@ def __init__(
         self._timeout = timeout
         # Ensure path is properly formatted for the host
         self._update_real_path(self.path)
+        super().__init__(node, **kwargs)
 
     def _setup_ssh_channel(self):
         self._ssh_channel = self._node.main_session.interactive_session.session.invoke_shell()
diff --git a/dts/framework/testbed_model/traffic_generator/__init__.py b/dts/framework/testbed_model/traffic_generator/__init__.py
index 6dac86a224..a319fa5320 100644
--- a/dts/framework/testbed_model/traffic_generator/__init__.py
+++ b/dts/framework/testbed_model/traffic_generator/__init__.py
@@ -36,7 +36,7 @@ def create_traffic_generator(
     """
     match traffic_generator_config:
         case ScapyTrafficGeneratorConfig():
-            return ScapyTrafficGenerator(tg_node, traffic_generator_config)
+            return ScapyTrafficGenerator(tg_node, traffic_generator_config, privileged=True)
         case _:
             raise ConfigurationError(
                 f"Unknown traffic generator: {traffic_generator_config.traffic_generator_type}"
diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py
index 13fc1107aa..e46a787ee5 100644
--- a/dts/framework/testbed_model/traffic_generator/scapy.py
+++ b/dts/framework/testbed_model/traffic_generator/scapy.py
@@ -6,311 +6,151 @@
 
 A traffic generator used for functional testing, implemented with
 `the Scapy library <https://scapy.readthedocs.io/en/latest/>`_.
-The traffic generator uses an XML-RPC server to run Scapy on the remote TG node.
+The traffic generator uses an interactive shell to run Scapy on the remote TG node.
 
-The traffic generator uses the :mod:`xmlrpc.server` module to run an XML-RPC server
-in an interactive remote Python SSH session. The communication with the server is facilitated
-with a local server proxy from the :mod:`xmlrpc.client` module.
+The traffic generator extends :class:`framework.remote_session.python_shell.PythonShell` to
+implement the methods for handling packets by sending commands into the interactive shell.
 """
 
-import inspect
-import marshal
+
+import re
 import time
-import types
-import xmlrpc.client
-from xmlrpc.server import SimpleXMLRPCServer
+from typing import ClassVar
 
-import scapy.all  # type: ignore[import-untyped]
+from scapy.compat import base64_bytes  # type: ignore[import-untyped]
 from scapy.layers.l2 import Ether  # type: ignore[import-untyped]
 from scapy.packet import Packet  # type: ignore[import-untyped]
 
 from framework.config import OS, ScapyTrafficGeneratorConfig
 from framework.remote_session.python_shell import PythonShell
-from framework.settings import SETTINGS
 from framework.testbed_model.node import Node
 from framework.testbed_model.port import Port
-
-from .capturing_traffic_generator import (
-    CapturingTrafficGenerator,
+from framework.testbed_model.traffic_generator.capturing_traffic_generator import (
     PacketFilteringConfig,
-    _get_default_capture_name,
 )
+from framework.utils import REGEX_FOR_BASE64_ENCODING
 
-"""
-========= BEGIN RPC FUNCTIONS =========
-
-All of the functions in this section are intended to be exported to a python
-shell which runs a scapy RPC server. These functions are made available via that
-RPC server to the packet generator. To add a new function to the RPC server,
-first write the function in this section. Then, if you need any imports, make sure to
-add them to SCAPY_RPC_SERVER_IMPORTS as well. After that, add the function to the list
-in EXPORTED_FUNCTIONS. Note that kwargs (keyword arguments) do not work via xmlrpc,
-so you may need to construct wrapper functions around many scapy types.
-"""
+from .capturing_traffic_generator import CapturingTrafficGenerator
 
-"""
-Add the line needed to import something in a normal python environment
-as an entry to this array. It will be imported before any functions are
-sent to the server.
-"""
-SCAPY_RPC_SERVER_IMPORTS = [
-    "from scapy.all import *",
-    "import xmlrpc",
-    "import sys",
-    "from xmlrpc.server import SimpleXMLRPCServer",
-    "import marshal",
-    "import pickle",
-    "import types",
-    "import time",
-]
-
-
-def scapy_send_packets_and_capture(
-    xmlrpc_packets: list[xmlrpc.client.Binary],
-    send_iface: str,
-    recv_iface: str,
-    duration: float,
-    sniff_filter: str,
-) -> list[bytes]:
-    """The RPC function to send and capture packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-        recv_iface: The logical name of the ingress interface.
-        duration: Capture for this amount of time, in seconds.
-
-    Returns:
-        A list of bytes. Each item in the list represents one packet, which needs
-        to be converted back upon transfer from the remote node.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    sniffer = scapy.all.AsyncSniffer(
-        iface=recv_iface,
-        store=True,
-        started_callback=lambda *args: scapy.all.sendp(scapy_packets, iface=send_iface),
-        filter=sniff_filter,
-    )
-    sniffer.start()
-    time.sleep(duration)
-    return [scapy_packet.build() for scapy_packet in sniffer.stop(join=True)]
-
-
-def scapy_send_packets(xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: str) -> None:
-    """The RPC function to send packets.
-
-    This function is meant to be executed on the remote TG node via the server proxy.
-    It only sends `xmlrpc_packets`, without capturing them.
-
-    Args:
-        xmlrpc_packets: The packets to send. These need to be converted to
-            :class:`~xmlrpc.client.Binary` objects before sending to the remote server.
-        send_iface: The logical name of the egress interface.
-    """
-    scapy_packets = [scapy.all.Packet(packet.data) for packet in xmlrpc_packets]
-    scapy.all.sendp(scapy_packets, iface=send_iface, realtime=True, verbose=True)
 
+class ScapyTrafficGenerator(PythonShell, CapturingTrafficGenerator):
+    """Provides access to scapy functions on a traffic generator node.
 
-"""
-Functions to be exposed by the scapy RPC server.
-"""
-RPC_FUNCTIONS = [
-    scapy_send_packets,
-    scapy_send_packets_and_capture,
-]
+    This class extends the base with remote execution of scapy functions. All methods for
+    processing packets are implemented using an underlying
+    :class:`framework.remote_session.python_shell.PythonShell` which imports the Scapy library. This
+    class also extends :class:`.capturing_traffic_generator.CapturingTrafficGenerator` to expose
+    methods that utilize said packet processing functionality to test suites.
 
-"""
-========= END RPC FUNCTIONS =========
-"""
-
-
-class QuittableXMLRPCServer(SimpleXMLRPCServer):
-    r"""Basic XML-RPC server.
-
-    The server may be augmented by functions serializable by the :mod:`marshal` module.
-
-    Example:
-        ::
+    Because of the double inheritance, this class has both methods that wrap scapy commands sent into
+    the shell (running on the TG node) and methods that run locally to fulfill traffic generation
+    needs. To help make a clear distinction between the two, the names of the methods that wrap the
+    logic of the underlying shell should be prepended with "shell".
 
-            def hello_world():
-                # to be sent to the XML-RPC server
-                print("Hello World!")
-
-            # start the XML-RPC server on the remote node
-            # this is done by starting a Python shell on the remote node
-            from framework.remote_session import PythonShell
-            # the example assumes you're already connected to a tg_node
-            session = tg_node.create_interactive_shell(PythonShell, timeout=5, privileged=True)
-
-            # then importing the modules needed to run the server
-            # and the modules for any functions later added to the server
-            session.send_command("import xmlrpc")
-            session.send_command("from xmlrpc.server import SimpleXMLRPCServer")
-
-            # sending the source code of this class to the Python shell
-            from xmlrpc.server import SimpleXMLRPCServer
-            src = inspect.getsource(QuittableXMLRPCServer)
-            src = "\n".join([l for l in src.splitlines() if not l.isspace() and l != ""])
-            spacing = "\n" * 4
-            session.send_command(spacing + src + spacing)
-
-            # then starting the server with:
-            command = "s = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));s.serve_forever()"
-            session.send_command(command, "XMLRPC OK")
+    Note that the order of inheritance is important for this class. In order to instantiate this
+    class, the abstract methods of :class:`~.capturing_traffic_generator.CapturingTrafficGenerator`
+    must be implemented. Since some of these methods are implemented in the underlying interactive
+    shell, according to Python's Method Resolution Order (MRO), the interactive shell must come
+    first.
+    """
 
-            # now the server is running on the remote node and we can add functions to it
-            # first connect to the server from the execution node
-            import xmlrpc.client
-            server_url = f"http://{tg_node.config.hostname}:8000"
-            rpc_server_proxy = xmlrpc.client.ServerProxy(server_url)
+    _config: ScapyTrafficGeneratorConfig
 
-            # get the function bytes to send
-            import marshal
-            function_bytes = marshal.dumps(hello_world.__code__)
-            rpc_server_proxy.add_rpc_function(hello_world.__name__, function_bytes)
+    #: Name of sniffer to ensure the same is used in all places
+    _sniffer_name: ClassVar[str] = "sniffer"
+    #: Name of variable that points to the list of packets inside the scapy shell.
+    _send_packet_list_name: ClassVar[str] = "packets"
+    #: Padding to add to the start of a line for python syntax compliance.
+    _python_indentation: ClassVar[str] = " " * 4
 
-            # now we can execute the function on the server
-            xmlrpc_binary_recv: xmlrpc.client.Binary = rpc_server_proxy.hello_world()
-            print(str(xmlrpc_binary_recv))
-    """
+    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig, **kwargs):
+        """Extend the constructor with Scapy TG specifics.
 
-    def __init__(self, *args, **kwargs):
-        """Extend the XML-RPC server initialization.
+        Initializes both the traffic generator and the interactive shell used to handle Scapy
+        functions. The interactive shell will be started on `tg_node`. The additional keyword
+        arguments in `kwargs` are used to pass into the constructor for the interactive shell.
 
         Args:
-            args: The positional arguments that will be passed to the superclass's constructor.
-            kwargs: The keyword arguments that will be passed to the superclass's constructor.
-                The `allow_none` argument will be set to :data:`True`.
+            tg_node: The node where the traffic generator resides.
+            config: The traffic generator's test run configuration.
+            kwargs: Additional keyword arguments. Supported arguments correspond to the parameters
+                of :meth:`PythonShell.__init__` in this case.
         """
-        kwargs["allow_none"] = True
-        super().__init__(*args, **kwargs)
-        self.register_introspection_functions()
-        self.register_function(self.quit)
-        self.register_function(self.add_rpc_function)
+        assert (
+            tg_node.config.os == OS.linux
+        ), "Linux is the only supported OS for scapy traffic generation"
 
-    def quit(self) -> None:
-        """Quit the server."""
-        self._BaseServer__shutdown_request = True
-        return None
+        super().__init__(tg_node, config=config, **kwargs)
+        self.start_application()
 
-    def add_rpc_function(self, name: str, function_bytes: xmlrpc.client.Binary) -> None:
-        """Add a function to the server from the local server proxy.
+    def start_application(self) -> None:
+        """Extends :meth:`framework.remote_session.interactive_shell.start_application`.
 
-        Args:
-              name: The name of the function.
-              function_bytes: The code of the function.
+        Adds a command that imports everything from the scapy library immediately after starting
+        the shell for usage in later calls to the methods of this class.
         """
-        function_code = marshal.loads(function_bytes.data)
-        function = types.FunctionType(function_code, globals(), name)
-        self.register_function(function)
+        super().start_application()
+        self.send_command("from scapy.all import *")
 
-    def serve_forever(self, poll_interval: float = 0.5) -> None:
-        """Extend the superclass method with an additional print.
+    def _send_packets(self, packets: list[Packet], port: Port) -> None:
+        """Implementation for sending packets without capturing any received traffic.
 
-        Once executed in the local server proxy, the print gives us a clear string to expect
-        when starting the server. The print means this function was executed on the XML-RPC server.
+        Provides a "fire and forget" method of sending packets.
         """
-        print("XMLRPC OK")
-        super().serve_forever(poll_interval)
-
-
-class ScapyTrafficGenerator(CapturingTrafficGenerator):
-    """Provides access to scapy functions via an RPC interface.
-
-    This class extends the base with remote execution of scapy functions.
-
-    Any packets sent to the remote server are first converted to bytes. They are received as
-    :class:`~xmlrpc.client.Binary` objects on the server side. When the server sends the packets
-    back, they are also received as :class:`~xmlrpc.client.Binary` objects on the client side, are
-    converted back to :class:`~scapy.packet.Packet` objects and only then returned from the methods.
-
-    Attributes:
-        session: The exclusive interactive remote session created by the Scapy
-            traffic generator where the XML-RPC server runs.
-        rpc_server_proxy: The object used by clients to execute functions
-            on the XML-RPC server.
-    """
-
-    session: PythonShell
-    rpc_server_proxy: xmlrpc.client.ServerProxy
-    _config: ScapyTrafficGeneratorConfig
-
-    def __init__(self, tg_node: Node, config: ScapyTrafficGeneratorConfig):
-        """Extend the constructor with Scapy TG specifics.
-
-        The traffic generator first starts an XML-RPC on the remote `tg_node`.
-        Then it populates the server with functions which use the Scapy library
-        to send/receive traffic:
+        self._shell_set_packet_list(packets)
+        send_command = [
+            "sendp(",
+            f"{self._send_packet_list_name},",
+            f"iface='{port.logical_name}',",
+            "realtime=True,",
+            "verbose=True",
+            ")",
+        ]
+        self.send_command(f"\n{self._python_indentation}".join(send_command))
 
-            * :func:`scapy_send_packets_and_capture`
-            * :func:`scapy_send_packets`
+    def _send_packets_and_capture(
+        self,
+        packets: list[Packet],
+        send_port: Port,
+        recv_port: Port,
+        filter_config: PacketFilteringConfig,
+        duration: float,
+    ) -> list[Packet]:
+        """Implementation for sending packets and capturing any received traffic.
 
-        To enable verbose logging from the xmlrpc client, use the :option:`--verbose`
-        command line argument or the :envvar:`DTS_VERBOSE` environment variable.
+        This method first creates an asynchronous sniffer that holds the packets to send, then
+        starts and stops said sniffer, collecting any packets that it had received while it was
+        running.
 
-        Args:
-            tg_node: The node where the traffic generator resides.
-            config: The traffic generator's test run configuration.
+        Returns:
+            A list of packets received after sending `packets`.
         """
-        super().__init__(tg_node, config)
-
-        assert (
-            self._tg_node.config.os == OS.linux
-        ), "Linux is the only supported OS for scapy traffic generation"
-
-        self.session = PythonShell(
-            self._tg_node, timeout=5, privileged=True, name="ScapyXMLRPCServer"
+        self._shell_create_sniffer(
+            packets, send_port, recv_port, self._create_packet_filter(filter_config)
         )
+        return self._shell_start_and_stop_sniffing(duration)
 
-        self.session.start_application()
+    def _shell_set_packet_list(self, packets: list[Packet]) -> None:
+        """Build a list of packets to send later.
 
-        # import libs in remote python console
-        for import_statement in SCAPY_RPC_SERVER_IMPORTS:
-            self.session.send_command(import_statement)
+        Sends the string that represents the Python command that was used to create each packet in
+        `packets` into the underlying Python session. The purpose behind doing this is to create a
+        list that is identical to `packets` inside the shell. This method should only be called by
+        methods for sending packets immediately prior to sending. The list of packets will continue
+        to exist in the scope of the shell until subsequent calls to this method, so failure to
+        rebuild the list prior to sending packets could lead to undesired "stale" packets to be
+        sent.
 
-        # start the server
-        xmlrpc_server_listen_port = 8000
-        self._start_xmlrpc_server_in_remote_python(xmlrpc_server_listen_port)
-
-        # connect to the server
-        server_url = f"http://{self._tg_node.config.hostname}:{xmlrpc_server_listen_port}"
-        self.rpc_server_proxy = xmlrpc.client.ServerProxy(
-            server_url, allow_none=True, verbose=SETTINGS.verbose
-        )
-
-        # add functions to the server
-        for function in RPC_FUNCTIONS:
-            # A slightly hacky way to move a function to the remote server.
-            # It is constructed from the name and code on the other side.
-            # Pickle cannot handle functions, nor can any of the other serialization
-            # frameworks aside from the libraries used to generate pyc files, which
-            # are even more messy to work with.
-            function_bytes = marshal.dumps(function.__code__)
-            self.rpc_server_proxy.add_rpc_function(function.__name__, function_bytes)
-
-    def _start_xmlrpc_server_in_remote_python(self, listen_port: int) -> None:
-        # load the source of the function
-        src = inspect.getsource(QuittableXMLRPCServer)
-        # Lines with only whitespace break the repl if in the middle of a function
-        # or class, so strip all lines containing only whitespace
-        src = "\n".join([line for line in src.splitlines() if not line.isspace() and line != ""])
-
-        # execute it in the python terminal
-        self.session.send_command(src + "\n")
-        self.session.send_command(
-            f"server = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));server.serve_forever()",
-            "XMLRPC OK",
+        Args:
+            packets: The list of packets to recreate in the shell.
+        """
+        self._logger.info("Building a list of packets to send.")
+        self.send_command(
+            f"{self._send_packet_list_name} = [{', '.join(map(Packet.command, packets))}]"
         )
 
-    def _send_packets(self, packets: list[Packet], port: Port) -> None:
-        packets = [packet.build() for packet in packets]
-        self.rpc_server_proxy.scapy_send_packets(packets, port.logical_name)
-
     def _create_packet_filter(self, filter_config: PacketFilteringConfig) -> str:
-        """Combines filter settings from `filter_config` into a BPF that scapy can use.
+        """Combine filter settings from `filter_config` into a BPF that scapy can use.
 
         Scapy allows for the use of Berkeley Packet Filters (BPFs) to filter what packets are
         collected based on various attributes of the packet.
@@ -331,36 +171,65 @@ def _create_packet_filter(self, filter_config: PacketFilteringConfig) -> str:
             bpf_filter.append("ether[12:2] != 0x88cc")
         return " && ".join(bpf_filter)
 
-    def _send_packets_and_capture(
-        self,
-        packets: list[Packet],
-        send_port: Port,
-        receive_port: Port,
-        filter_config: PacketFilteringConfig,
-        duration: float,
-        capture_name: str = _get_default_capture_name(),
-    ) -> list[Packet]:
-        binary_packets = [packet.build() for packet in packets]
-
-        xmlrpc_packets: list[
-            xmlrpc.client.Binary
-        ] = self.rpc_server_proxy.scapy_send_packets_and_capture(
-            binary_packets,
-            send_port.logical_name,
-            receive_port.logical_name,
-            duration,
-            self._create_packet_filter(filter_config),
-        )  # type: ignore[assignment]
-
-        scapy_packets = [Ether(packet.data) for packet in xmlrpc_packets]
-        return scapy_packets
-
-    def close(self) -> None:
-        """Close the traffic generator."""
-        try:
-            self.rpc_server_proxy.quit()
-        except ConnectionRefusedError:
-            # Because the python instance closes, we get no RPC response.
-            # Thus, this error is expected
-            pass
-        self.session.close()
+    def _shell_create_sniffer(
+        self, packets_to_send: list[Packet], send_port: Port, recv_port: Port, filter_config: str
+    ) -> None:
+        """Create an asynchronous sniffer in the shell.
+
+        A list of packets is passed to the sniffer's callback function so that they are immediately
+        sent at the time sniffing is started.
+
+        Args:
+            packets_to_send: A list of packets to send when sniffing is started.
+            send_port: The port to send the packets on when sniffing is started.
+            recv_port: The port to collect the traffic from.
+            filter_config: An optional BPF format filter to use when sniffing for packets. Omitted
+                when set to an empty string.
+        """
+        self._shell_set_packet_list(packets_to_send)
+        sniffer_commands = [
+            f"{self._sniffer_name} = AsyncSniffer(",
+            f"iface='{recv_port.logical_name}',",
+            "store=True,",
+            # *args is used in the arguments of the lambda since Scapy sends parameters to the
+            # callback function which we do not need for our purposes.
+            "started_callback=lambda *args: sendp(",
+            (
+                # Additional indentation is added to this line only for readability of the logs.
+                f"{self._python_indentation}{self._send_packet_list_name},"
+                f" iface='{send_port.logical_name}'),"
+            ),
+            ")",
+        ]
+        if filter_config:
+            sniffer_commands.insert(-1, f"filter='{filter_config}'")
+
+        self.send_command(f"\n{self._python_indentation}".join(sniffer_commands))
+
+    def _shell_start_and_stop_sniffing(self, duration: float) -> list[Packet]:
+        """Start asynchronous sniffer, run for a set `duration`, then collect received packets.
+
+        This method expects that you have first created an asynchronous sniffer inside the shell
+        and will fail if you haven't. Received packets are collected by printing the base64
+        encoding of each packet in the shell and then harvesting these encodings using regex to
+        convert back into packet objects.
+
+        Args:
+            duration: The amount of time in seconds to sniff for received packets.
+
+        Returns:
+            A list of all packets that were received while the sniffer was running.
+        """
+        sniffed_packets_name = "gathered_packets"
+        self.send_command(f"{self._sniffer_name}.start()")
+        time.sleep(duration)
+        self.send_command(f"{sniffed_packets_name} = {self._sniffer_name}.stop(join=True)")
+        # An extra newline is required here due to the nature of interactive Python shells
+        packet_strs = self.send_command(
+            f"for pakt in {sniffed_packets_name}: print(bytes_base64(pakt.build()))\n"
+        )
+        # In the string of bytes "b'XXXX'", we only want the contents ("XXXX")
+        list_of_packets_base64 = re.findall(
+            f"^b'({REGEX_FOR_BASE64_ENCODING})'", packet_strs, re.MULTILINE
+        )
+        return [Ether(base64_bytes(pakt)) for pakt in list_of_packets_base64]
diff --git a/dts/framework/testbed_model/traffic_generator/traffic_generator.py b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
index 4ce1148706..469a12a780 100644
--- a/dts/framework/testbed_model/traffic_generator/traffic_generator.py
+++ b/dts/framework/testbed_model/traffic_generator/traffic_generator.py
@@ -16,23 +16,29 @@
 from framework.logger import DTSLogger, get_dts_logger
 from framework.testbed_model.node import Node
 from framework.testbed_model.port import Port
-from framework.utils import get_packet_summaries
+from framework.utils import MultiInheritanceBaseClass, get_packet_summaries
 
 
-class TrafficGenerator(ABC):
+class TrafficGenerator(MultiInheritanceBaseClass, ABC):
     """The base traffic generator.
 
     Exposes the common public methods of all traffic generators and defines private methods
-    that must implement the traffic generation logic in subclasses.
+    that must implement the traffic generation logic in subclasses. This class also extends from
+    :class:`framework.utils.MultiInheritanceBaseClass` to allow subclasses the ability to inherit
+    from multiple classes to fulfil the traffic generating functionality without breaking
+    single inheritance.
     """
 
     _config: TrafficGeneratorConfig
     _tg_node: Node
     _logger: DTSLogger
 
-    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
+    def __init__(self, tg_node: Node, config: TrafficGeneratorConfig, **kwargs):
         """Initialize the traffic generator.
 
+        Additional keyword arguments can be passed through `kwargs` if needed for fulfilling other
+        constructors in the case of multiple inheritance.
+
         Args:
             tg_node: The traffic generator node where the created traffic generator will be running.
             config: The traffic generator's test run configuration.
@@ -40,6 +46,7 @@ def __init__(self, tg_node: Node, config: TrafficGeneratorConfig):
         self._config = config
         self._tg_node = tg_node
         self._logger = get_dts_logger(f"{self._tg_node.name} {self._config.traffic_generator_type}")
+        super().__init__(tg_node, **kwargs)
 
     def send_packet(self, packet: Packet, port: Port) -> None:
         """Send `packet` and block until it is fully sent.
diff --git a/dts/framework/utils.py b/dts/framework/utils.py
index c768dd0c99..970d1844dc 100644
--- a/dts/framework/utils.py
+++ b/dts/framework/utils.py
@@ -29,6 +29,7 @@
 from .exception import ConfigurationError, InternalError
 
 REGEX_FOR_PCI_ADDRESS: str = "/[0-9a-fA-F]{4}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}.[0-9]{1}/"
+REGEX_FOR_BASE64_ENCODING: str = "[-a-zA-Z0-9+\\/]*={0,3}"
 
 
 def expand_range(range_str: str) -> list[int]:
@@ -319,3 +320,17 @@ def _make_packet() -> Packet:
         return packet / random.randbytes(usable_payload_size)
 
     return [_make_packet() for _ in range(number_of)]
+
+
+class MultiInheritanceBaseClass:
+    """A base class for classes utilizing multiple inheritance.
+
+    This class enables it's subclasses to support both single and multiple inheritance by acting as
+    a stopping point in the tree of calls to the constructors of superclasses. This class is able
+    to exist at the end of the Method Resolution Order (MRO) so that subclasses can call
+    :meth:`super.__init__` without repercussion.
+    """
+
+    def __init__(self, *args, **kwargs) -> None:
+        """Call the init method of :class:`object`."""
+        super().__init__()
-- 
2.46.0

Re: [PATCH v4 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell

[Juraj Linkeš]

>>> +    Note that the order of inheritance is important for this class. In order to instantiate this
>>> +    class, the abstract methods of :class:`~.capturing_traffic_generator.CapturingTrafficGenerator`
>>> +    must be implemented. Since some of these methods are implemented in the underlying interactive
>>> +    shell, according to Python's Method Resolution Order (MRO), the interactive shell must come
>>> +    first.
>>
>> I didn't notice this before. Is this because of the close() method? Do
>> we need to add any special steps to close the TG? Closing the
>> interactive session should be enough, but I wanted to check with you.
> 
> Yes it is because of the close method in the traffic generator. I
> think closing the shell should be all we need to do really, there
> isn't anything else this traffic generator is really using on the
> host.
> 

Ok, thanks. We can keep it this way then.

Re: [PATCH v5 1/1] dts: use PythonShell for Scapy instead of XML-RPC

[Juraj Linkeš]

On 26. 9. 2024 18:50, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> Previously all scapy commands were handled using an XML-RPC server that
> ran on the TGNode. This unnecessarily enforces a minimum Python version
> of 3.10 on the server that is being used as a traffic generator and
> complicates the implementation of scapy methods. This patch removes the
> XML-RPC server completely and instead allows the Scapy TG to extend from
> the PythonShell to implement the functionality of a traffic generator.
> This is done by importing the Scapy library in the PythonShell and
> sending commands directly to the interactive session on the TG Node.
> 
> Bugzilla ID: 1374
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>
> ---

Reviewed-by: Juraj Linkeš <juraj.linkes@pantheon.tech>

Re: [PATCH v5 1/1] dts: use PythonShell for Scapy instead of XML-RPC

[Luca Vizzarro]

Reviewed-by: Luca Vizzarro <luca.vizzarro@arm.com>

Re: [PATCH v5 1/1] dts: use PythonShell for Scapy instead of XML-RPC

[Juraj Linkeš]

On 26. 9. 2024 18:50, jspewock@iol.unh.edu wrote:
> From: Jeremy Spewock <jspewock@iol.unh.edu>
> 
> Previously all scapy commands were handled using an XML-RPC server that
> ran on the TGNode. This unnecessarily enforces a minimum Python version
> of 3.10 on the server that is being used as a traffic generator and
> complicates the implementation of scapy methods. This patch removes the
> XML-RPC server completely and instead allows the Scapy TG to extend from
> the PythonShell to implement the functionality of a traffic generator.
> This is done by importing the Scapy library in the PythonShell and
> sending commands directly to the interactive session on the TG Node.
> 
> Bugzilla ID: 1374
> 
> Signed-off-by: Jeremy Spewock <jspewock@iol.unh.edu>

Applied to next-dts, thanks.