DPDK patches and discussions
 help / color / mirror / Atom feed
* [RFC PATCH v1 0/2] dts: replace XML-RPC server
@ 2024-06-05 17:52 jspewock
  2024-06-05 17:52 ` [RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy jspewock
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: jspewock @ 2024-06-05 17:52 UTC (permalink / raw)
  To: Luca.Vizzarro, probb, npratte, paul.szczepanek, juraj.linkes,
	yoan.picchi, thomas, wathsala.vithanage, Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy
  2024-06-05 17:52 [RFC PATCH v1 0/2] dts: replace XML-RPC server jspewock
@ 2024-06-05 17:52 ` jspewock
  2024-06-11 11:12   ` Juraj Linkeš
  2024-06-05 17:52 ` [RFC PATCH v1 2/2] dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell jspewock
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: jspewock @ 2024-06-05 17:52 UTC (permalink / raw)
  To: Luca.Vizzarro, probb, npratte, paul.szczepanek, juraj.linkes,
	yoan.picchi, thomas, wathsala.vithanage, Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH v1 2/2] dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell
  2024-06-05 17:52 [RFC PATCH v1 0/2] dts: replace XML-RPC server jspewock
  2024-06-05 17:52 ` [RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy jspewock
@ 2024-06-05 17:52 ` jspewock
  2024-06-11 10:46   ` Juraj Linkeš
  2024-06-20 23:11 ` [PATCH v1 0/1] dts: replace XML-RPC server jspewock
  2024-06-25 21:11 ` [PATCH v2 0/1] dts: replace XML-RPC server jspewock
  3 siblings, 1 reply; 13+ messages in thread
From: jspewock @ 2024-06-05 17:52 UTC (permalink / raw)
  To: Luca.Vizzarro, probb, npratte, paul.szczepanek, juraj.linkes,
	yoan.picchi, thomas, wathsala.vithanage, Honnappa.Nagarahalli
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v1 2/2] dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell
  2024-06-05 17:52 ` [RFC PATCH v1 2/2] dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell jspewock
@ 2024-06-11 10:46   ` Juraj Linkeš
  2024-06-17 19:57     ` Jeremy Spewock
  0 siblings, 1 reply; 13+ messages in thread
From: Juraj Linkeš @ 2024-06-11 10:46 UTC (permalink / raw)
  To: jspewock, Luca.Vizzarro, probb, npratte, paul.szczepanek,
	yoan.picchi, thomas, wathsala.vithanage, Honnappa.Nagarahalli
  Cc: dev

> 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).


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy
  2024-06-05 17:52 ` [RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy jspewock
@ 2024-06-11 11:12   ` Juraj Linkeš
  2024-06-17 19:45     ` Jeremy Spewock
  0 siblings, 1 reply; 13+ messages in thread
From: Juraj Linkeš @ 2024-06-11 11:12 UTC (permalink / raw)
  To: jspewock, Luca.Vizzarro, probb, npratte, paul.szczepanek,
	yoan.picchi, thomas, wathsala.vithanage, Honnappa.Nagarahalli
  Cc: dev


> 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)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy
  2024-06-11 11:12   ` Juraj Linkeš
@ 2024-06-17 19:45     ` Jeremy Spewock
  0 siblings, 0 replies; 13+ messages in thread
From: Jeremy Spewock @ 2024-06-17 19:45 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: Luca.Vizzarro, probb, npratte, paul.szczepanek, yoan.picchi,
	thomas, wathsala.vithanage, Honnappa.Nagarahalli, dev

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)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v1 2/2] dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell
  2024-06-11 10:46   ` Juraj Linkeš
