On Tue, Jun 11, 2024 at 6:46 AM Juraj Linkeš <juraj.lin...@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. >
