Skip to content

feat: Use BPF iterator instead of procfs#336

Open
vadorovsky wants to merge 3 commits intofix-race-conditionfrom
bpf-iterator
Open

feat: Use BPF iterator instead of procfs#336
vadorovsky wants to merge 3 commits intofix-race-conditionfrom
bpf-iterator

Conversation

@vadorovsky
Copy link
Member

Replace the process tracker initialization process from procfs parsing to BPF iterators, which are less erronous and result in an easier implementation.

@vadorovsky vadorovsky changed the base branch from rust-2024 to cgroup-skb-core-read March 17, 2025 08:57
@vadorovsky vadorovsky force-pushed the cgroup-skb-core-read branch from 8dad79c to 87966a5 Compare March 17, 2025 09:49
@vadorovsky vadorovsky force-pushed the bpf-iterator branch 2 times, most recently from 4e8a25d to cdf890c Compare March 17, 2025 10:51
@vadorovsky vadorovsky changed the base branch from cgroup-skb-core-read to main March 24, 2025 07:12
@vadorovsky vadorovsky force-pushed the bpf-iterator branch 3 times, most recently from 4933373 to 949f7b1 Compare March 31, 2025 06:49
@vadorovsky vadorovsky marked this pull request as ready for review March 31, 2025 07:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR replaces the process tracker initialization from procfs parsing to a BPF iterator approach to improve reliability and simplify the implementation.

  • Introduces a new enum ContainerEngineKind in pulsar-core to represent container engines.
  • Removes the duplicate ContainerEngineKind from process-monitor and updates the process tree loading logic to use BPF iterator output.
  • Updates the initializer to use the new process tree loader and changes the aya dependency configuration in Cargo.toml.

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
crates/pulsar-core/src/event.rs Added ContainerEngineKind enum for container engine distinction.
crates/modules/process-monitor/src/lib.rs Updated import statements by removing duplicate enum definition.
crates/bpf-filtering/src/process_tree.rs Replaced procfs parsing with reading from a BPF iterator and updated error handling for incorrect output formats.
crates/bpf-filtering/src/initializer.rs Updated process tree initialization to use the new BPF iterator loader.
Cargo.toml Updated aya dependency to a specific git revision.
Files not reviewed (1)
  • crates/modules/process-monitor/probes.bpf.c: Language not supported

@vadorovsky vadorovsky force-pushed the bpf-iterator branch 3 times, most recently from 595d168 to ad0ed39 Compare May 18, 2025 02:43
@vadorovsky vadorovsky changed the base branch from main to clippy May 18, 2025 02:43
@vadorovsky vadorovsky changed the base branch from clippy to main May 18, 2025 15:57
@vadorovsky vadorovsky force-pushed the bpf-iterator branch 2 times, most recently from c1915f0 to 679c994 Compare May 18, 2025 16:13
@banditopazzo
Copy link
Member

On the start I always see

[2025-05-20T11:05:59Z WARN  bpf_filtering::initializer] process 0 not found while building map_interest
[2025-05-20T11:05:59Z WARN  bpf_filtering::initializer] process 0 not found while building map_interest

and sometimes

[2025-05-20T11:07:19Z ERROR file-system-monitor] Process not found in tracker 63980: process not found
[2025-05-20T11:07:19Z ERROR file-system-monitor] Process not found in tracker 63980: process not found

the goal of BPF iterators should be to solve the Process not found in tracker error

@vadorovsky
Copy link
Member Author

On the start I always see

[2025-05-20T11:05:59Z WARN  bpf_filtering::initializer] process 0 not found while building map_interest
[2025-05-20T11:05:59Z WARN  bpf_filtering::initializer] process 0 not found while building map_interest

Hmm... this is because we use PID 0 as a ppid for PID 1. Good catch, that doesn't make sense. We should define ppid as Option<Pid>. Will fix.

and sometimes

[2025-05-20T11:07:19Z ERROR file-system-monitor] Process not found in tracker 63980: process not found
[2025-05-20T11:07:19Z ERROR file-system-monitor] Process not found in tracker 63980: process not found

Interesting. Haven't seen these on my end, but will try to reproduce.

the goal of BPF iterators should be to solve the Process not found in tracker error

@vadorovsky
Copy link
Member Author

@banditopazzo Are you seeing these errors after, let's say, the first 15s of Pulsar's runtime?

In my case, I see them only at the very beginning and then never again. And after dumping the process tree to a file with the following diff

diff --git a/crates/bpf-filtering/src/process_tree.rs b/crates/bpf-filtering/src/process_tree.rs
index 014fcf6..c7622fa 100644
--- a/crates/bpf-filtering/src/process_tree.rs
+++ b/crates/bpf-filtering/src/process_tree.rs
@@ -171,6 +171,8 @@ impl ProcessTree {
                 ProcessData::from_str(&line)
             })
             .collect::<Result<Vec<ProcessData>, Error>>()?;
+        std::fs::write("/tmp/processes", format!("{processes:#?}")).unwrap();
+        log::error!("CONSUMED BPF ITERATOR");

         Ok(Self { processes })
     }

I see that all the PIDs mentioned in the errors, like:

[2025-05-24T23:49:33Z ERROR file-system-monitor] Process not found in tracker 8525: process not found

are present in the map created from the iterator:

    ProcessData {
        pid: Pid(
            8525,
        ),
        uid: Uid(
            0,
        ),
        gid: Gid(
            0,
        ),
        image: "/home/vadorovsky/src/pulsar/target/release/pulsar-exec",
        parent: Pid(
            8524,
        ),
        namespaces: Namespaces {
            uts: 4026531838,
            ipc: 4026531839,
            mnt: 4026531841,
            pid: 4026531836,
            net: 4026531840,
            time: 4026531834,
            cgroup: 4026531835,
        },
        container_id: None,
    },

What's funny, is that it's almost always the pulsar process generating this error.

After adding some more logs I'm 99% sure that the issue is a race condition with other modules. It seems like file-system-monitor (and all other probes) are starting and generating events before the process tree and process tracker are initialized. I will keep thinking about the solution, but what I would try for now is blocking all the other modules from starting and loading BPF programs before the process tracker is initialized.

Allow modules to be dependent on others by adding the `DEPENDS_ON`
constant to `PulsarModule` and `SimplePulsarModule` traits. This
constant can be set either as empty (`&[]`) or can specify dependencies
(e.g. `&["process-monitor"]`, `&["process-monitor", "network-monitor"]`).

Make `file-system-monitor` and `network-monitor` dependent on
`process-monitor`. This way we get rid of the race condition, where
these modules start before `process-monitor`, generating errors like:

```
Process not found in tracker 63980: process not found
```

These errors are not there anymore after introducing the dependency.
@vadorovsky vadorovsky changed the base branch from main to fix-race-condition May 26, 2025 09:18
@vadorovsky
Copy link
Member Author

@banditopazzo My assumption was right, all the errors you mentioned were a race condition between modules. I fixed that in #347 and rebased this PR on top of it. Can you check again if the issue occurs to you?

Replace the process tracker initialization process from procfs parsing
to BPF iterators, which are less erronous and result in an easier
implementation.
@vadorovsky vadorovsky force-pushed the fix-race-condition branch 2 times, most recently from 9b9a1c7 to 7aef8fe Compare May 26, 2025 10:18
Using a `Vec` is not efficient, because lookups for the given process
are done with O(n) complexity. Changing it to `HashMap` makes the lookup
an O(1) operation.
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.

2 participants