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

feat: Add vmlinux for kernel 5.15 for arm & amd #26

Merged
merged 10 commits into from
Aug 2, 2023
Merged

Conversation

MikeGoldsmith
Copy link
Contributor

@MikeGoldsmith MikeGoldsmith commented Jul 17, 2023

Which problem is this PR solving?

Adds complete vmlinux.h files for both arm and amd platforms.

Short description of changes

  • add vmlinux-arm64.h and vmlinux-x86_64.h header files generated from linux
  • update vmlinux.h to use appropriate header file based on architecture
  • add Makefile targets specifically for building and running on Mac

How to verify this has the expected result

Make sure your HONEYCOMB_API_KEY env var is set

# For Kubernetes in EKS or other BTF-enabled Kernel:
make docker-build
make apply-ebpf-agent

# For Kubernetes on Docker Desktop for Mac:
make mac-docker-build
make apply-ebpf-agent
  • confirmed building with make docker-build on arm64 laptop runs in EKS with Amazon Linux 2 OS, 5.10.184-175.731.amzn2.aarch64
  • confirmed building with make mac-docker-build on arm64 laptop runs in Kubernetes for Docker Desktop, Host kernel version: 5.15.49
  • confirmed building with make mac-docker-build on x86_64 laptop runs in Kubernetes for Docker Desktop, Host kernel version: 5.15.49

@MikeGoldsmith MikeGoldsmith self-assigned this Jul 17, 2023
@MikeGoldsmith MikeGoldsmith added the type: enhancement New feature or request label Jul 17, 2023
@MikeGoldsmith MikeGoldsmith changed the title Add vmlinux for kernel 5.15 for arm & amd feat: Add vmlinux for kernel 5.15 for arm & amd Jul 17, 2023
@JamieDanielson
Copy link
Contributor

JamieDanielson commented Jul 31, 2023

The original vmlinux.h file is still in there because removing it causes errors with the bpf_helpers.h file, which makes sense with this specific note in there:

/*
 * Note that bpf programs need to include either
 * vmlinux.h (auto-generated from BTF) or linux/types.h
 * in advance since bpf_helper_defs.h uses such types
 * as __u64.
 */

EDIT: We now just have the vmlinux.h file including the arch-specific vmlinux files.

@MikeGoldsmith MikeGoldsmith marked this pull request as ready for review August 2, 2023 15:03
@MikeGoldsmith MikeGoldsmith requested a review from a team August 2, 2023 15:03
@vreynolds vreynolds added this to the Trial with HNY Dogfood milestone Aug 2, 2023
@vreynolds
Copy link
Contributor

getting CrashLoopBackOff on the agent pod with this error:

loading objects: field KprobeTcpConnect: program kprobe__tcp_connect: apply CO-RE relocations: load kernel spec: no BTF found for kernel version 5.15.49-linuxkit-pr: not supported

@JamieDanielson
Copy link
Contributor

JamieDanielson commented Aug 2, 2023

getting CrashLoopBackOff on the agent pod with this error:

loading objects: field KprobeTcpConnect: program kprobe__tcp_connect: apply CO-RE relocations: load kernel spec: no BTF found for kernel version 5.15.49-linuxkit-pr: not supported

Hm, same here on local arm 🤔 Will look into this a bit more.

@JamieDanielson
Copy link
Contributor

JamieDanielson commented Aug 2, 2023

@vreynolds can you try again when you get a moment? I've added a flag to the Makefile -DBPF_NO_PRESERVE_ACCESS_INDEX which avoids CO-RE relocation.

Copy link
Contributor

@vreynolds vreynolds left a comment

Choose a reason for hiding this comment

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

It works! 🎉

@JamieDanielson are there any gotchas with including the BPF_NO_PRESERVE_ACCESS_INDEX?

@JamieDanielson
Copy link
Contributor

JamieDanielson commented Aug 2, 2023

It works! 🎉

@JamieDanielson are there any gotchas with including the BPF_NO_PRESERVE_ACCESS_INDEX?

As I understand it, this flag disables CO-RE relocation entirely with a flag, which means we can run it on our Mac or similar devices that do not have BTF or BPF CO-RE support. Using the generated vmlinux.h file means automatically generating BPF CO-RE relocations by default. There is a section near the top of the vmlinux files that enables the preserve_access_index unless this environment variable is added:

#ifndef BPF_NO_PRESERVE_ACCESS_INDEX
#pragma clang attribute push (__attribute__((preserve_access_index)), apply_to = record)
#endif

From this post:

All the types in vmlinux.h come with extra __attribute__((preserve_access_index)) applied, which makes Clang generate BPF CO-RE relocations, allowing libbpf to adapt your BPF code to the specific memory layout of the host kernel, even if it differs from the one that vmlinux.h was originally generated from. This is a crucial aspect of building portable pre-compiled BPF application that doesn't require entire Clang/LLVM toolchain to be deployed alongside it to the target system.

So if/when we decide to use BPF CO-RE, we'll want to remove this flag and/or add extra conditional logic checking for existence of BTF. And until BTF is available on our local workstations, we'll have to build on BTF-enabled devices to use the CO-RE macros (probably colima or EC2 instance).

An alternative is to add the conditional logic now 🤔

Copy link
Contributor

@vreynolds vreynolds left a comment

Choose a reason for hiding this comment

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

make it work ™️

@JamieDanielson JamieDanielson merged commit 4767f9a into main Aug 2, 2023
8 checks passed
@JamieDanielson JamieDanielson deleted the mike/vmlinux branch August 2, 2023 21:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Figure out vmlinux.h stuff
3 participants