new(driver/modern_bpf): Add ppc64le support for modern-bpf #1475

mdafsanhossain · 2023-11-07T13:20:54Z

What type of PR is this?
/kind feature

Any specific area of the project related to this PR?

/area driver-modern-bpf
/area libscap-engine-modern-bpf
/area libscap

What this PR does / why we need it:
This PR adds PPC64LE specific changes to allow compiling of modern-bpf on PowerPC architecture. This is to add modern-bpf support for Stackrox on ppc64le.

Output of make ProbeSkeleton and make scap are attached below.

scap-build.log
probeskeleton.log

Note: syscall_compat_* headers were generated using ./build/syscalls-bumper -repo-root /root/libs -overwrite

Does this PR introduce a user-facing change?:

new(modern-bpf): support ppc64le architecture.

poiana · 2023-11-07T13:21:04Z

Welcome @mdafsanhossain! It looks like this is your first PR to falcosecurity/libs 🎉

FedeDP · 2023-11-07T13:42:40Z

Hi! Thanks for this PR! Is there any plan to support ppc64le also for old bpf probe (and eventually kmod too)?
Btw:

Note: syscall_compat_* headers were generated using ./build/syscalls-bumper -repo-root /root/libs -overwrite

syscalls-bumper does not support ppc64le though; was its support added locally? Can you open a PR over there too?
I noticed that:

the report.md changes look weird (it seems like riscv64 arch was dropped?)
some syscalls were removed from syscall_compat_* and that is weird too

mdafsanhossain · 2023-11-08T06:28:02Z

Hi! Thanks for this PR! Is there any plan to support ppc64le also for old bpf probe (and eventually kmod too)? Btw:

Note: syscall_compat_* headers were generated using ./build/syscalls-bumper -repo-root /root/libs -overwrite

syscalls-bumper does not support ppc64le though; was its support added locally? Can you open a PR over there too? I noticed that:

the report.md changes look weird (it seems like riscv64 arch was dropped?)

some syscalls were removed from syscall_compat_* and that is weird too

Yes I added the support locally and built it.

diff --git a/main.go b/main.go
index ae9eae9..d085ea1 100644
--- a/main.go
+++ b/main.go
@@ -119,10 +119,10 @@ func initOpts() {
 // key should be consistent with the one used by https://github.com/hrw/syscalls-table/tree/master/tables.
 // Value should be the suffix used by libs/driver/syscall_compat_* headers.
 var supportedArchs = map[string]string{
-       "x86_64":  "x86_64",
-       "arm64":   "aarch64",
-       "s390x":   "s390x",
-       "riscv64": "riscv64",
+       "x86_64":    "x86_64",
+       "arm64":     "aarch64",
+       "s390x":     "s390x",
+       "powerpc64": "ppc64le",
 }

I removed riscv64 from the list. I will revert the last commit and regenerate it with riscv64 included.
Do you know why some of the syscalls are being removed?
And, are the above changes sufficient to generate correctly?

FedeDP · 2023-11-08T07:46:25Z

I removed riscv64 from the list. I will revert the last commit and regenerate it with riscv64 included.

Thanks!

And, are the above changes sufficient to generate correctly?

Yes, they are!

Do you know why some of the syscalls are being removed?

Just double checked! Yep because they were cleaned up by the project that syscalls-bumper uses under the hood: hrw/syscalls-table@079ef06, hrw/syscalls-table@94b4691, hrw/syscalls-table@046c3b5

FedeDP · 2023-11-08T07:47:05Z

Also, you now need a rebase since a PR with automatic bump of syscall-bumper was merged ;)

FedeDP · 2023-11-08T07:48:32Z

Also, once this is fully in place, mind to also update README adding the new supported arch for modern bpf probe as EXPERIMENTAL?

mdafsanhossain · 2023-11-08T07:50:59Z

Also, you now need a rebase since a PR with automatic bump of syscall-bumper was merged ;)

Thanks. I will do that, clean it up and after that update the README too.

FedeDP · 2023-11-08T08:34:39Z

driver/modern_bpf/helpers/interfaces/attached_programs.h

-	SYSCALL = 0,
+	//SYSCALL definition is skipped on powerpc since SYSCALL is already defined in vmlinux.h
+	#ifndef __TARGET_ARCH_powerpc
+		SYSCALL = 0,


I think we just need to avoid name collision, by prefixing these ones with eg: MODERN_BPF_.
@Andreagit97 thoughts?

@mdafsanhossain here !

@FedeDP Do you mean something like this? MODERN_BPF_SYSCALL=0? I am not sure if I am understanding this correctly.

enum intrumentation_type { MODERN_BPF_SYSCALL = 0, MODERN_BPF_TRACEPOINT = 1, }

and then rename their usages whereveer sampling_logic is called.

yes I agree, probably we can rename them in INSTR_TYPE_SYSCALL and INSTR_TYPE_TRACEPOINT

uhm I didn't see new messages, if we all agree it's also okay to go with

enum intrumentation_type { MODERN_BPF_SYSCALL = 0, MODERN_BPF_TRACEPOINT = 1, }

Makes sense now. Thanks :)

FedeDP · 2023-11-08T09:14:02Z

/milestone next-driver

mdafsanhossain · 2023-11-08T09:43:51Z

@FedeDP Is it fine that the syscalls are being removed?

FedeDP · 2023-11-08T09:49:41Z

Is it fine that the syscalls are being removed?

It is ;)
This LGTM except for the comment on driver/modern_bpf/helpers/interfaces/attached_programs.h.

Were you able to run drivers_test tests on a real ppc64le machine?

mdafsanhossain · 2023-11-08T09:58:27Z

Is it fine that the syscalls are being removed?

It is ;) This LGTM except for the comment on driver/modern_bpf/helpers/interfaces/attached_programs.h.

Were you able to run drivers_test tests on a real ppc64le machine?

I am running it now.
Do you have a better way to resolve the conflict on driver/modern_bpf/helpers/interfaces/attached_programs.h.?

Andreagit97 · 2023-11-08T10:14:35Z

driver/SCHEMA_VERSION

we bumped this because new PPM_CODES could be sent from the drivers

mdafsanhossain · 2023-11-08T13:23:17Z

Is it fine that the syscalls are being removed?

It is ;) This LGTM except for the comment on driver/modern_bpf/helpers/interfaces/attached_programs.h.

Were you able to run drivers_test tests on a real ppc64le machine?

@FedeDP I am running into an issue while trying to run the drivers_test

-----------------------------------------------------
- Setup phase
-----------------------------------------------------

* Configure modern BPF probe tests!
* Using buffer dim: 8388608
Unable to open the engine: the specified per-CPU ring buffer dimension (0) is not allowed! Please use a power of 2 and a multiple of the actual page_size (65536)!

Steps to reproduce:

cmake -DUSE_BUNDLED_DEPS=On -DBUILD_LIBSCAP_MODERN_BPF=On -DENABLE_DRIVERS_TESTS=On -DBUILD_LIBSCAP_GVISOR=Off -DCREATE_TEST_TARGETS=On ..
make drivers_test
sudo ./test/drivers/drivers_test -m

Btw there are 2 small bugs in these files which prevents drivers_test compilation.

https://github.com/mdafsanhossain/libs/blob/ec188eb2f97fa927c961595c05ff74275b057b7b/test/drivers/test_suites/syscall_exit_suite/socketcall_x.cpp#L3149 - int64_t errno_value = -errno; is not declared
https://github.com/mdafsanhossain/libs/blob/e2cdf1e084dabc8b42e69bae83ee49a74096ca1c/test/drivers/test_suites/syscall_exit_suite/send_x.cpp#L15 -No semicolon after int flags=0

FedeDP · 2023-11-08T14:00:45Z

Btw there are 2 small bugs in these files which prevents drivers_test compilation.

Wow! Can you fix them? We only run drivers tests on x86_64 and aarch64 that haven't got __NR_send nor __NR_socketcall defined and nobody ever caught those issues :)
Btw the errno one is pretty weird, errno should be declared.

Andreagit97 · 2023-11-08T14:18:34Z

-----------------------------------------------------
- Setup phase
-----------------------------------------------------

* Configure modern BPF probe tests!
* Using buffer dim: 8388608
Unable to open the engine: the specified per-CPU ring buffer dimension (0) is not allowed! Please use a power of 2 and a multiple of the actual page_size (65536)!

This error is weird, the buffer dim is correct in this line

* Using buffer dim: 8388608

but in the below line is 0

Unable to open the engine: the specified per-CPU ring buffer dimension (0) is not allowed!

It seems like someone on powerpc clears this buffer_bytes_dim 🤔

Steps to reproduce:

Just tried on my machine and it works, moreover these tests run in CI so they are working it seems something specific to powerpc

mdafsanhossain · 2023-11-08T14:35:18Z

-----------------------------------------------------
- Setup phase
-----------------------------------------------------

* Configure modern BPF probe tests!
* Using buffer dim: 8388608
Unable to open the engine: the specified per-CPU ring buffer dimension (0) is not allowed! Please use a power of 2 and a multiple of the actual page_size (65536)!
This error is weird, the buffer dim is correct in this line
* Using buffer dim: 8388608
but in the below line is 0
Unable to open the engine: the specified per-CPU ring buffer dimension (0) is not allowed! 
It seems like someone on powerpc clears this buffer_bytes_dim 🤔

Steps to reproduce:

Just tried on my machine and it works, moreover these tests run in CI so they are working it seems something specific to powerpc

I wonder what is resetting buffer_bytes_dim to 0. I will try to debug it but I don't have much idea on it.

Signed-off-by: mdafsanhossain <[email protected]>

Signed-off-by: Afsan Hossain <[email protected]>

Signed-off-by: mdafsanhossain <[email protected]>

Signed-off-by: Afsan Hossain <[email protected]>

Andreagit97

For me it's great, thank you
/approve

poiana · 2023-11-15T09:25:48Z

LGTM label has been added.

Git tree hash: c2202ba3d7f45e72b107e5b3f050343da3b4f6a8

FedeDP · 2023-11-15T10:12:44Z

@mdafsanhossain can you also open the PR on syscalls-bumper please? :)

FedeDP

/approve

poiana · 2023-11-15T10:13:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, mdafsanhossain

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [Andreagit97,FedeDP]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

mdafsanhossain · 2023-11-15T10:14:24Z

@mdafsanhossain can you also open the PR on syscalls-bumper please? :)

Will do

FedeDP · 2023-11-15T10:43:21Z

/unhold

hbrueckner · 2023-11-16T08:24:05Z

too late.... however, this looks very nice! Thanks a lot!
/approve

FedeDP · 2023-11-16T08:29:30Z

Ops sorry @hbrueckner , i completely forgot... my fault 😭 sorry again!

poiana added dco-signoff: yes kind/feature New feature or request area/driver-modern-bpf do-not-merge/release-note-label-needed area/libscap-engine-modern-bpf area/libscap labels Nov 7, 2023

poiana requested review from FedeDP and hbrueckner November 7, 2023 13:21

poiana added the size/XXL label Nov 7, 2023

Andreagit97 added this to the TBD milestone Nov 7, 2023

mdafsanhossain force-pushed the ppc64le-modern-bpf branch from df2cba6 to 6ef477f Compare November 8, 2023 08:10

FedeDP reviewed Nov 8, 2023

View reviewed changes

poiana modified the milestones: TBD, next-driver Nov 8, 2023

Andreagit97 reviewed Nov 8, 2023

View reviewed changes

driver/SCHEMA_VERSION Outdated

Copy link

Member

Andreagit97 Nov 8, 2023

Choose a reason for hiding this comment

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

we bumped this because new PPM_CODES could be sent from the drivers

mdafsanhossain added 7 commits November 15, 2023 11:16

vmlinux.h created on fedore 35

37a0712

Signed-off-by: mdafsanhossain <[email protected]>

Skip SYSCALL redefinition on powerpc

8978993

Signed-off-by: Afsan Hossain <[email protected]>

Remove endif

90d67f1

Signed-off-by: mdafsanhossain <[email protected]>

Update README to add ppc64le modern-bpf as EXPERIMENTAL

60acbcf

Signed-off-by: mdafsanhossain <[email protected]>

Generated syscall_compat_* header files

9043393

Signed-off-by: mdafsanhossain <[email protected]>

Rename SYSCALL and TRACEPOINT to avoid conflicts

84ccc70

Signed-off-by: Afsan Hossain <[email protected]>

powerpc support in bpf_in_ia32_syscall

8ea66a7

Signed-off-by: Afsan Hossain <[email protected]>

mdafsanhossain force-pushed the ppc64le-modern-bpf branch from fca6fef to 8ea66a7 Compare November 15, 2023 05:55

Andreagit97 approved these changes Nov 15, 2023

View reviewed changes

poiana assigned Andreagit97 Nov 15, 2023

poiana added the lgtm label Nov 15, 2023

poiana added the approved label Nov 15, 2023

FedeDP approved these changes Nov 15, 2023

View reviewed changes

poiana assigned FedeDP Nov 15, 2023

poiana added release-note-none release-note and removed do-not-merge/hold do-not-merge/release-note-label-needed release-note-none labels Nov 15, 2023

poiana merged commit 7092b1e into falcosecurity:master Nov 15, 2023
30 of 31 checks passed

mdafsanhossain deleted the ppc64le-modern-bpf branch November 16, 2023 06:50

Andreagit97 mentioned this pull request Nov 27, 2023

new(driver): add ppc64le support to old bpf and kmod plus CI job #1497

Merged

Andreagit97 changed the title ~~new(modern_bpf): Add ppc64le support for modern-bpf~~ new(driver/modern_bpf): Add ppc64le support for modern-bpf Jan 8, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

new(driver/modern_bpf): Add ppc64le support for modern-bpf #1475

new(driver/modern_bpf): Add ppc64le support for modern-bpf #1475

mdafsanhossain commented Nov 7, 2023 •

edited by FedeDP

Loading

poiana commented Nov 7, 2023

FedeDP commented Nov 7, 2023

mdafsanhossain commented Nov 8, 2023 •

edited

Loading

FedeDP commented Nov 8, 2023

FedeDP commented Nov 8, 2023

FedeDP commented Nov 8, 2023

mdafsanhossain commented Nov 8, 2023

FedeDP Nov 8, 2023

FedeDP Nov 8, 2023

mdafsanhossain Nov 8, 2023

FedeDP Nov 8, 2023

Andreagit97 Nov 8, 2023

Andreagit97 Nov 8, 2023

mdafsanhossain Nov 8, 2023

FedeDP commented Nov 8, 2023

mdafsanhossain commented Nov 8, 2023

FedeDP commented Nov 8, 2023

mdafsanhossain commented Nov 8, 2023

Andreagit97 Nov 8, 2023

mdafsanhossain commented Nov 8, 2023

FedeDP commented Nov 8, 2023

Andreagit97 commented Nov 8, 2023

mdafsanhossain commented Nov 8, 2023

Andreagit97 left a comment

poiana commented Nov 15, 2023

FedeDP commented Nov 15, 2023

FedeDP left a comment

poiana commented Nov 15, 2023

mdafsanhossain commented Nov 15, 2023

FedeDP commented Nov 15, 2023

hbrueckner commented Nov 16, 2023

FedeDP commented Nov 16, 2023

new(driver/modern_bpf): Add ppc64le support for modern-bpf #1475

new(driver/modern_bpf): Add ppc64le support for modern-bpf #1475

Conversation

mdafsanhossain commented Nov 7, 2023 • edited by FedeDP Loading

poiana commented Nov 7, 2023

FedeDP commented Nov 7, 2023

mdafsanhossain commented Nov 8, 2023 • edited Loading

FedeDP commented Nov 8, 2023

FedeDP commented Nov 8, 2023

FedeDP commented Nov 8, 2023

mdafsanhossain commented Nov 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

FedeDP commented Nov 8, 2023

mdafsanhossain commented Nov 8, 2023

FedeDP commented Nov 8, 2023

mdafsanhossain commented Nov 8, 2023

Choose a reason for hiding this comment

mdafsanhossain commented Nov 8, 2023

FedeDP commented Nov 8, 2023

Andreagit97 commented Nov 8, 2023

mdafsanhossain commented Nov 8, 2023

Andreagit97 left a comment

Choose a reason for hiding this comment

poiana commented Nov 15, 2023

FedeDP commented Nov 15, 2023

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented Nov 15, 2023

mdafsanhossain commented Nov 15, 2023

FedeDP commented Nov 15, 2023

hbrueckner commented Nov 16, 2023

FedeDP commented Nov 16, 2023

mdafsanhossain commented Nov 7, 2023 •

edited by FedeDP

Loading

mdafsanhossain commented Nov 8, 2023 •

edited

Loading