Generalize PcapWriter, and add rdpcap function

[jafingerhut]

No description provided.

[jafingerhut]

With these changes to PcapWriter, and the addition of rdpcap, if these are merged in, I can make small changes to 2 Python test source files in p4lang/p4c so that they no longer need to import the scapy module, and thus they can be licensed as Apache-2.0 instead of GPL-2.0-only:

• https://github.com/p4lang/p4c/blob/main/backends/bmv2/bmv2stf.py
• https://github.com/p4lang/p4c/blob/main/backends/ebpf/targets/target.py

Several other Python programs in the p4c repo import target.py, and can also then be license Apache-2.0, since they will no longer have any transitive import of scapy module.

Note: There are some existing uses of the current ptf PcapWriter class sprinkled throughout the p4lang repositories, thus I have been careful to make the new linktype parameter to its __init__ method optional, with its default value keeping the previous behavior. Only if the caller provides a new supported value for that parameter will they get new behavior.

The new rdpcap function here is brand new, with no existing callers, so no backwards compatibility concerns (yet :-)

[jafingerhut]

The CI tests are failing, waiting on this PR to be merged (or some other PR that is similarly effective): #212

[fruffy]

Should ideally be tested on P4C, too. Can you create the corresponding PR?

[jafingerhut]

Should ideally be tested on P4C, too. Can you create the corresponding PR?

There is no code in p4c using this implementation of PcapWriter.

There is code in p4c using a class named PcapWriter that is in the scapy library, in exactly two Python source files. That is a GPL v2 implementation of this functionality that I hope to replace soon. My plan was to first merge this change. Then create a PR that modifies those two source files in p4c that uses this implementation of PcapWriter instead of the one in scapy. That PR in p4c will test this.

[jafingerhut]

Or your comment means: "Please, before merging this PR, create the changes in p4c that use this implementation, and test it in p4c, before merging in these changes?"

I can do that. I think we are being a little bit too cautious, if that is the case :-)

[fruffy]

Or your comment means: "Please, before merging this PR, create the changes in p4c that use this implementation, and test it in p4c, before merging in these changes?"

I can do that. I think we are being a little bit too cautious, if that is the case :-)

Yes, you can now directly test your changes on p4c by updating the PTF refpoint in the PR: https://github.com/p4lang/p4c/pull/5127/files#diff-4d7c51b1efe9043e44439a949dfd92e5827321b34082903477fd04876edb7552R6

This way we do not merge code that breaks downstream dependencies or is functionally broken.

[jafingerhut]

This way we do not merge code that breaks downstream dependencies or is functionally broken.

Testing starting now on this p4c PR: p4lang/p4c#5132

[fruffy]

Can't comment on the code since it is pcap-specific. Trusting it since we have tested it extensively.

[fruffy]

Not a big fan of assertions sprinkled in the code like this. Should probably be an exception.

[fruffy]

There are more cases in the code below.

[fruffy]

Could benefit from type annotations for data and timestamp. Makes it easier to know what input is expected. I know these are commented but modern Python actually has great type annotations.

[jafingerhut]

Hmmm. Python 3.8, the default installed version on Ubuntu 20.04, does not like this type hint:

def foo(path: str | os.PathLike):

which I found as one recommended answer for the type hint of a parameter that can be passed as the first argument of open.

Python 3.10 and later seem fine with it.

If I don't find something better, I might just replace it with str, since that is the only type we currently ever pass to it in our code that I know of.

[fruffy]

Hmmm. Python 3.8, the default installed version on Ubuntu 20.04, does not like this type hint:

def foo(path: str | os.PathLike):

which I found as one recommended answer for the type hint of a parameter that can be passed as the first argument of open.

Python 3.10 and later seem fine with it.

If I don't find something better, I might just replace it with str, since that is the only type we currently ever pass to it in our code that I know of.

Anything before Python 3.10 requires Union: def foo(path: Union[str, os.PathLike])

[jafingerhut]

As one little extra quick test, I recorded and saved a pcap file with Wireshark, and if you pick the right variant of pcap from the 20 or so formats Wireshark supports, this PR's implementation of rdpcap can read it. This PR's implementation is not cover all of those formats, by a mile, but it only needs to be able to communicate with BMv2, plus whatever it was being used for before. I will go ahead and merge this, and update the corresponding p4c PR to point to the new commit SHA of the ptf repo.