[calico-node] Enable CGO builds for ppc64le#11707
[calico-node] Enable CGO builds for ppc64le#11707kishen-v wants to merge 2 commits intoprojectcalico:masterfrom
Conversation
felix/bpf/maps/syscall.go
Outdated
| defer C.free(cV) | ||
|
|
||
| C.bpf_maps_attr_setup_map_elem(bpfAttr, C.uint(mapFD), cK, cV, C.ulonglong(flags)) | ||
| C.bpf_maps_attr_setup_map_elem(bpfAttr, C.uint(mapFD), cK, cV, C.__u64(flags)) |
There was a problem hiding this comment.
On ppc64le , __u64 is defined as unsigned long, whereas on amd64 it is unsigned long long. Casting to C.ulonglong causes a CGO type mismatch on ppc64le. Since the function signature already expects __u64, the value is kept as-is from the definition.
There was a problem hiding this comment.
Previously, I used -D__SANE_USERSPACE_TYPES__ to get int-ll64.h included for ppc64le builds. I’m not sure whether this is preferable to using __u64, or if it’s the right way to work around the build issue. Just sharing for contexts.
There was a problem hiding this comment.
Hi @hjiawei, I've adopted the usage of -D__SANE_USERSPACE_TYPES__ in Makefiles, wherever necessary to have a minimal diff in *.go files.
Thanks!
|
Do we need the ppc64le CFLAGS definitions in Lines 50 to 54 in 060acba --- a/felix/bpf-gpl/Makefile
+++ b/felix/bpf-gpl/Makefile
@@ -51,6 +51,8 @@ ifeq ($(findstring x86_64,$(TRIPLET)),x86_64)
CFLAGS += -D__TARGET_ARCH_x86 -D__x86_64__
else ifeq ($(findstring aarch64,$(TRIPLET)),aarch64)
CFLAGS += -D__TARGET_ARCH_arm64
+else ifeq ($(findstring ppc64le,$(TRIPLET)),ppc64le)
+ CFLAGS += -D__TARGET_ARCH_powerpc -D__SANE_USERSPACE_TYPES__
endif
CC := clang
LD := llc |
There was a problem hiding this comment.
Pull request overview
Enable CGO builds for the ppc64le architecture so Calico can link against real libbpf (avoiding the BPF syscall stub panic on PowerPC nodes).
Changes:
- Enable CGO for
ppc64leinnodeandfelixbuild Makefiles (including adding__SANE_USERSPACE_TYPES__for PowerPC). - Add missing
ppc64leCGO LDFLAGS for libbpf linkage in Felix. - Update the ppc64le node image Dockerfile to a glibc/UBI-based build flow consistent with other arches.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| node/Makefile | Enables CGO for ppc64le and sets PowerPC-specific CGO CFLAGS to support real libbpf usage. |
| node/Dockerfile.ppc64le | Moves ppc64le node image to the UBI/AlmaLinux-based build pattern needed for CGO-linked binaries. |
| felix/bpf/libbpf/libbpf.go | Adds ppc64le-specific CGO LDFLAGS to link against the ppc64le libbpf build output. |
| felix/bpf-gpl/Makefile | Adds PowerPC target defines for BPF program compilation on ppc64le. |
| felix/Makefile | Enables CGO for ppc64le and adds PowerPC-specific CGO CFLAGS consistent with the node build. |
| ARG BIRD_IMAGE=calico/bird:latest | ||
|
|
||
| FROM ${BIRD_IMAGE} AS bird | ||
|
|
||
| FROM calico/bpftool:v7.4.0 AS bpftool | ||
| FROM ${BIRD_IMAGE} AS bird |
There was a problem hiding this comment.
This Dockerfile hard-codes the bpftool image (calico/bpftool:v7.4.0) instead of using the repo-wide pinned BPFTOOL_IMAGE value (see metadata.mk and how node/Dockerfile.amd64 uses ARG BPFTOOL_IMAGE + FROM ${BPFTOOL_IMAGE}). This can cause version skew between architectures and makes it harder to bump bpftool consistently. Consider switching this file to the same BPFTOOL_IMAGE build-arg pattern (or at least updating the hard-coded tag to match the current pin).
| # Update base packages to pick up security updates. Must do this before adding the AlmaLinux repo. | ||
| RUN microdnf upgrade -y | ||
|
|
There was a problem hiding this comment.
RUN microdnf upgrade -y is inconsistent with the other node Dockerfiles (e.g. node/Dockerfile.arm64 / node/Dockerfile.amd64 use RUN microdnf upgrade without -y). It’d be better to align this for consistency unless -y is required for ppc64le specifically.
| RUN curl -sfL https://ftp.debian.org/debian/pool/main/r/runit/runit_${RUNIT_VER}.orig.tar.gz | tar xz -C /root && \ | ||
| cd /root/admin/runit-${RUNIT_VER} && \ | ||
| patch -p1 < /svlogd_use_0644_permission_instead_of_0744.patch && \ | ||
| package/compile |
There was a problem hiding this comment.
The almalinux build stage downloads and executes runit source via curl from https://ftp.debian.org/.../runit_${RUNIT_VER}.orig.tar.gz without any integrity verification. If an attacker can tamper with that distribution path (e.g., compromise the mirror/DNS/TLS), they could inject malicious code into the built image that then runs with root privileges at runtime. To reduce this supply-chain risk, ensure the downloaded archive is verified using a pinned checksum or GPG signature, or obtain it via a package manager that performs signature verification.
|
/sem-approve |
|
Re-tested with the latest changes, the cross-built image works as intended: |
build: fix cross-compilation for felix on ppc64le. Co-authored-by: Pedro Coutinho <coutinhop@users.noreply.github.com>
|
Hey @coutinhop and @hjiawei, |
|
/sem-approve |
Description
This PR aims at enabling CGO builds for the ppc64le architecture. Post merge, it would remediate the issue tied to the "BPF syscall stub" panic observed on PowerPC nodes during startup. The changes are based on
arm64related CGO enablement.Previously, ppc64le builds relied on CGO_ENABLED=0, which forced the usage of unimplemented BPF stubs. This had caused panics when the startup logic unconditionally invoked these BPF functions.
This PR contains the following changes:
felix/bpf/libbpf/libbpf.goFixing a C type alignment (__u64 vs ulonglong) that prevented compilation on PowerPC.Please let me know if any supporting changes are needed in this PR. Thanks!
Related issues/PRs: #11703
Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*label.docs-pr-required: This change requires a change to the documentation that has not been completed yet.docs-completed: This change has all necessary documentation completed.docs-not-required: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*label.release-note-required: This PR has user-facing changes. Most PRs should have this label.release-note-not-required: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.