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.

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

[1.15] reproducible bpf unit tests, tcx downgrade, stable testdata #31663

Merged
merged 7 commits into from Apr 2, 2024

Conversation

ti-mo
Copy link
Contributor

@ti-mo ti-mo commented Mar 28, 2024

@ti-mo ti-mo added kind/backports This PR provides functionality previously merged into master. backport/1.15 This PR represents a backport for Cilium 1.15.x of a PR that was merged to main. labels Mar 28, 2024
@ti-mo ti-mo requested a review from rgo3 March 28, 2024 15:52
@ti-mo ti-mo force-pushed the pr/v1.15-backport-2024-03-28-02-32 branch from eb7eac8 to 38e782c Compare March 28, 2024 16:04
@ti-mo ti-mo changed the title v1.15 Backports 2024-03-28 [1.15] reproducible bpf unit tests, tcx downgrade, stable testdata Mar 28, 2024
@ti-mo
Copy link
Contributor Author

ti-mo commented Mar 28, 2024

/test-backport-1.15

@auto-committer auto-committer bot temporarily deployed to release-base-images March 28, 2024 16:17 Inactive
@ti-mo ti-mo marked this pull request as ready for review March 28, 2024 16:17
@ti-mo ti-mo requested review from a team as code owners March 28, 2024 16:17
@ti-mo
Copy link
Contributor Author

ti-mo commented Mar 28, 2024

/test-backport-1.15

@julianwiedmann julianwiedmann added the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Apr 1, 2024
rgo3 and others added 6 commits April 2, 2024 08:26
[ upstream commit 1409a37 ]

replaceNetworkDatapath() is only called from one place and adds an
additional loop over the encryption devices. This commit removes the
function and calls replaceDatapath() from reinitializeIPSec() directly.

There are no functional changes.

Signed-off-by: Robin Gögge <r.goegge@isovalent.com>
Signed-off-by: Timo Beckers <timo@isovalent.com>
[ upstream commit 0b9b390 ]

Currently, we develop the datapath against multiple versions of LLVM: various local
(host) toolchains during development, CI uses an LLVM version installed by a GH workflow,
and the agent uses the cilium-builder container image.

This PR changes the BPF unit tests to use cilium-builder by default using the
`run_bpf_tests` make target in the root Makefile of the project.

Small overview of the changes:
- run bpf unit tests CI using the cilium-builder image so we use the same LLVM
  toolchain across all tests
- set -j<numcpu> on the root Docker invocation to build .o's in parallel, as
  building the tests was becoming rather slow
- moved `test/bpf_tests/` to `bpf/tests/bpftest` to keep the BPF test runner closer
  to the .c test files it's used with
- removed the layer of indirection through `test/Makefile`; the root Makefile now
  calls `bpf/tests/Makefile` directly
- added a `run` target to `bpf/tests/Makefile` to make it easier to invoke the tests
  using the host Go toolchain without rebuilding the world. sudo is now used
  automatically for 'go test' if `make` is invoked as a non-root user.
- cleaned up output generated by bpf/tests/Makefile

Signed-off-by: Timo Beckers <timo@isovalent.com>
[ upstream commit e2d90da ]

The arguments to the replaceDatapath functions are already quite numerous
and make the function signature hard to read. In preparation for future
commits, this patch aggregates almost all arguments to the function into
one option parameter.

Signed-off-by: Robin Gögge <r.goegge@isovalent.com>
Signed-off-by: Timo Beckers <timo@isovalent.com>
[ upstream commit f2d804b ]

A follow-up commit will introduce attaching TC programs using tcx. Those
attachments cannot be overridden using netlink. If an older version of
Cilium wants to replace an TC program on a managed interface, it'll need to
remove the tcx attachment first.

This commit teaches the agent to remove leftover tcx link objects from previous
installs, before reattaching it using netlink. Note that this transition is
never seamless, since some time passes between deleting the link and attaching
the new program using netlink. However, as explained in 7a8e3c8
("loader: clean up XDP bpf_links created by newer Cilium versions"), this
downgrade path should rarely happen.

Signed-off-by: Robin Gögge <r.goegge@isovalent.com>
Co-authored-by: Timo Beckers <timo@isovalent.com>
Signed-off-by: Timo Beckers <timo@isovalent.com>
[ upstream commit 5c35dc3 ]

Centralize the declaration so we can assume it's present in other Makefiles
importing Makefile.defs.

Signed-off-by: Timo Beckers <timo@isovalent.com>
[ upstream commit cd5bc4e ]

This patch should make testdata play a bit nicer with backports, since
including headers like node_config.h, ep_config.h and maps.h cause potential
churn in the resulting BTF info.

Include a minimal subset of headers and reduce testdata code to what's
strictly necessary for the Go tests to run.

Signed-off-by: Timo Beckers <timo@isovalent.com>
@rgo3 rgo3 force-pushed the pr/v1.15-backport-2024-03-28-02-32 branch from 3c902fc to 1b98201 Compare April 2, 2024 06:33
@rgo3 rgo3 temporarily deployed to release-base-images April 2, 2024 06:33 — with GitHub Actions Inactive
@rgo3 rgo3 removed the dont-merge/needs-rebase This PR needs to be rebased because it has merge conflicts. label Apr 2, 2024
@rgo3 rgo3 force-pushed the pr/v1.15-backport-2024-03-28-02-32 branch from 1b98201 to c8ebf0b Compare April 2, 2024 10:51
@rgo3 rgo3 temporarily deployed to release-base-images April 2, 2024 10:51 — with GitHub Actions Inactive
Signed-off-by: Cilium Imagebot <noreply@cilium.io>
@rgo3 rgo3 temporarily deployed to release-base-images April 2, 2024 12:31 — with GitHub Actions Inactive
@rgo3
Copy link
Contributor

rgo3 commented Apr 2, 2024

/test-backport-1.15

@aanm aanm merged commit 0e5953e into v1.15 Apr 2, 2024
232 of 235 checks passed
@aanm aanm deleted the pr/v1.15-backport-2024-03-28-02-32 branch April 2, 2024 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.15 This PR represents a backport for Cilium 1.15.x of a PR that was merged to main. kind/backports This PR provides functionality previously merged into master.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants