Skip to content
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

Warning for p4c developers: as of 2025-Feb-19, latest p4c requires latest version of ptf to successfully run tests #5135

Open
jafingerhut opened this issue Feb 20, 2025 · 2 comments

Comments

@jafingerhut
Copy link
Contributor

jafingerhut commented Feb 20, 2025

Executive summary:

If you update to the latest p4c and want to run p4c tests, you need to install the latest version of ptf, or else those tests will fail.

How to install latest version of ptf:

You can use pip to install arbitrary commit SHAs of Python packages in git repos, using syntax like this: https://github.com/p4lang/p4c/blob/main/requirements.txt#L6

except if you do it from the command line instead of a requirements.txt file, you need to omit the white space, like this:

pip install ptf@git+https://github.com/p4lang/ptf@d016cdfe99f2d609cc9c7fd7f8c414b56d5b3c5c

You can also say you want the current latest version on the branch main by using that branch name instead of a commit SHA:

pip install ptf@git+https://github.com/p4lang/ptf@main

Symptom:

If you run recent p4c tests with an older version of ptf, you will see errors like the one below, where it cannot import names from module 'ptf.pcap_writer':

Traceback (most recent call last):
  File "/home/cdodd/repos/p4org-p4c/backends/bmv2/run-bmv2-test.py", line 139, in <module>
    from bmv2stf import Options, RunBMV2
  File "/localhome/local-cdodd/repos/p4org-p4c/backends/bmv2/bmv2stf.py", line 33, in <module>
    from ptf.pcap_writer import LINKTYPE_ETHERNET, PcapWriter, rdpcap
ImportError: cannot import name 'LINKTYPE_ETHERNET' from 'ptf.pcap_writer' (/home/cdodd/.local/lib/python3.10/site-packages/ptf/pcap_writer.py)

Why are these errors happening?

This commit was made earlier today on 2025-Feb-19: f44f445

Reason for these changes:

For reasons described in this proposed PR: https://github.com/p4lang/p4c/pull/5110/files#diff-dee2da6f36744b5ea438dd91ff033302a1cbc0fadc89e983c510c6181e86f429 I am trying to make the licenses of all individual source files in p4lang repositories correct.

My belief is that the only choice of license that is not legally questionable for Python source files that import the Scapy library, whether directly, or indirectly, is GPL v2, the same as Scapy.

All such Python code that we have in the p4lang repos are test programs. Even so, Fabian Ruffy suggested trying to remove the import of the Scapy module from as many Python source files as can be done with reasonable time and effort.

The BMv2 and ebpf back ends have test programs that import scapy for only one reason: to use some functions in scapy that read and write pcap files.

There are many libraries for reading and writing pcap files that are released under non-copyleft licenses. One of them was in ptf, or rather, was partly implemented in ptf. It did writing for a different pcap file variant, but did not implement reading pcap files at all. I have written an Apache-2.0 licensed enhancement to ptf that reads and writes pcap files, with at least the functionality required by our bmv2 and ebpf back end tests. It has been tested to work for all p4c tests.

Today we merged in that enhancement to ptf, and then we merged in changes to p4c Python test code so that it uses the new ptf implementation of pcap file reading and writing.

Unfortunately this has the side effect of causing errors for others if you have the latest p4c, but not the latest ptf. Apologies for the trouble.

@jafingerhut
Copy link
Contributor Author

Pinging everyone who has had a PR merged in the last several months so hopefully they will learn of this more quickly.
@chridodd @fruffy @kfcripps @asl @vlstill @komaljai @AdarshRawat1 @qobilidop @vbnogueira @maheswari-s @katre @pkotikal @jhavrane @hanw @snapdgn @Sosutha @grg @zzmic @matthewtlam @rupesh-chiluka-marvell

@fruffy
Copy link
Collaborator

fruffy commented Feb 20, 2025

Ideally, this should be an assertion in the tests frameworks. Anywhere where we import ptf we should check that the version is recent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants