From: "Juraj Linkeš" <juraj.linkes@pantheon.tech>
To: Jeremy Spewock <jspewock@iol.unh.edu>
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
Subject: Re: [PATCH v3 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell
Date: Wed, 25 Sep 2024 09:49:28 +0200 [thread overview]
Message-ID: <218e4b67-bdc9-4162-b0a0-a053001fd9f9@pantheon.tech> (raw)
In-Reply-To: <CAAA20UQMt-VE-cbp4tFL7Es-QVqYGfpY39-DyChsKX=G7=XViw@mail.gmail.com>
On 24. 9. 2024 18:34, Jeremy Spewock wrote:
> On Tue, Sep 24, 2024 at 6:55 AM Juraj Linkeš <juraj.linkes@pantheon.tech> 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 <jspewock@iol.unh.edu>
>>>
>>> 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?
next prev parent reply other threads:[~2024-09-25 7:49 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-06-05 17:52 [RFC PATCH v1 0/2] dts: replace XML-RPC server jspewock
2024-06-05 17:52 ` [RFC PATCH v1 1/2] dts: Add interactive shell for managing Scapy jspewock
2024-06-11 11:12 ` Juraj Linkeš
2024-06-17 19:45 ` Jeremy Spewock
2024-06-05 17:52 ` [RFC PATCH v1 2/2] dts: Remove XML-RPC server for Scapy TG and instead us ScapyShell jspewock
2024-06-11 10:46 ` Juraj Linkeš
2024-06-17 19:57 ` Jeremy Spewock
2024-06-20 23:11 ` [PATCH v1 0/1] dts: replace XML-RPC server jspewock
2024-06-20 23:11 ` [PATCH v1 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell jspewock
2024-06-21 14:14 ` Juraj Linkeš
2024-06-24 20:54 ` Jeremy Spewock
2024-06-25 21:11 ` [PATCH v2 0/1] dts: replace XML-RPC server jspewock
2024-06-25 21:11 ` [PATCH v2 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell jspewock
2024-09-12 4:00 ` Patrick Robb
2024-09-19 19:02 ` [PATCH v3 0/1] dts: replace XML-RPC server jspewock
2024-09-19 19:02 ` [PATCH v3 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell jspewock
2024-09-24 10:55 ` Juraj Linkeš
2024-09-24 16:34 ` Jeremy Spewock
2024-09-25 7:49 ` Juraj Linkeš [this message]
2024-09-25 17:37 ` [PATCH v4 0/1] dts: replace XML-RPC server jspewock
2024-09-25 17:37 ` [PATCH v4 1/1] dts: Remove XML-RPC server for Scapy TG and instead use PythonShell jspewock
2024-09-26 9:12 ` Juraj Linkeš
2024-09-26 14:54 ` Jeremy Spewock
2024-09-27 9:35 ` Juraj Linkeš
2024-09-26 14:55 ` Jeremy Spewock
2024-09-26 16:50 ` [PATCH v5 0/1] dts: replace XML-RPC server jspewock
2024-09-26 16:50 ` [PATCH v5 1/1] dts: use PythonShell for Scapy instead of XML-RPC jspewock
2024-09-27 9:42 ` Juraj Linkeš
2024-09-27 11:47 ` Luca Vizzarro
2024-09-30 13:41 ` Juraj Linkeš
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=218e4b67-bdc9-4162-b0a0-a053001fd9f9@pantheon.tech \
--to=juraj.linkes@pantheon.tech \
--cc=Honnappa.Nagarahalli@arm.com \
--cc=Luca.Vizzarro@arm.com \
--cc=alex.chapman@arm.com \
--cc=dev@dpdk.org \
--cc=jspewock@iol.unh.edu \
--cc=npratte@iol.unh.edu \
--cc=paul.szczepanek@arm.com \
--cc=probb@iol.unh.edu \
--cc=thomas@monjalon.net \
--cc=wathsala.vithanage@arm.com \
--cc=yoan.picchi@foss.arm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).