Skip to content

Add cgroup bcc flag for compilation#1748

Open
tewaro wants to merge 8 commits intopixie-io:mainfrom
tewaro:add-cgroup-bcc-flag
Open

Add cgroup bcc flag for compilation#1748
tewaro wants to merge 8 commits intopixie-io:mainfrom
tewaro:add-cgroup-bcc-flag

Conversation

@tewaro
Copy link
Copy Markdown
Contributor

@tewaro tewaro commented Oct 20, 2023

Summary: This is to ensure that the reversions in #1738 are not necessary for #1638. It enables a flag for bcc compilation for detecting that the bpf_get_current_cgroup_id exists within that kernel version. That flag is GET_CGROUP_ID_ENABLED. And an example of it's usage is in src/stirling/bpf_tools/testdata/get_current_cgroup_id.c.

Type of change: /kind feature

Test Plan: Automated test added to ensure that this feature is enabled and works.

Signed-off-as: AdityaAtulTewari adityaatewari@gmail.com

Signed-off-by: AdityaAtulTewari <adityaatewari@gmail.com>
Signed-off-by: AdityaAtulTewari <adityaatewari@gmail.com>
Signed-off-by: AdityaAtulTewari <adityaatewari@gmail.com>
Signed-off-by: AdityaAtulTewari <adityaatewari@gmail.com>
Signed-off-by: AdityaAtulTewari <adityaatewari@gmail.com>
@tewaro tewaro force-pushed the add-cgroup-bcc-flag branch from a47e3b2 to f389cc3 Compare October 20, 2023 17:11
Signed-off-by: AdityaAtulTewari <adityaatewari@gmail.com>
@tewaro tewaro force-pushed the add-cgroup-bcc-flag branch from 4acaa62 to 2f2a5e8 Compare October 23, 2023 00:14
@tewaro tewaro temporarily deployed to pr-actions-approval October 23, 2023 00:15 — with GitHub Actions Inactive
@tewaro tewaro marked this pull request as ready for review October 23, 2023 01:40
@tewaro tewaro requested a review from a team as a code owner October 23, 2023 01:40
Signed-off-by: AdityaAtulTewari <adityaatewari@gmail.com>
@tewaro tewaro temporarily deployed to pr-actions-approval November 6, 2023 13:30 — with GitHub Actions Inactive
@tewaro tewaro temporarily deployed to pr-actions-approval November 21, 2023 20:02 — with GitHub Actions Inactive
ddelnano added a commit to ddelnano/pixie that referenced this pull request Jan 12, 2024
…resource to support concatenated (not preprocessed) header files

Signed-off-by: Dom Del Nano <ddelnano@gmail.com>
@ddelnano
Copy link
Copy Markdown
Member

@AdityaAtulTewari sorry for the late reply here. The solution I envisioned was what I've implemented in 3bab3f0. That change isn't production ready, but shows how the pl_bpf_cc_resource bazel rule can be enhanced to support delaying some c preprocessing logic. I've verified that this prevents the bcc compilation issues and that the resulting socket_trace.c file still contains the #ifdef macros.

ddelnano@pixie-dev:~/code/pixie (ddelnano/add-cgroup-short-proc-fix-behind-feature-toggle) $ bazel build  src/stirling/source_connectors/socket_tracer\/bcc_bpf:socket_trace_bpf_preprocess
Starting local Bazel server and connecting to it...
INFO: Analyzed target //src/stirling/source_connectors/socket_tracer/bcc_bpf:socket_trace_bpf_preprocess (87 packages loaded, 1828 targets configured).
INFO: Found 1 target...
INFO: Deleting stale sandbox base /home/ddelnano/.cache/bazel/_bazel_ddelnano/b2e8b093089e374aa7d44dfed4f9953b/sandbox
Target //src/stirling/source_connectors/socket_tracer/bcc_bpf:socket_trace_bpf_preprocess up-to-date:
  bazel-bin/src/stirling/source_connectors/socket_tracer/bcc_bpf/socket_trace_bpf_preprocess.c
INFO: Elapsed time: 13.225s, Critical Path: 0.18s
INFO: 2 processes: 1 internal, 1 linux-sandbox.
INFO: Build completed successfully, 2 total actions


ddelnano@pixie-dev:~/code/pixie (ddelnano/add-cgroup-short-proc-fix-behind-feature-toggle) $ head -n 11 bazel-bin/src/stirling/source_connectors/socket_tracer/bcc_bpf/socket_trace_bpf_preprocess.c
#ifdef GET_CGROUP_ID_ENABLED
uint64_t pl_bpf_get_current_cgroup_id() {
    return bpf_get_current_cgroup_id();
}
#else

uint64_t pl_bpf_get_current_cgroup_id() {
    // TODO(ddelnano): UINT64_MAX doesn't work in BPF. Find another way to represent this.
    return (uint64_t)18446744073709551615;
}
#endif

In order to move forward with this, I think we will need some feedback on this approach from @vihangm and @JamesMBartlett. If this is something we are comfortable with, then we can shape up what I've prototyped.

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