Skip to content

Conversation

giuseppe
Copy link
Member

@giuseppe giuseppe commented Sep 1, 2025

systemd ignores errors with BPFProgram so make sure the ebpf program is installed when requested.

Alternative to #1854

Summary by Sourcery

Validate that systemd has installed the requested eBPF device filter on the cgroup by querying attached programs

Enhancements:

  • Add libcrun_ebpf_query_cgroup_progs to list eBPF programs attached to a cgroup
  • Introduce verify_ebpf_device_filter_installed to error if no eBPF filter is found on the cgroup
  • Invoke eBPF filter verification after creating the systemd cgroup scope when bpf_dev_set is requested

Copy link

sourcery-ai bot commented Sep 1, 2025

Reviewer's Guide

The PR adds a mechanism to query attached eBPF programs on a cgroup and integrates a verification step into the systemd cgroup entry path, causing an early failure if a requested eBPF device filter isn’t actually installed.

Sequence diagram for systemd cgroup entry with eBPF device filter verification

sequenceDiagram
    participant Caller
    participant libcrun_cgroup_enter_systemd
    participant verify_ebpf_device_filter_installed
    participant libcrun_ebpf_query_cgroup_progs
    participant cgroup
    Caller->>libcrun_cgroup_enter_systemd: Enter systemd cgroup
    libcrun_cgroup_enter_systemd->>verify_ebpf_device_filter_installed: If bpf_dev_set, verify filter
    verify_ebpf_device_filter_installed->>libcrun_ebpf_query_cgroup_progs: Query attached eBPF programs
    libcrun_ebpf_query_cgroup_progs->>cgroup: Open cgroup path and read programs
    cgroup-->>libcrun_ebpf_query_cgroup_progs: Return program list
    libcrun_ebpf_query_cgroup_progs-->>verify_ebpf_device_filter_installed: Return program count
    verify_ebpf_device_filter_installed-->>libcrun_cgroup_enter_systemd: Return success or error
    libcrun_cgroup_enter_systemd-->>Caller: Return success or error
Loading

Class diagram for new and updated eBPF-related functions

classDiagram
    class libcrun_ebpf_query_cgroup_progs {
        +int libcrun_ebpf_query_cgroup_progs(const char *cgroup_path, uint32_t **progs_out, size_t *n_progs_out, libcrun_error_t *err)
    }
    class verify_ebpf_device_filter_installed {
        +int verify_ebpf_device_filter_installed(const char *cgroup_path, libcrun_error_t *err)
    }
    class libcrun_ebpf_read_program {
        +int libcrun_ebpf_read_program(struct bpf_program **program_ret, const char *path, libcrun_error_t *err)
    }
    class libcrun_ebpf_cmp_programs {
        +bool libcrun_ebpf_cmp_programs(struct bpf_program *program1, struct bpf_program *program2)
    }
    libcrun_ebpf_query_cgroup_progs <|-- verify_ebpf_device_filter_installed
Loading

Class diagram for updated systemd cgroup entry logic

classDiagram
    class libcrun_cgroup_enter_systemd {
        +int libcrun_cgroup_enter_systemd(struct libcrun_cgroup_args *args, ...)
        +calls verify_ebpf_device_filter_installed if bpf_dev_set
    }
    class verify_ebpf_device_filter_installed {
        +int verify_ebpf_device_filter_installed(const char *cgroup_path, libcrun_error_t *err)
    }
    libcrun_cgroup_enter_systemd --> verify_ebpf_device_filter_installed
Loading

File-Level Changes

Change Details Files
Introduce a function to list eBPF programs on a cgroup
  • Added libcrun_ebpf_query_cgroup_progs with HAVE_EBPF guard
  • Opened cgroup path and delegated to read_all_progs
  • Returned an unsupported error when eBPF isn’t enabled
src/libcrun/ebpf.c
src/libcrun/ebpf.h
Create helper to verify that systemd installed the eBPF filter
  • Built full cgroup path
  • Queried the cgroup for attached programs
  • Errored if no programs were found
src/libcrun/cgroup-systemd.c
Invoke verification in systemd cgroup entry path
  • Check out->bpf_dev_set flag
  • Remove systemd scope suffix from path
  • Call verify_ebpf_device_filter_installed and fail early on error
src/libcrun/cgroup-systemd.c

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@giuseppe
Copy link
Member Author

giuseppe commented Sep 1, 2025

@sohankunkerkar @kolyshkin PTAL

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@giuseppe giuseppe force-pushed the add-check-for-ebpf-load branch from 50e58d1 to fefe6e1 Compare September 1, 2025 13:09
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

1 similar comment
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@giuseppe giuseppe marked this pull request as draft September 1, 2025 13:21
systemd ignores errors with BPFProgram so make sure the ebpf program
is installed when requested.

Alternative to containers#1854

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe giuseppe force-pushed the add-check-for-ebpf-load branch from fefe6e1 to a111995 Compare September 1, 2025 13:30
@giuseppe giuseppe marked this pull request as ready for review September 1, 2025 13:30
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • Extract the logic for stripping the systemd scope suffix from the cgroup path into a small helper function to improve readability and reduce duplication.
  • In libcrun_ebpf_query_cgroup_progs the HAVE_EBPF disabled path returns an error instead of an empty list; consider returning zero programs (no error) or documenting that callers must handle this specific unsupported‐feature error.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract the logic for stripping the systemd scope suffix from the cgroup path into a small helper function to improve readability and reduce duplication.
- In libcrun_ebpf_query_cgroup_progs the HAVE_EBPF disabled path returns an error instead of an empty list; consider returning zero programs (no error) or documenting that callers must handle this specific unsupported‐feature error.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Copy link

TMT tests failed. @containers/packit-build please check.

Copy link
Member

@sohankunkerkar sohankunkerkar left a comment

Choose a reason for hiding this comment

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

Yeah, this looks much better.
LGTM

@giuseppe
Copy link
Member Author

giuseppe commented Sep 2, 2025

given the reason in systemd/systemd#38732, this is probably the best we can do.

@kolyshkin what do you think?

Copy link
Collaborator

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

@giuseppe giuseppe merged commit 14e99b7 into containers:main Sep 2, 2025
63 of 64 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants