On 6/9/23 18: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,
Thanks for putting this together Ihar! I don't have a lot of comments but, if I get a vote, I vote for option 3 above. Regards, Dumitru > > Cheers, > Ihar > _______________________________________________ > dev mailing list > d...@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
