p4c icon indicating copy to clipboard operation
p4c copied to clipboard

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

Open jafingerhut opened this issue 10 months ago • 2 comments

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: https://github.com/p4lang/p4c/commit/f44f4459b71b78b752977cd952b266fcb1e77943

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 avatar Feb 20 '25 03:02 jafingerhut

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

jafingerhut avatar Feb 20 '25 03:02 jafingerhut

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

fruffy avatar Feb 20 '25 19:02 fruffy

Closing this issue, since it was really only intended as a quick notification to recent p4c contributors about this change in dependency.

jafingerhut avatar Apr 15 '25 16:04 jafingerhut