-
Notifications
You must be signed in to change notification settings - Fork 456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Modify CI tests to use bf-pktpy module, with no scapy, except for EBPF backend tests #5145
base: main
Are you sure you want to change the base?
Modify CI tests to use bf-pktpy module, with no scapy, except for EBPF backend tests #5145
Conversation
Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
Also add post-test 'pip --verbose list' command to confirm whether scapy was ever installed. Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
Since originally writing this comment, I have made the ptf-ebpf tests by installing scapy, but only for the ptf-ebpf tests. I have verified that scapy is not installed for any of the other tests, only bf-pktpy. @fruffy I am not sure whether there would be any interest in checking in changes similar to this, but basically the all-tests-pass-except the ptf-ebpf test demonstrates that if you do not have scapy installed on your system at all, but you do have the bf-pktpy module installed (which is licensed Apache-2.0), it implements enough functionality that all tests work, both those that do not use ptf at all, and those that do (except the ptf-ebpf tests). The reason that ptf-ebpf tests fail here is that they import the ptf module, and they import and use scapy directly, too. Some of those ptf-ebpf tests using scapy appear to me unlikely to work with bf-pktpy as an attempted replacement, without probably-significant enhancements to bf-pktpy. I have not attempted to make such enhancements to bf-pktpy, nor do I plan to. The other passing tests that do use ptf typically only use a handful of functions in ptf, and I believe that only the testgen-p4c-bmv2-ptf tests import the module
All packets sent in, or verified as output packets, by testgen-generated PTF tests give the contents of the packets as raw byte strings, so there is no use of any Scapy classes like Ether, IP, IPv6, TCP, UDP, etc. p4testgen is written to be generic to whatever the parser definition is in the P4 program it is generating tests for, so it would be more work to try to use those features of scapy than not to. |
One possible reason to consider committing changes like these, is that they would be likely to catch any changes someone tried to make that added dependencies on scapy -- such tests would fail, unless they were part of the ebpf tests. |
Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
... installed after tests complete. Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
This replacement allows the resulting test program to be licensed under Apache 2.0 license. Signed-off-by: Andy Fingerhut <[email protected]>
…-code-in-p4tc-tests
Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
…-with-bf-pktpy-and-no-scapy
This gives the option to users to use bf-pktpy instead of scapy. Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
… ptf branch Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
because in the ptf branch we are testing now, these packages are installed when one does `pip install ptf`. Signed-off-by: Andy Fingerhut <[email protected]>
Signed-off-by: Andy Fingerhut <[email protected]>
All prerequisite PRs have been approved and merged, so this PR is ready for review now. All changes to requirements.txt to point at their final "official" commit SHAs are in this PR now.
With these changes, 1 out of the 7 remaining test Python programs that uses Scapy now no longer uses Scapy, leaving only 6 that I believe would take significantly more work to update, and I don't have any plans to change them any time soon. A separate PR proposes licensing those remaining 6 EBPF back end Python test programs as license GPL-2.0-only.
Also with these changes, only the EBPF CI test uses the Scapy library. The rest of the CI tests DO NOT install scapy at all, and they pass. This is good for ensuring that no one accidentally slips in new code that depends on Scapy in the future.
The following prerequisites have already been merged: