Ilya Maximets <[email protected]> writes: > On 2/7/25 15:45, Aaron Conole wrote: >> When writing tests it can be very difficult to understand what >> the test is doing without properly documenting the test. Added >> to that difficulty is when a stream of bytes gets posted with >> the patch as a packet definition. We then need to write out >> detailed descriptions of the packets, and tend to get packet >> strings that break line length parsers (leading to errors applying >> patches. >> >> With this change, we can write out clear descriptions of packets >> that make use of Scapy to generate the actual bytes strings. >> This should serve as accurate documentation, while also trimming >> the amount of bytes a developer needs to write to represent a >> packet. >> >> Signed-off-by: Aaron Conole <[email protected]> >> --- >> NOTE: I'm submitting it here as RFC, because I'm a bit nervous >> with the eval() call I've baked into the parser routine. >> Probably it's okay because users just trust the test suite >> anyway. >> > > Hi, Aaron. Thanks for the patch, but I'm not sure if we should do that. > A few things about scapy: > > 1. The distribution packages have a very large amount of dependencies, > so it's not a very desirable thing to install, unless installed > from pip. And installing from pip is another can of worms.
This is a good point. > 2. Importing scapy is generally slow, unless we cherry-pick small > modules to import, but then there might not be a point in using > it at all. > > We have an 'ovs-ofctl compose-packet' that can generate almost any type of > a packet that OVS can parse. A few things that it can't generate include: > > 1. Intentionally crafted malformed packets that we use for regression > testing. Scapy can't really help more than our existing tools with > that. > > 2. Fragmented packets. Not sure if scapy can help here, but it feels > like we should be extending the compose-packet to support fragments > instead. That makes sense. > 3. IGMP. The only potential use case for scapy as OpenFlow doesn't > support IGMP fields. But might make sense to just add a special > support for this to the compose-packet. > > One other benefit of using compose-packet is also that we're testing > our own packet parse/compose code that doesn't get a lot of testing > otherwise. > > Also, OVN has a scapy service utility already implemented, so we may > port that instead, if absolutely necessary. (OVN needs scapy, because > it has to generate a lot of different packet types for various protocols > that do not make sense for OVS.) Okay - it's probably worth looking into adding the support to compose packet. BTW, this was triggered because a recent patch from Paolo ended up being flagged malformed (patchwork issue, IIUC). And then when trying to read the packets (and having done the read/write work before), I realized there are certainly better ways that we could be expressing this. Maybe scapy isn't the ideal one - but probably we could do something with compose-packet (or other facility, too). > Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
