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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
