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 D5F4E454B7; Fri, 21 Jun 2024 16:14:28 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id C6C524026A; Fri, 21 Jun 2024 16:14:28 +0200 (CEST) Received: from mail-lj1-f182.google.com (mail-lj1-f182.google.com [209.85.208.182]) by mails.dpdk.org (Postfix) with ESMTP id CAB1E40265 for ; Fri, 21 Jun 2024 16:14:27 +0200 (CEST) Received: by mail-lj1-f182.google.com with SMTP id 38308e7fff4ca-2ec4a11a297so15954261fa.0 for ; Fri, 21 Jun 2024 07:14:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1718979267; x=1719584067; darn=dpdk.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=Mo9P+kYkVKmZuOLzJqjw3ONCWsFcN2pj0OYjP100KqU=; b=IoR8mUDHubA1OWhlRF9RoXogbTs8kHaXA4pGGeCN3bwQbQ5NbJ7/oGaOHr4S/RE1RT VZLrZJ6KUn+cIgmRS01tneSKhD3xUZ5pj8N2z5tb0HqiOJXR+m8f0eZIRAspPx5L4VDq WGr9L/VVRTIiyEmSJYNQ3fk44fsSo9I6ECxh9f5Ve++ladn2pxq5F03JKTYRPX5mSiQl H/P/ue8yp+LIOQEkF/Y16ZWdnbiYZxjmkDQwx0Of5TahOGMYqdL5N4fP8qf1gDFBIdjZ jIViGT/guV4KoUbTtZPlf6Uq5XPKoDWdO/Zoq3I69hIl1kwspRrvZk2uW0YpZQr1k8yL uEGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718979267; x=1719584067; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Mo9P+kYkVKmZuOLzJqjw3ONCWsFcN2pj0OYjP100KqU=; b=G+9NqjWVqYjXIG/cvg/Etx9JxHJlBLBA4SHilH7v2kR1QBBdrKsz91P2xWzbKJzVCv Ta9rpA+BY+GwvTzQDsAw6sr3qdmC35gf8JSY/Uc7dvNvrlb2saK+1YXtYbvzslxfp8/F iKs2BZWzN7Hm4vNOZ3DJ6HSkIW7BfI78QtodQ4UQAvYMyf5QjVUEw0ngXLGdgc2vSsn6 6uBfeC/uZ44iZaaUBlJnKhRniuiqeQGFRSqFNo4q5vANU+zQq89i0bMI5SqM5K1thJE3 1uvFuRndkoyrGiiHpPQk7od0j0+6TAPfvvQAf98Jngd7DSfC6jFzwj9K/ytPLDtMtDoC Wi8A== X-Gm-Message-State: AOJu0YznHxgRGgLKSY/xAymbC8KW7u1MCWT/hY4GYsVzsL8CbViX8ffe ttfUud0zURRGBvdEouRi8IKe+DLVuNqgoF3Q4827q2i4t9YEaw42anD5nOKXaNs= X-Google-Smtp-Source: AGHT+IGynl5Z/s6MY5deFNJ5BT0W2srGYiPtgIfl4arbc6XjKc6Oyj6TMJzhSPBhoVryDGqBjWtdGA== X-Received: by 2002:a2e:8096:0:b0:2eb:e4b1:92e with SMTP id 38308e7fff4ca-2ec3cffc75dmr57514891fa.48.1718979267112; Fri, 21 Jun 2024 07:14:27 -0700 (PDT) Received: from [192.168.1.113] ([84.245.121.236]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-57d3042b9eesm995648a12.35.2024.06.21.07.14.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 21 Jun 2024 07:14:26 -0700 (PDT) Message-ID: Date: Fri, 21 Jun 2024 16:14:25 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v1 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell To: jspewock@iol.unh.edu, Honnappa.Nagarahalli@arm.com, paul.szczepanek@arm.com, yoan.picchi@foss.arm.com, npratte@iol.unh.edu, probb@iol.unh.edu, thomas@monjalon.net, wathsala.vithanage@arm.com, Luca.Vizzarro@arm.com Cc: dev@dpdk.org References: <20240605175227.7003-1-jspewock@iol.unh.edu> <20240620231158.12008-1-jspewock@iol.unh.edu> <20240620231158.12008-3-jspewock@iol.unh.edu> Content-Language: en-US From: =?UTF-8?Q?Juraj_Linke=C5=A1?= In-Reply-To: <20240620231158.12008-3-jspewock@iol.unh.edu> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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 > + #: 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.