@ 2024-06-17 19:57     ` Jeremy Spewock
  0 siblings, 0 replies; 13+ messages in thread
From: Jeremy Spewock @ 2024-06-17 19:57 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: Luca.Vizzarro, probb, npratte, paul.szczepanek, yoan.picchi,
	thomas, wathsala.vithanage, Honnappa.Nagarahalli, dev

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.

>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v1 0/1] dts: replace XML-RPC server
  2024-06-05 17:52 [RFC PATCH v1 0/2] dts: replace XML-RPC server jspewock
  2024-06-05 17:52 ` [RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy jspewock
  2024-06-05 17:52 ` [RFC PATCH v1 2/2] dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell jspewock
@ 2024-06-20 23:11 ` jspewock
  2024-06-20 23:11   ` [PATCH v1 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell jspewock
  2024-06-25 21:11 ` [PATCH v2 0/1] dts: replace XML-RPC server jspewock
  3 siblings, 1 reply; 13+ messages in thread
From: jspewock @ 2024-06-20 23:11 UTC (permalink / raw)
  To: juraj.linkes, Honnappa.Nagarahalli, paul.szczepanek, yoan.picchi,
	npratte, probb, thomas, wathsala.vithanage, Luca.Vizzarro
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v1 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell
  2024-06-20 23:11 ` [PATCH v1 0/1] dts: replace XML-RPC server jspewock
@ 2024-06-20 23:11   ` jspewock
  2024-06-21 14:14     ` Juraj Linkeš
  0 siblings, 1 reply; 13+ messages in thread
From: jspewock @ 2024-06-20 23:11 UTC (permalink / raw)
  To: juraj.linkes, Honnappa.Nagarahalli, paul.szczepanek, yoan.picchi,
	npratte, probb, thomas, wathsala.vithanage, Luca.Vizzarro
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell
  2024-06-20 23:11   ` [PATCH v1 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell jspewock
@ 2024-06-21 14:14     ` Juraj Linkeš
  2024-06-24 20:54       ` Jeremy Spewock
  0 siblings, 1 reply; 13+ messages in thread
From: Juraj Linkeš @ 2024-06-21 14:14 UTC (permalink / raw)
  To: jspewock, Honnappa.Nagarahalli, paul.szczepanek, yoan.picchi,
	npratte, probb, thomas, wathsala.vithanage, Luca.Vizzarro
  Cc: dev


> +    #: 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.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v1 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell
  2024-06-21 14:14     ` Juraj Linkeš
@ 2024-06-24 20:54       ` Jeremy Spewock
  0 siblings, 0 replies; 13+ messages in thread
From: Jeremy Spewock @ 2024-06-24 20:54 UTC (permalink / raw)
  To: Juraj Linkeš
  Cc: Honnappa.Nagarahalli, paul.szczepanek, yoan.picchi, npratte,
	probb, thomas, wathsala.vithanage, Luca.Vizzarro, dev

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.

>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 0/1] dts: replace XML-RPC server
  2024-06-05 17:52 [RFC PATCH v1 0/2] dts: replace XML-RPC server jspewock
                   ` (2 preceding siblings ...)
  2024-06-20 23:11 ` [PATCH v1 0/1] dts: replace XML-RPC server jspewock
@ 2024-06-25 21:11 ` jspewock
  2024-06-25 21:11   ` [PATCH v2 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell jspewock
  3 siblings, 1 reply; 13+ messages in thread
From: jspewock @ 2024-06-25 21:11 UTC (permalink / raw)
  To: thomas, juraj.linkes, Honnappa.Nagarahalli, wathsala.vithanage,
	paul.szczepanek, npratte, probb, yoan.picchi, Luca.Vizzarro
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell
  2024-06-25 21:11 ` [PATCH v2 0/1] dts: replace XML-RPC server jspewock
@ 2024-06-25 21:11   ` jspewock
  0 siblings, 0 replies; 13+ messages in thread
From: jspewock @ 2024-06-25 21:11 UTC (permalink / raw)
  To: thomas, juraj.linkes, Honnappa.Nagarahalli, wathsala.vithanage,
	paul.szczepanek, npratte, probb, yoan.picchi, Luca.Vizzarro
  Cc: dev, Jeremy Spewock

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-06-25 21:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-06-05 17:52 [RFC PATCH v1 0/2] dts: replace XML-RPC server jspewock
2024-06-05 17:52 ` [RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy jspewock
2024-06-11 11:12   ` Juraj Linkeš
2024-06-17 19:45     ` Jeremy Spewock
2024-06-05 17:52 ` [RFC PATCH v1 2/2] dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell jspewock
2024-06-11 10:46   ` Juraj Linkeš
2024-06-17 19:57     ` Jeremy Spewock
2024-06-20 23:11 ` [PATCH v1 0/1] dts: replace XML-RPC server jspewock
2024-06-20 23:11   ` [PATCH v1 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell jspewock
2024-06-21 14:14     ` Juraj Linkeš
2024-06-24 20:54       ` Jeremy Spewock
2024-06-25 21:11 ` [PATCH v2 0/1] dts: replace XML-RPC server jspewock
2024-06-25 21:11   ` [PATCH v2 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell jspewock

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).