p4lang / behavioral-model

The reference P4 software switch
Apache License 2.0
538 stars 327 forks source link

Known issues with using `pip install nnpy` instead of particular commit SHA of nnpy? #1212

Closed jafingerhut closed 1 year ago

jafingerhut commented 1 year ago

Right now install_deps.sh invokes ci/install-nnpy.py, which checks out a particular commit SHA of the Python nnpy library: https://github.com/p4lang/behavioral-model/blob/main/ci/install-nnpy.sh#L3-L6

The p4lang/p4c repo has some CI scripts that run PTF tests with BMv2 and some other p4c back ends, that appear to me to simply do pip3 install nnpy and install whatever version that gets. I do not know precisely which version that is, but I know the Python nnpy source code is different between the one installed by install-nnpy.sh vs. that installed by pip3 install nnpy as of 2023-Aug-31.

I have found that when installing the version of nnpy installed by install-nnpy.sh, the resulting system fails on all or almost all of the p4c bmv2 ptf tests, due to an exception thrown in the cffi Python package, after calling from ptf into nnpy into cffi. When I replace the install-nnpy.sh version of nnpy with pip3 install nnpy, a much larger number of those tests pass.

Is there any known reason to continue installing the commit SHA version of nnpy that install-nnpy.sh currently does?

rst0git commented 1 year ago

nnpy is deprecated and perhaps we should update the Python code to use pynng instead.

@antoninbas What do you think?

jafingerhut commented 1 year ago

This PR I have tested on freshly installed Ubuntu 20.04 system, using my script that installs many open source P4 dev tools, and with these changes many more of p4c's bmv2-ptf tests pass, whereas all of them fail without these changes: https://github.com/p4lang/behavioral-model/pull/1213