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 DD2E945A25; Wed, 25 Sep 2024 09:49:32 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 7DB154025D; Wed, 25 Sep 2024 09:49:32 +0200 (CEST) Received: from mail-ej1-f52.google.com (mail-ej1-f52.google.com [209.85.218.52]) by mails.dpdk.org (Postfix) with ESMTP id AAE48400EF for ; Wed, 25 Sep 2024 09:49:31 +0200 (CEST) Received: by mail-ej1-f52.google.com with SMTP id a640c23a62f3a-a8a897bd4f1so942570166b.3 for ; Wed, 25 Sep 2024 00:49:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pantheon.tech; s=google; t=1727250571; x=1727855371; 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=lVu/p9j7Bq0H5Q0Vzhqc2n0hT/mnEZLSNYcbckrqEMo=; b=tSDvSVGcHXrGlQRz4WDHfQxJk89guPFWsAh0QHn7wiclWVGRxp6v8CVlOqg7zEBLWc YdGufoDmd634o78sxVuPoKettNQsoNgmfVka8vW/RGUpAgb7h3zVeJXNJ03UcPkddybM 0OID0kJRXZSuwNUS9ECLlQr78GG+DfXNGQ65RXcpXKDylVd6/Rd+OMlElMvlDwKQafsZ q3TDn2BY9O8V2yjUDsEc8e7vdNEiBKo9Gmc2i8tBfaWyY7Wpm3mvfRTGM8Mfj53hovqX ZfP2vR9QgiNLV0mMURMhliLSy/sutFGgNIwuYi7XFVJNAWWUFzjLxQneMXV0i+scEz/w J2Pg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727250571; x=1727855371; 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=lVu/p9j7Bq0H5Q0Vzhqc2n0hT/mnEZLSNYcbckrqEMo=; b=QSTfrkLobs2V4xKbjFfKcL/Q2O+J4riCBI2Rb5F1iXGvSTEQvpxDPCAF9m7adUI2y/ EDDbOrqYEdw/W48XTV8n0bXXI7lrmSo02yig5pnRJYRntJNf/vSTqgWzb2GC/mVtjqDG CY1/AZvWinTdRuFcVLyUjSB7Iwc2fE8O7iMgcqcw2eZU7oIZvkzsvuQbfKdoTaxrw88O /sFRQRz7quHSh1CGEtfoDO9QviwDDduA3kdBnmvh41zHp5UO4RcuizBKnicfUORNCwtD uNcETKkhpq9gqyX1oNXBht7AjJdQh++QRqfZwstce8GE+qTtW2N9rLlDSImW6JciEbrx rEEA== X-Forwarded-Encrypted: i=1; AJvYcCVMxR51xNUGjBYhh6Z7GcEGgmL0MfeKO0I0qkQYbhx3ukmgEwZA/CypBmq9RaooG8aLSQw=@dpdk.org X-Gm-Message-State: AOJu0YxzMQ9CFyBSWTg+5QsstYz60Ff+syOILYdV/Jvco8X3S0RHP1We efKV3ZMHcKfgPgDx/vIHK5SfefQXPz5mjTMZNSLGP5vUA0o+GnBrxx4f01roC/o= X-Google-Smtp-Source: AGHT+IGTPZf2sWBGtLzaTyOW9Cn31x6Mj6t7IF6ngeP1U2c/gSI3l6kf+dHnuvlhE1JLU7I4efXT8g== X-Received: by 2002:a17:906:c141:b0:a86:af10:6a47 with SMTP id a640c23a62f3a-a93a06bd07amr138721766b.60.1727250571177; Wed, 25 Sep 2024 00:49:31 -0700 (PDT) Received: from [192.168.200.22] ([84.245.121.62]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a9392f341fesm182274366b.18.2024.09.25.00.49.28 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 25 Sep 2024 00:49:30 -0700 (PDT) Message-ID: <218e4b67-bdc9-4162-b0a0-a053001fd9f9@pantheon.tech> Date: Wed, 25 Sep 2024 09:49:28 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell To: Jeremy Spewock Cc: alex.chapman@arm.com, npratte@iol.unh.edu, Luca.Vizzarro@arm.com, probb@iol.unh.edu, paul.szczepanek@arm.com, thomas@monjalon.net, Honnappa.Nagarahalli@arm.com, wathsala.vithanage@arm.com, yoan.picchi@foss.arm.com, dev@dpdk.org References: <20240605175227.7003-1-jspewock@iol.unh.edu> <20240919190229.22095-1-jspewock@iol.unh.edu> <20240919190229.22095-2-jspewock@iol.unh.edu> <0e66d03d-652b-481a-96ac-7c18337a1e07@pantheon.tech> Content-Language: en-US From: =?UTF-8?Q?Juraj_Linke=C5=A1?= In-Reply-To: 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 24. 9. 2024 18:34, Jeremy Spewock wrote: > On Tue, Sep 24, 2024 at 6:55 AM Juraj Linkeš wrote: >> >> I like how this looks. I have a number of minor comments (mainly wording >> and naming), but overall it looks very good. >> >> On 19. 9. 2024 21:02, jspewock@iol.unh.edu wrote: >>> From: Jeremy Spewock >>> >>> 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. >> >> What is the TG's minimum Python version now? >> https://bugs.dpdk.org/show_bug.cgi?id=1388 says this will become a moot >> point, but we're still using Python on the remote node. > > Right, there is still some dependency there. I'm not sure the exact > versions, but doing some looking around I believe one of the newest > things scapy tools we use in the framework is the AsyncSniffer and the > scapy documentation [1] says that it has been available since version > 2.4.3, and then when I looked at that version of the scapy package [2] > it looks like it claims to support python 2.7 and python 3.4-7. Poking > around in the documentation/code from scapy version 2.4.3 it also > looks like the syntax is very similar, so I believe it would work, but > I'm not sure I have any hosts that I could run on which have python > 3.4. Maybe that does make the dependency essentially a moot point > considering these are fairly old python versions. > > [1] https://github.com/secdev/scapy/blob/master/doc/scapy/usage.rst > [2] https://pypi.org/project/scapy/2.4.3/ > Great. We should still document that the TG needs Python. I've updated the ticket. [0] [0] https://bugs.dpdk.org/show_bug.cgi?id=1388 >>> + def _create_packet_list(self, packets: list[Packet]) -> None: >> >> Maybe we could apply some of the ideas from the local/remote naming >> scheme I talked about in the tg devbind script patch here. Whatever >> happens on the TG could be prefixed with remote and whatever is >> happening locally would be without the prefix (or maybe whatever is >> happnening on the TG shouldn't be prefixed (or a different prefix - >> shell)? Makes sense, but then we'd need a good prefix for what's >> happening on the execution environment. Maybe this also needs to be in a >> different patch, after it's been though through with everything else in > > I'll still write something here that makes the distinction and that > other patch could reformat if the author thought something else was > better. > Great. >>> + sniffer_commands = [ >>> + f"{self._sniffer_name} = AsyncSniffer(", >>> + f"iface='{recv_port.logical_name}',", >>> + "store=True,", >>> + "started_callback=lambda *args: sendp(", >> >> As far as I can tell, we're not using the args, so we can just use >> "lambda: sendp()" > > We don't use the argument, but there are positional arguments passed > into this function by scapy which is why we have to catch and ignore > them. > Makes sense now, thanks for the explanation. Maybe we could put somewhere in here a little comment pointing this out? >> >>> + ( >>> + f"{self._python_indentation}{self._send_packet_list_name}," >> >> Is the indentation needed here? > > It's not required, but I think it makes the logs more readable. > That's a worthy use, let's keep it. Maybe also add an explanatory comment here?