Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

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

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: m-pellizzer
Once this PR has been reviewed and has the lgtm label, please assign hbrueckner for approval. For more information see the Kubernetes Code Review Process.

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

Perf diff from master - unit tests

    19.22%     -0.76%  [.] sinsp_threadinfo::get_main_thread
     5.95%     -0.45%  [.] sinsp_parser::reset
     1.90%     -0.32%  [.] sinsp_parser::process_event
     4.19%     -0.32%  [.] next_event_from_file
     1.92%     -0.28%  [.] sinsp_thread_manager::get_thread_ref
     5.61%     -0.27%  [.] sinsp_evt::get_type
     2.93%     +0.26%  [.] gzfile_read
     6.45%     +0.22%  [.] sinsp::next
     0.40%     +0.21%  [.] std::_Hashtable<conversion_key, std::pair<conversion_key const, conversion_info>, std::allocator<std::pair<conversion_key const, conversion_info> >, std::__detail::_Select1st, std::equal_to<conversion_key>, std::hash<conversion_key>, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_M_find_before_node
     0.92%     +0.21%  [.] user_group_updater::~user_group_updater

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.0571         -0.0572           151           143           151           143
BM_sinsp_split_median                                          -0.0625         -0.0625           152           142           152           142
BM_sinsp_split_stddev                                          -0.3423         -0.3418             2             1             2             1
BM_sinsp_split_cv                                              -0.3025         -0.3018             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  -0.0647         -0.0648            60            56            60            56
BM_sinsp_concatenate_paths_relative_path_median                -0.0679         -0.0680            60            56            60            56
BM_sinsp_concatenate_paths_relative_path_stddev                -0.4726         -0.4727             2             1             2             1
BM_sinsp_concatenate_paths_relative_path_cv                    -0.4361         -0.4362             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     -0.0156         -0.0157            25            25            25            25
BM_sinsp_concatenate_paths_empty_path_median                   -0.0119         -0.0120            25            24            25            24
BM_sinsp_concatenate_paths_empty_path_stddev                   -0.4775         -0.4778             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       -0.4692         -0.4695             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  -0.1142         -0.1143            59            52            59            52
BM_sinsp_concatenate_paths_absolute_path_median                -0.1243         -0.1244            59            52            59            52
BM_sinsp_concatenate_paths_absolute_path_stddev                -0.0148         -0.0147             1             1             1             1
BM_sinsp_concatenate_paths_absolute_path_cv                    +0.1123         +0.1125             0             0             0             0

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.17%. Comparing base (b242889) to head (2f63a55).
Report is 40 commits behind head on master.

@@           Coverage Diff           @@
##           master    #2301   +/-   ##
=======================================
  Coverage   77.17%   77.17%           
=======================================
  Files         224      224           
  Lines       30133    30133           
  Branches     4605     4605           
=======================================
  Hits        23256    23256           
  Misses       6877     6877           

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

Hi! Thanks for opening this PR! We already tried to fix this with @dkogan here: #2277
but couldn't find a solution that worked reliably for <6.13 and >=6.13 :/
Feel free to add your comments over there!

Thanks @FedeDP for the heads-up!
I modified the PR to keep compatibility with older kernels.

I tested the driver in Ubuntu against 6.12 and 6.14.

Hi. Please read all of #2277. Your fix does not work in general. In particular, look at #2277 (comment)

I looked around a bit, and I'm thinking that maybe the final fix in #2277 might actually work in many more than just Debian systems. I looked around, and the $(MODLIB)/build symlink does exist in the several places that I have looked. Can somebody poke at a few different distros to see if it ever doesn't exist anywhere?

Hello @dkogan. Thanks for the suggestion, I will test again my code and update the PR if needed.
In the meantime I can tell you that $(MODLIB)/build does not work on Ubuntu Plucky 6.14.

I tested the latest PR update against multiple kernels.
As a DKMS the driver builds against the following versions:

$ sudo dkms status
scap/0.20.0, 6.11.0-061100-generic, x86_64: installed
scap/0.20.0, 6.12.0-16-generic, x86_64: installed
scap/0.20.0, 6.13.0-061300-generic, x86_64: installed
scap/0.20.0, 6.14.0-061400rc6-generic, x86_64: installed

The driver compiles also from source without using the DKMS framework.

Notice that 6.14-rc6, 6.13, 6.11 are mainline kernels, while 6.12 is the Ubuntu generic kernel.

@dkogan can you please test also on your side and let me know if you get the same compile time errors mentioned in the issue?

I Just triggered the CI! Thanks for looking into this. Hopefully @dkogan is able to test this too.

I don't understand. Once again. Please read all of #2277. Your fix does not work in general. In particular, look at #2277 (comment)

@dkogan Can you please provide a specific case in which this solution is not working? This would help me debug further the issue

It does not work in Debian kernels, as noted in #2277. Linus presumably doesn't use a distro, and his $(srctree) points to the kernel he just built. In Debian, packages are available in many flavors and for many architectures. The $(srctree) points to the arch-independent directory. I guess ubuntu redid the kernel packaging, or something. Does $(srctree) work in your tests for kernels < 6.13? If so, how about this:

KERNELDIR ?= $(or $(realpath $(MODLIB)/build),$(srctree))

This is the result in debian unstable with kernel 6.12:

$ uname -a
Linux debian-unstable-amd64-tester 6.12.17-amd64 #1 SMP PREEMPT_DYNAMIC Debian 6.12.17-1 (2025-03-01) x86_64 GNU/Linux
$ sudo dkms status
scap/0.20.0, 6.12.17-amd64, x86_64: installed

As you said $(srctree) does not work with kernels > 6.13, for this reason in my PR there is the if statement, which makes sure that $(srctree) is used only if the kernel is >= 6.13, otherwise it falls back to what it was using before $(CURDIR)

OK. But then it doesn't work with Debian kernels. For those the patch in #2277 works in both old and new kernels. You can add another branch to your logic to cover those. But it really feels like there should be a better way.

The kernel I used in the previous testing (6.12.17-1) it's from Debian archive.
Can you please test the patch yourself on a system that you think might fail and provide the results?
Thanks

I think we're talking past each other. You tested 6.12 with $(CURDIR) (behind some if statements). This is the configuration that has been working for years. In 6.13 (obtained anywhere) this doesn't work.

The patch in #2277 works with both old and new Debian kernels, but you have observed it to fail with some others, and you're proposing a different patch to use $(srctree) in 6.13. I have observed this to fail in Debian. Are you saying that you have observed $(srctree) to work properly with Debian 6.13 kernels? If so, which package specifically is providing the kernel image and/or sources and/or headers that you're looking at?

Thanks

@dkogan Thanks for explaining me the issue more in depth.
I tested the patch on Debian experimental. On mainline 6.13 the patch works, however on Debian packaged 6.13 the driver does not compile, as you said. This is due, I think, to the version used for experimental kernels in Debian, in this case 6.13-amd64. I wonder if this version scheme, which differs from other version schemes used in older Debian kernels, is just because 6.13 is in experimental or it will stay like that also in unstable

Do you have evidence that the slightly different naming scheme is breaking it? The observations in #2277 don't agree: #2277 (comment)

$(srctree) doesn't work in both 6.12 and 6.13 in Debian. There are arch-independent packages (linux-headers-VERSION-common) and arch-specific packages (linux-headers-VERSION-ARCHITECTURE). The headers are split between those two packages, with the arch-independent ones shared by ALL architectures, and thus avoiding duplication. $(srctree) points to the arch-independent directory, while for dkms we want the arch-specific directory. This is why $(srctree) doesn't work, but $(MODLIB)/build does.

If you live in a world where you build a kernel and install it on the build machine (what the person that made this breaking kernel change almost certainly does), then you only are looking at one architecture, and there's no duplication to think about. So they inadvertently broke it in the Debian case.

Since #2329 was merged, we can close this one!
/close

@FedeDP: Closed this PR.