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.

Sign up for GitHub

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

Jump to bottom

Update networking container to fix unused p4c flag & add new dependencies #395

Merged
merged 1 commit into from
Jul 27, 2023
Merged

Update networking container to fix unused p4c flag & add new dependencies #395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion build/networking/scripts/build_p4c.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ git submodule update --init --recursive
mkdir build && mkdir -p "${WORKING_DIR}"/p4c/install && cd build

cmake -DCMAKE_INSTALL_PREFIX="${WORKING_DIR}"/p4c/install \
-DENABLE_BMV2=OFF \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we removing this?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the reason for build failures in container build. Confirmed by the p4c team and needs to be removed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

latest P4 Compiler uses some code from bmv2 , so it shouldn't be disabled
Hence they asked to remove this in build script.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the reason for build failures in container build. Confirmed by the p4c team and needs to be removed

What were the build failures?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error Logs:
[ 87%] Building CXX object backends/tc/CMakeFiles/p4c-pna-p4tc.dir/midend.cpp.o
In file included from /root/P4C/backends/bmv2/common/helpers.h:22,
from /root/P4C/backends/bmv2/common/action.h:20,
from /root/P4C/backends/bmv2/psa_switch/psaSwitch.h:20,
from /root/P4C/backends/ebpf/psa/ebpfPsaGen.h:20,
from /root/P4C/backends/tc/backend.h:20,
from /root/P4C/backends/tc/ebpfCodeGen.h:20,
from /root/P4C/backends/tc/ebpfCodeGen.cpp:17:
/root/P4C/backends/bmv2/common/expression.h:125:30: error: 'IntMod' in namespace 'IR' does not name a type
125 | void postorder(const IR::IntMod expression) override;
| ^~~~~~
/root/P4C/backends/bmv2/common/expression.h:125:10: error: 'void BMV2::ExpressionConverter::postorder(const int)' marked 'override', but does not override
125 | void postorder(const IR::IntMod *expression) override;
| ^~~~~~~~~
In file included from /root/P4C/backends/bmv2/common/helpers.h:22,
from /root/P4C/backends/bmv2/common/action.h:20,
from /root/P4C/backends/bmv2/psa_switch/psaSwitch.h:20,
from /root/P4C/backends/ebpf/psa/ebpfPsaGen.h:20,
from /root/P4C/backends/tc/backend.h:20,
from /root/P4C/backends/tc/backend.cpp:17:
/root/P4C/backends/bmv2/common/expression.h:125:30: error: 'IntMod' in namespace 'IR' does not name a type
125 | void postorder(const IR::IntMod expression) override;
| ^~~~~~
/root/P4C/backends/bmv2/common/expression.h:125:10: error: 'void BMV2::ExpressionConverter::postorder(const int)' marked 'override', but does not override
125 | void postorder(const IR::IntMod *expression) override;
| ^~~~~~~~~
[ 87%] Building CXX object backends/tc/CMakeFiles/p4c-pna-p4tc.dir/pnaProgramStructure.cpp.o
[ 87%] Building CXX object backends/tc/CMakeFiles/p4c-pna-p4tc.dir/tc.cpp.o
[ 88%] Building CXX object backends/tc/CMakeFiles/p4c-pna-p4tc.dir/tcAnnotations.cpp.o
[ 88%] Building CXX object backends/tc/CMakeFiles/p4c-pna-p4tc.dir/version.cpp.o
[ 89%] Building CXX object backends/tc/CMakeFiles/p4c-pna-p4tc.dir//ebpf/ebpfBackend.cpp.o
[ 89%] Building CXX object backends/tc/CMakeFiles/p4c-pna-p4tc.dir//ebpf/ebpfProgram.cpp.o
[ 89%] Building CXX object backends/tc/CMakeFiles/p4c-pna-p4tc.dir//ebpf/ebpfTable.cpp.o
make[2]: *** [backends/tc/CMakeFiles/p4c-pna-p4tc.dir/build.make:95: backends/tc/CMakeFiles/p4c-pna-p4tc.dir/ebpfCodeGen.cpp.o] Error 1
make[2]: *** Waiting for unfinished jobs....
make[2]: *** [backends/tc/CMakeFiles/p4c-pna-p4tc.dir/build.make:82: backends/tc/CMakeFiles/p4c-pna-p4tc.dir/backend.cpp.o] Error 1
In file included from /root/P4C/backends/bmv2/common/backend.h:22,
from /root/P4C/backends/tc/pnaProgramStructure.h:20,
from /root/P4C/backends/tc/pnaProgramStructure.cpp:17:
/root/P4C/backends/bmv2/common/expression.h:125:30: error: 'IntMod' in namespace 'IR' does not name a type
125 | void postorder(const IR::IntMod expression) override;
| ^~~~~~
/root/P4C/backends/bmv2/common/expression.h:125:10: error: 'void BMV2::ExpressionConverter::postorder(const int)' marked 'override', but does not override
125 | void postorder(const IR::IntMod *expression) override;
| ^~~~~~~~~
In file included from /root/P4C/backends/bmv2/common/helpers.h:22,
from /root/P4C/backends/bmv2/common/action.h:20,
from /root/P4C/backends/bmv2/psa_switch/psaSwitch.h:20,
from /root/P4C/backends/ebpf/psa/ebpfPsaGen.h:20,
from /root/P4C/backends/tc/backend.h:20,
from /root/P4C/backends/tc/tc.cpp:17:
/root/P4C/backends/bmv2/common/expression.h:125:30: error: 'IntMod' in namespace 'IR' does not name a type
125 | void postorder(const IR::IntMod expression) override;
| ^~~~~~
/root/P4C/backends/bmv2/common/expression.h:125:10: error: 'void BMV2::ExpressionConverter::postorder(const int)' marked 'override', but does not override
125 | void postorder(const IR::IntMod *expression) override;
| ^~~~~~~~~
In file included from /root/P4C/backends/bmv2/common/helpers.h:22,
from /root/P4C/backends/bmv2/common/action.h:20,
from /root/P4C/backends/bmv2/psa_switch/psaSwitch.h:20,
from /root/P4C/backends/ebpf/ebpfBackend.cpp:19:
/root/P4C/backends/bmv2/common/expression.h:125:30: error: 'IntMod' in namespace 'IR' does not name a type
125 | void postorder(const IR::IntMod expression) override;
| ^~~~~~
/root/P4C/backends/bmv2/common/expression.h:125:10: error: 'void BMV2::ExpressionConverter::postorder(const int)' marked 'override', but does not override
125 | void postorder(const IR::IntMod *expression) override;
| ^~~~~~~~~
make[2]: *** [backends/tc/CMakeFiles/p4c-pna-p4tc.dir/build.make:134: backends/tc/CMakeFiles/p4c-pna-p4tc.dir/pnaProgramStructure.cpp.o] Error 1
[ 89%] Linking CXX executable p4c-dpdk
make[2]: *** [backends/tc/CMakeFiles/p4c-pna-p4tc.dir/build.make:186: backends/tc/CMakeFiles/p4c-pna-p4tc.dir//ebpf/ebpfBackend.cpp.o] Error 1
[ 89%] Built target p4c-dpdk
make[2]: *** [backends/tc/CMakeFiles/p4c-pna-p4tc.dir/build.make:147: backends/tc/CMakeFiles/p4c-pna-p4tc.dir/tc.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:1537: backends/tc/CMakeFiles/p4c-pna-p4tc.dir/all] Error 2
make: *** [Makefile:182: all] Error 2
The command '/bin/sh -c /root/scripts/distro_pkg_install.sh --install-dev-pkgs --scripts-dir=/root/scripts && /root/scripts/install_nr_modules.sh --workdir=/root && /root/scripts/post_build_cleanup.sh --workdir=/root --keep-source-code="${KEEP_SOURCE_CODE}" --deployment-image="${DEPLOYMENT_IMAGE}"' returned a non-zero code: 1

has context menu

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Address two build issues:

  • Missing 'netaddr' module. Updated install script to provide it.
  • Compile errors in P4C. The latest P4C compiler uses some code from bmv2 , so it shouldn't be disabled. Removed ENABLE_BMV2=OFF from the cmake command. Verified with compiler team that this is correct.

Copy link
Contributor

@ffoulkes ffoulkes Jul 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This provides a concise explanation of each problem and an explanation of the solution. In the case of the P4C compile errors, you've shared the information that solution was confirmed by the code's authors.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The critical one here is the change to the ENABLE_BMV2 parameter. The reason for adding the 'netaddr' module can reasonably be induced. The ENABLE_BMV2 is considerably more subtle. The most important reason for adding a comment is to answer the question, "Why?"

-DENABLE_EBPF=OFF \
-DENABLE_UBPF=OFF \
-DENABLE_GTESTS=OFF \
Expand Down
6 changes: 6 additions & 0 deletions build/networking/scripts/distro_pkg_install.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ fedora_install_build_pkgs() {

python -m pip install --upgrade pip
python -m pip install grpcio
python -m pip install netaddr
python -m pip install ovspy
python -m pip install protobuf==3.20.3
python -m pip install p4runtime
Expand Down Expand Up @@ -86,6 +87,7 @@ fedora_install_deployment_pkgs() {

python -m pip install --upgrade pip
python -m pip install grpcio
python -m pip install netaddr
python -m pip install ovspy
python -m pip install protobuf==3.20.3
python -m pip install p4runtime
Expand Down Expand Up @@ -119,6 +121,7 @@ fedora_install_default_pkgs() {

python -m pip install --upgrade pip
python -m pip install grpcio
python -m pip install netaddr
python -m pip install ovspy
python -m pip install protobuf==3.20.3
python -m pip install p4runtime
Expand Down Expand Up @@ -185,6 +188,7 @@ ubuntu_install_build_pkgs() {

python3 -m pip install --no-cache-dir --upgrade pip
python3 -m pip install --no-cache-dir grpcio
python3 -m pip install --no-cache-dir netaddr
python3 -m pip install --no-cache-dir ovspy \
protobuf=="${PROTOBUF_VER}" \
p4runtime \
Expand Down Expand Up @@ -221,6 +225,7 @@ ubuntu_install_deployment_pkgs() {

python3 -m pip install --no-cache-dir --upgrade pip
python3 -m pip install --no-cache-dir grpcio
python3 -m pip install --no-cache-dir netaddr
python3 -m pip install --no-cache-dir ovspy \
protobuf=="${PROTOBUF_VER}" \
p4runtime
Expand Down Expand Up @@ -258,6 +263,7 @@ ubuntu_install_default_pkgs() {

python3 -m pip install --no-cache-dir --upgrade pip
python3 -m pip install --no-cache-dir grpcio
python3 -m pip install --no-cache-dir netaddr
python3 -m pip install --no-cache-dir ovspy \
protobuf=="${PROTOBUF_VER}" \
p4runtime
Expand Down