On 6/9/23 12:19, Ihar Hrachyshka wrote:
This thread is to discuss improvements that would allow OVN (and maybe
OVS?) to adopt fmt_pkt (frontend to scapy packet constructor) in the test
suite.
(For those not aware of what it is, please consult this commit:
https://github.com/ovn-org/ovn/commit/d8c0d3ae4f3a293737d662a865761235ed9ea1cf
There are a number of examples of usage of the tool in the OVN tree, please
`grep` for `fmt_pkt`.
tl;dr it allows us to get rid of hex strings that define packets under test
and replace them with symbolic representations of packet format.
Here is what I sent recently to my colleagues that captures my current
thinking. What I'm looking for is: feedback on the ideas listed, maybe also
their relative priorities. I'm also looking for any other ideas that would
improve scapy without destroying its value in speed of test composition and
consumption by human beings.
I may or may not work on these improvements in the near future, but
regardless, let's at least agree on the direction this tool should take.
===
**Opinion**: fmt_pkt is great.
**Problem**: it’s slow: every time it’s invoked, a `python` interpreter is
started, all `scapy` modules are loaded, then a packet string
representation is produced. This takes a long time. If we want to adopt it
wider for the test suite, we'll have to make it quicker.
```
stack@ubuntu2204:~/neutron$ time python3 -c 'from scapy.all import *'
real 0m0.551s
user 0m0.454s
sys 0m0.093s
```
If a test does it a number of times, the total clock time runs to a minute
for a single test case. It doesn't scale.
**Ideas**
1. Don't import all modules from `scapy`.
Initial tests suggest that tailoring the list of `scapy` modules to import
significantly reduces the time spent to produce a string for a packet.
```
stack@ubuntu2204:~/neutron$ time python3 -c 'from scapy.all import *'
real 0m0.551s
user 0m0.454s
sys 0m0.093s
stack@ubuntu2204:~/neutron$ time python3 -c 'from scapy.layers import l2'
real 0m0.190s
user 0m0.161s
sys 0m0.025s
```
In the example above, we shaved 2/3 of the time.
How the list of imported modules is passed into `fmt_pkt` is subject to
discussion.
- One way is making the caller pass the list - or a macro that refers to a
pre-defined list - to each call. This can be further hidden from the caller
by introducing special helpers for specific types of packets, e.g. have
`fmt_l2_pkt` or `fmt_tcp_pkt` depending on how specific we would like to be.
- Another way is making the test case declare the list of modules of
interest before using `fmt_pkt` once at the start, then assume this
case-scoped global setting is applied to all consequent `fmt_pkt` calls in
the test case.
2. Avoid duplicate module imports by running a `scapy` wrapper daemon.
The daemon would start `python`, load the (necessary?) `scapy` modules,
open a UNIX socket and run a simple server that will:
- receive a symbolic string from `fmt_pkt` call that represents the packet;
- run the transformation step to produce a string that represents the
packet byte stream;
- return the byte stream through the socket.
Since the test suite is executed in public CI on patches posted to ML by
unauthorized users, the daemon must expect - and deny - transformation for
anything that doesn't look like a `scapy` packet definition.
Whether the daemon should be scoped to a test case or to the whole run is
something to discuss.
- On one hand, scoping the daemon to a test case simplifies the cleanup and
tracking of the process. (We can make sure it's killed during the regular
CLEANUP phase for a particular test case.)
- On the other hand, running the same daemon for all test cases shaves time
spent to start it and import `scapy` modules to near-zero (we could say
it's `O(1)` if we feel fancy.) I don't know if having a global daemon for
the whole test suite run complicates the cleanup procedure.
If we go with per-case daemon mode, then we should at least not start it
for test cases that won't use `fmt_pkt` at all. I don't know which
mechanism should be used to track these. Perhaps there should be a macro
that tags tests for this setup step.
3. If `fmt_pkt` proves to be useful and successful in broader scope and
gets adopted in most of test cases, we may consider making daemon startup
the default behavior for a test suite run.
4. If this happens, we should as well remove the `HAVE_SCAPY` macro and
assume it's always installed on a test node.
===
Looking forward to any comments,
Cheers,
Ihar
Hi Ihar,
I think 3 is overall the best goal here. It makes test-writing easiest
since the author does not need to consider which individual scapy
modules might be needed. The big thing is that the daemon will need to
be in a "clean" state between tests. That may mean establishing a new
unix socket for each test, or it may mean ensuring that the common unix
socket used by all tests is flushed between tests.
I suspect that fmt_pkt will be the recommended way of rendering packets
in the future, and I suspect existing tests may be re-written to use it.
It will be easier and more efficient to always have the daemon running
than to try to conditionally run it for certain tests.
Thanks for the writeup,
Mark Michelson
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
