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

["Juraj Linkeš" <juraj.linkes@pantheon.tech>]

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