From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6103C43390; Tue, 21 Nov 2023 17:33:07 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id DF4D642EA2; Tue, 21 Nov 2023 17:33:06 +0100 (CET) Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mails.dpdk.org (Postfix) with ESMTP id 40B6B42E9D for ; Tue, 21 Nov 2023 17:33:05 +0100 (CET) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7A2F71042; Tue, 21 Nov 2023 08:33:51 -0800 (PST) Received: from [10.1.34.179] (e125442.arm.com [10.1.34.179]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 558CC3F6C4; Tue, 21 Nov 2023 08:33:03 -0800 (PST) Message-ID: Date: Tue, 21 Nov 2023 16:33:01 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v7 20/21] dts: scapy tg docstring update Content-Language: en-US To: =?UTF-8?Q?Juraj_Linke=C5=A1?= , thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, jspewock@iol.unh.edu, probb@iol.unh.edu, paul.szczepanek@arm.com Cc: dev@dpdk.org References: <20231108125324.191005-23-juraj.linkes@pantheon.tech> <20231115130959.39420-1-juraj.linkes@pantheon.tech> <20231115130959.39420-21-juraj.linkes@pantheon.tech> From: Yoan Picchi In-Reply-To: <20231115130959.39420-21-juraj.linkes@pantheon.tech> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 11/15/23 13:09, Juraj Linkeš wrote: > Format according to the Google format and PEP257, with slight > deviations. > > Signed-off-by: Juraj Linkeš > --- > .../testbed_model/traffic_generator/scapy.py | 91 +++++++++++-------- > 1 file changed, 54 insertions(+), 37 deletions(-) > > diff --git a/dts/framework/testbed_model/traffic_generator/scapy.py b/dts/framework/testbed_model/traffic_generator/scapy.py > index 51864b6e6b..ed4f879925 100644 > --- a/dts/framework/testbed_model/traffic_generator/scapy.py > +++ b/dts/framework/testbed_model/traffic_generator/scapy.py > @@ -2,14 +2,15 @@ > # Copyright(c) 2022 University of New Hampshire > # Copyright(c) 2023 PANTHEON.tech s.r.o. > > -"""Scapy traffic generator. > +"""The Scapy traffic generator. > > -Traffic generator used for functional testing, implemented using the Scapy library. > +A traffic generator used for functional testing, implemented with > +`the Scapy library `_. > The traffic generator uses an XML-RPC server to run Scapy on the remote TG node. > > -The XML-RPC server runs in an interactive remote SSH session running Python console, > -where we start the server. The communication with the server is facilitated with > -a local server proxy. > +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. > """ > > import inspect > @@ -69,20 +70,20 @@ def scapy_send_packets_and_capture( > recv_iface: str, > duration: float, > ) -> list[bytes]: > - """RPC function to send and capture packets. > + """The RPC function to send and capture packets. > > - The function is meant to be executed on the remote TG node. > + The 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 > - xmlrpc.client.Binary before sending to the remote server. > + :class:`~xmlrpc.client.Binary` objects before sending to the remote server. The string is not raw and no \s. As per you explanation a few commits earlier this might cause an issue with the tilda ? Looking around I see it also happen several time here and also in the previous commit. > 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. > + 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( > @@ -98,19 +99,15 @@ def scapy_send_packets_and_capture( > def scapy_send_packets( > xmlrpc_packets: list[xmlrpc.client.Binary], send_iface: str > ) -> None: > - """RPC function to send packets. > + """The RPC function to send packets. > > - The function is meant to be executed on the remote TG node. > - It doesn't return anything, only sends packets. > + The 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 > - xmlrpc.client.Binary before sending to the remote server. > + :class:`~xmlrpc.client.Binary` objects before sending to the remote server. > send_iface: The logical name of the egress interface. > - > - 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] > scapy.all.sendp(scapy_packets, iface=send_iface, realtime=True, verbose=True) > @@ -130,11 +127,19 @@ def scapy_send_packets( > > > class QuittableXMLRPCServer(SimpleXMLRPCServer): > - """Basic XML-RPC server that may be extended > - by functions serializable by the marshal module. > + r"""Basic XML-RPC server. But you have a raw string here, and I don't see the need why. > + > + The server may be augmented by functions serializable by the :mod:`marshal` module. > """ > > 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() > @@ -142,13 +147,12 @@ def __init__(self, *args, **kwargs): > 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. > - > - This is meant to be executed remotely. > + """Add a function to the server from the local server proxy. > > Args: > name: The name of the function. > @@ -159,6 +163,11 @@ def add_rpc_function(self, name: str, function_bytes: xmlrpc.client.Binary) -> N > 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 the function was executed on the XML-RPC server. > + """ > print("XMLRPC OK") > super().serve_forever(poll_interval) > > @@ -166,19 +175,12 @@ def serve_forever(self, poll_interval: float = 0.5) -> None: > class ScapyTrafficGenerator(CapturingTrafficGenerator): > """Provides access to scapy functions via an RPC interface. > > - 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. > - > - Any packets sent to the remote server are first converted to bytes. > - They are received as xmlrpc.client.Binary objects on the server side. > - When the server sends the packets back, they are also received as > - xmlrpc.client.Binary object on the client side, are converted back to Scapy > - packets and only then returned from the methods. > + The class extends the base with remote execution of scapy functions. > > - Arguments: > - tg_node: The node where the traffic generator resides. > - config: The user configuration of the traffic generator. > + 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 > @@ -192,6 +194,22 @@ class ScapyTrafficGenerator(CapturingTrafficGenerator): > _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. > + > + Args: > + tg_node: The node where the traffic generator resides. > + config: The traffic generator's test run configuration. > + """ > super().__init__(tg_node, config) > > assert ( > @@ -237,10 +255,8 @@ def _start_xmlrpc_server_in_remote_python(self, listen_port: int) -> None: > [line for line in src.splitlines() if not line.isspace() and line != ""] > ) > > - spacing = "\n" * 4 > - > # execute it in the python terminal > - self.session.send_command(spacing + src + spacing) > + self.session.send_command(src + "\n") > self.session.send_command( > f"server = QuittableXMLRPCServer(('0.0.0.0', {listen_port}));" > f"server.serve_forever()", > @@ -274,6 +290,7 @@ def _send_packets_and_capture( > return scapy_packets > > def close(self) -> None: > + """Close the traffic generator.""" > try: > self.rpc_server_proxy.quit() > except ConnectionRefusedError: