feat(libsinsp_e2e): add unix_udp_client_server_read test #2231

therealbobo · 2025-01-13T22:04:42Z

What type of PR is this?

Uncomment one (or more) /kind <> lines:

/kind bug

/kind cleanup

/kind design

/kind documentation

/kind failing-test

/kind feature

Any specific area of the project related to this PR?

Uncomment one (or more) /area <> lines:

/area API-version

/area build

/area CI

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-bpf

/area libscap-engine-gvisor

/area libscap-engine-kmod

/area libscap-engine-modern-bpf

/area libscap-engine-nodriver

/area libscap-engine-noop

/area libscap-engine-source-plugin

/area libscap-engine-savefile

/area libscap

/area libpman

/area libsinsp

/area tests

/area proposals

Does this PR require a change in the driver versions?

/version driver-API-version-major

/version driver-API-version-minor

/version driver-API-version-patch

/version driver-SCHEMA-version-major

/version driver-SCHEMA-version-minor

/version driver-SCHEMA-version-patch

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

chore(libsinsp_e2e): add unix_udp_client_server_read test
fix(driver): properly add back fallback to user data when peer socket data is missing
fix(driver/modern_bpf): lower sendmmsg and recvmmsg loop support to 8 to avoid limit size failures

github-actions · 2025-01-13T22:12:49Z

Perf diff from master - unit tests

     9.84%     +0.70%  [.] sinsp::next
     3.20%     +0.47%  [.] sinsp_evt::load_params
     1.06%     +0.44%  [.] sinsp_evt::get_syscall_return_value
     2.99%     -0.37%  [.] sinsp_parser::process_event
    10.35%     +0.32%  [.] sinsp_parser::reset
     1.17%     -0.30%  [.] sinsp_parser::event_cleanup
     1.16%     -0.28%  [.] std::_Hashtable<long, std::pair<long const, std::shared_ptr<sinsp_threadinfo> >, std::allocator<std::pair<long const, std::shared_ptr<sinsp_threadinfo> > >, std::__detail::_Select1st, std::equal_to<long>, std::hash<long>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<false, false, true> >::find
     2.17%     -0.24%  [.] next
     5.54%     +0.19%  [.] next_event_from_file
     0.73%     -0.19%  [.] 0x00000000000fd5b0

Heap diff from master - unit tests

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Benchmarks diff from master

Comparing gbench_data.json to /root/actions-runner/_work/libs/libs/build/gbench_data.json
Benchmark                                                         Time             CPU      Time Old      Time New       CPU Old       CPU New
----------------------------------------------------------------------------------------------------------------------------------------------
BM_sinsp_split_mean                                            +0.0330         +0.0330           147           152           147           152
BM_sinsp_split_median                                          +0.0324         +0.0324           147           152           147           152
BM_sinsp_split_stddev                                          -0.3417         -0.3415             2             1             2             1
BM_sinsp_split_cv                                              -0.3628         -0.3626             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  -0.1057         -0.1057            61            54            61            54
BM_sinsp_concatenate_paths_relative_path_median                -0.1057         -0.1057            61            54            61            54
BM_sinsp_concatenate_paths_relative_path_stddev                +0.2474         +0.2481             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_cv                    +0.3948         +0.3955             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     -0.0011         -0.0011            24            24            24            24
BM_sinsp_concatenate_paths_empty_path_median                   -0.0019         -0.0018            24            24            24            24
BM_sinsp_concatenate_paths_empty_path_stddev                   +0.1959         +0.1961             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       +0.1973         +0.1975             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  -0.1406         -0.1406            65            55            65            55
BM_sinsp_concatenate_paths_absolute_path_median                -0.1410         -0.1410            64            55            64            55
BM_sinsp_concatenate_paths_absolute_path_stddev                -0.4069         -0.4070             1             1             1             1
BM_sinsp_concatenate_paths_absolute_path_cv                    -0.3099         -0.3100             0             0             0             0
BM_sinsp_split_container_image_mean                            -0.0025         -0.0024           395           394           395           394
BM_sinsp_split_container_image_median                          -0.0034         -0.0034           396           394           396           394
BM_sinsp_split_container_image_stddev                          +0.4490         +0.4482             2             3             2             3
BM_sinsp_split_container_image_cv                              +0.4526         +0.4517             0             0             0             0

codecov · 2025-01-13T22:17:29Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.09%. Comparing base (8362ae9) to head (600fefb).
Report is 5 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2231   +/-   ##
=======================================
  Coverage   75.09%   75.09%           
=======================================
  Files         276      276           
  Lines       34391    34391           
  Branches     5927     5927           
=======================================
  Hits        25826    25826           
  Misses       8565     8565

Flag	Coverage Δ
libsinsp	`75.09% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

FedeDP · 2025-01-14T07:27:38Z

Thanks @therealbobo ! Since this PR highlights a test failure we should try to fix it up before the 0.20.0 release.
/milestone 0.20.0

Signed-off-by: Roberto Scolaro <[email protected]>

FedeDP · 2025-01-14T07:30:51Z

In the meantime, i rebased the PR on latest master.

FedeDP · 2025-01-14T09:26:19Z

/hold

FedeDP · 2025-01-14T09:41:20Z

driver/bpf/filler_helpers.h

+			// The first byte to 0 can be confused with an `abstract socket address` for this reason
+			// we put also the second byte to 0 to comunicate to the caller that the address is not
+			// valid.
+			sunaddr->sun_path[1] = 0;


Same fix applied in #2215 for kmod.

FedeDP · 2025-01-14T09:41:28Z

driver/modern_bpf/helpers/store/auxmap_store_params.h

@@ -776,7 +776,7 @@ static __always_inline void auxmap__store_socktuple_param(struct auxiliary_map *

 	case AF_UNIX: {
 		struct unix_sock *socket_local = (struct unix_sock *)sk;
-		struct unix_sock *socket_remote = (struct unix_sock *)BPF_CORE_READ(socket_local, peer);
+		struct unix_sock *socket_peer = (struct unix_sock *)BPF_CORE_READ(socket_local, peer);


FedeDP · 2025-01-14T09:41:41Z

driver/modern_bpf/helpers/store/auxmap_store_params.h

-		unsigned long start_reading_point;
-		char first_path_byte = *(char *)path;
-		if(first_path_byte == '\0') {
+		if(path[0] == '\0') {


Avoid convoluted logic with multiple variables involved.

i have the suspect it was used to avoid some verifier madness, but let's see 🤞

FedeDP · 2025-01-14T09:42:16Z

driver/ppm_events.c

@@ -861,6 +861,11 @@ static struct socket *ppm_sockfd_lookup_light(int fd, int *err, int *fput_needed
 */

 static void unix_socket_path(char *dest, const char *path, size_t size) {
+	if(path == NULL) {


Avoid de-referencing a possibly null ptr.

… paths handling in drivers. Signed-off-by: Federico Di Pierro <[email protected]>

driver/modern_bpf/helpers/store/auxmap_store_params.h

Andreagit97 · 2025-01-14T11:36:30Z

driver/modern_bpf/helpers/store/auxmap_store_params.h

-		unsigned long start_reading_point;
-		char first_path_byte = *(char *)path;
-		if(first_path_byte == '\0') {
+		if(path[0] == '\0') {


i have the suspect it was used to avoid some verifier madness, but let's see 🤞

… data is missing. Signed-off-by: Federico Di Pierro <[email protected]> Co-authored-by: Andrea Terzolo <[email protected]>

FedeDP · 2025-01-14T12:03:09Z

X64 matrix; same failures we see on master (related to #2027) :/

KERNEL	CMAKE-CONFIGURE	KMOD BUILD	KMOD SCAP-OPEN	BPF-PROBE BUILD	BPF-PROBE SCAP-OPEN	MODERN-BPF SCAP-OPEN
amazonlinux2-4.19	🟢	🟢	🟢	🟢	🟢	🟡
amazonlinux2-5.10	🟢	🟢	🟢	🟢	🟢	❌
amazonlinux2-5.15	🟢	🟢	🟢	🟢	🟢	❌
amazonlinux2-5.4	🟢	🟢	🟢	🟢	🟢	🟡
amazonlinux2022-5.15	🟢	🟢	🟢	🟢	🟢	❌
amazonlinux2023-6.1	🟢	🟢	🟢	🟢	🟢	❌
archlinux-6.0	🟢	🟢	🟢	🟢	🟢	❌
archlinux-6.7	🟢	🟢	🟢	🟢	🟢	🟢
centos-3.10	🟢	🟢	🟢	🟡	🟡	🟡
centos-4.18	🟢	🟢	🟢	🟢	🟢	❌
centos-5.14	🟢	🟢	🟢	🟢	🟢	🟢
fedora-5.17	🟢	🟢	🟢	🟢	🟢	❌
fedora-5.8	🟢	🟢	🟢	🟢	🟢	🟢
fedora-6.2	🟢	🟢	🟢	🟢	🟢	❌
oraclelinux-3.10	🟢	🟢	🟢	🟡	🟡	🟡
oraclelinux-4.14	🟢	🟢	🟢	🟢	🟢	🟡
oraclelinux-5.15	🟢	🟢	🟢	🟢	🟢	❌
oraclelinux-5.4	🟢	🟢	🟢	🟢	🟢	🟡
ubuntu-4.15	🟢	🟢	🟢	🟢	🟢	🟡
ubuntu-5.8	🟢	🟢	🟢	🟢	🟢	🟡
ubuntu-6.5	🟢	🟢	🟢	🟢	🟢	🟢

Will try to fix them in this same PR.

Signed-off-by: Andrea Terzolo <[email protected]>

… to avoid limit size failures. Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP · 2025-01-14T14:36:27Z

Latest kernel matrix is finally back green 🚀

KERNEL	CMAKE-CONFIGURE	KMOD BUILD	KMOD SCAP-OPEN	BPF-PROBE BUILD	BPF-PROBE SCAP-OPEN	MODERN-BPF SCAP-OPEN
amazonlinux2-4.19	🟢	🟢	🟢	🟢	🟢	🟡
amazonlinux2-5.10	🟢	🟢	🟢	🟢	🟢	🟢
amazonlinux2-5.15	🟢	🟢	🟢	🟢	🟢	🟢
amazonlinux2-5.4	🟢	🟢	🟢	🟢	🟢	🟡
amazonlinux2022-5.15	🟢	🟢	🟢	🟢	🟢	🟢
amazonlinux2023-6.1	🟢	🟢	🟢	🟢	🟢	🟢
archlinux-6.0	🟢	🟢	🟢	🟢	🟢	🟢
archlinux-6.7	🟢	🟢	🟢	🟢	🟢	🟢
centos-3.10	🟢	🟢	🟢	🟡	🟡	🟡
centos-4.18	🟢	🟢	🟢	🟢	🟢	🟢
centos-5.14	🟢	🟢	🟢	🟢	🟢	🟢
fedora-5.17	🟢	🟢	🟢	🟢	🟢	🟢
fedora-5.8	🟢	🟢	🟢	🟢	🟢	🟢
fedora-6.2	🟢	🟢	🟢	🟢	🟢	🟢
oraclelinux-3.10	🟢	🟢	🟢	🟡	🟡	🟡
oraclelinux-4.14	🟢	🟢	🟢	🟢	🟢	🟡
oraclelinux-5.15	🟢	🟢	🟢	🟢	🟢	🟢
oraclelinux-5.4	🟢	🟢	🟢	🟢	🟢	🟡
ubuntu-4.15	🟢	🟢	🟢	🟢	🟢	🟡
ubuntu-5.8	🟢	🟢	🟢	🟢	🟢	🟡
ubuntu-6.5	🟢	🟢	🟢	🟢	🟢	🟢

FedeDP · 2025-01-14T14:36:37Z

/unhold

poiana · 2025-01-14T14:41:33Z

LGTM label has been added.

Git tree hash: fed2fa492496b13098a891d9d9fe081a27d43f43

FedeDP · 2025-01-14T15:31:40Z

/approve

poiana · 2025-01-14T15:31:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: FedeDP, therealbobo

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~~ [FedeDP]

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

poiana added release-note-none dco-signoff: yes kind/feature New feature or request area/libsinsp area/tests size/XL labels Jan 13, 2025

poiana requested review from hbrueckner and Molter73 January 13, 2025 22:05

therealbobo force-pushed the therealbobo-misc branch from f0cafa7 to 19036af Compare January 13, 2025 22:05

therealbobo force-pushed the therealbobo-misc branch from 19036af to 7e8e51c Compare January 13, 2025 22:21

poiana added this to the 0.20.0 milestone Jan 14, 2025

feat(libsinsp_e2e): add unix_udp_client_server_read test

38e20c8

Signed-off-by: Roberto Scolaro <[email protected]>

FedeDP force-pushed the therealbobo-misc branch from 7e8e51c to 38e20c8 Compare January 14, 2025 07:30

poiana added the do-not-merge/hold label Jan 14, 2025

FedeDP force-pushed the therealbobo-misc branch from a056f3c to 78aa54d Compare January 14, 2025 09:34

FedeDP reviewed Jan 14, 2025

View reviewed changes

fix(driver): round of small fixes and improvements around unix socket…

7809820

… paths handling in drivers. Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP force-pushed the therealbobo-misc branch 2 times, most recently from 50bc294 to e1baab6 Compare January 14, 2025 10:59

Andreagit97 reviewed Jan 14, 2025

View reviewed changes

fix(driver): properly add back fallback to user data when peer socket…

258aa7e

… data is missing. Signed-off-by: Federico Di Pierro <[email protected]> Co-authored-by: Andrea Terzolo <[email protected]>

FedeDP force-pushed the therealbobo-misc branch from e1baab6 to 258aa7e Compare January 14, 2025 11:42

Andreagit97 and others added 2 commits January 14, 2025 14:52

fix: always initialize peer_address in the kmod

6498f92

Signed-off-by: Andrea Terzolo <[email protected]>

fix(driver/modern_bpf): lower sendmmsg and recvmmsg loop support to 8…

600fefb

… to avoid limit size failures. Signed-off-by: Federico Di Pierro <[email protected]>

FedeDP force-pushed the therealbobo-misc branch from f2d861b to 600fefb Compare January 14, 2025 13:53

poiana removed the do-not-merge/hold label Jan 14, 2025

leogr approved these changes Jan 14, 2025

View reviewed changes

poiana assigned leogr Jan 14, 2025

poiana added the lgtm label Jan 14, 2025

poiana added release-note and removed release-note-none labels Jan 14, 2025

LucaGuerra approved these changes Jan 14, 2025

View reviewed changes

poiana assigned LucaGuerra Jan 14, 2025

poiana added the approved label Jan 14, 2025

poiana merged commit d327a3b into falcosecurity:master Jan 14, 2025
55 of 57 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(libsinsp_e2e): add unix_udp_client_server_read test #2231

feat(libsinsp_e2e): add unix_udp_client_server_read test #2231

therealbobo commented Jan 13, 2025 •

edited by FedeDP

Loading

github-actions bot commented Jan 13, 2025 •

edited

Loading

codecov bot commented Jan 13, 2025 •

edited

Loading

FedeDP commented Jan 14, 2025

FedeDP commented Jan 14, 2025

FedeDP commented Jan 14, 2025

FedeDP Jan 14, 2025

FedeDP Jan 14, 2025

FedeDP Jan 14, 2025

Andreagit97 Jan 14, 2025

FedeDP Jan 14, 2025

Andreagit97 Jan 14, 2025

FedeDP commented Jan 14, 2025 •

edited

Loading

FedeDP commented Jan 14, 2025

FedeDP commented Jan 14, 2025

poiana commented Jan 14, 2025

FedeDP commented Jan 14, 2025

poiana commented Jan 14, 2025

feat(libsinsp_e2e): add unix_udp_client_server_read test #2231

feat(libsinsp_e2e): add unix_udp_client_server_read test #2231

Conversation

therealbobo commented Jan 13, 2025 • edited by FedeDP Loading

github-actions bot commented Jan 13, 2025 • edited Loading

Perf diff from master - unit tests

Heap diff from master - unit tests

Heap diff from master - scap file

Benchmarks diff from master

codecov bot commented Jan 13, 2025 • edited Loading

Codecov Report

FedeDP commented Jan 14, 2025

FedeDP commented Jan 14, 2025

FedeDP commented Jan 14, 2025

FedeDP Jan 14, 2025

Choose a reason for hiding this comment

FedeDP Jan 14, 2025

Choose a reason for hiding this comment

FedeDP Jan 14, 2025

Choose a reason for hiding this comment

Andreagit97 Jan 14, 2025

Choose a reason for hiding this comment

FedeDP Jan 14, 2025

Choose a reason for hiding this comment

Andreagit97 Jan 14, 2025

Choose a reason for hiding this comment

FedeDP commented Jan 14, 2025 • edited Loading

FedeDP commented Jan 14, 2025

FedeDP commented Jan 14, 2025

poiana commented Jan 14, 2025

FedeDP commented Jan 14, 2025

poiana commented Jan 14, 2025

therealbobo commented Jan 13, 2025 •

edited by FedeDP

Loading

github-actions bot commented Jan 13, 2025 •

edited

Loading

codecov bot commented Jan 13, 2025 •

edited

Loading

FedeDP commented Jan 14, 2025 •

edited

Loading