new(driver): update exit events PPME_SYSCALL_READ_X and PPME_SYSCALL_PREAD_X with enter params

[Andreagit97]

What type of PR is this?

/kind feature

Any specific area of the project related to this PR?

/area driver-kmod

/area driver-bpf

/area driver-modern-bpf

/area libscap-engine-gvisor

/area libscap

/area libsinsp

/area tests

Does this PR require a change in the driver versions?

/version driver-SCHEMA-version-major

What this PR does / why we need it:

This PR is part of #2068. We update the first 2 exit events PPME_SYSCALL_READ_X and PPME_SYSCALL_PREAD_X with enter params. This is still a big PR because it enables the conversion in the scap-file engine and adds tests for scap-files at sinsp level. All the following PRs should only contain the code related to the event conversion no more scaffolding.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

[Andreagit97]

~400 lines are tests

[Andreagit97]

As discussed here #2068 (comment) we need a major bump in the SCHEMA VERSION because we are adding a new filler for pread64. This should be an isolated case

[github-actions]

Perf diff from master - unit tests

     5.34%     -2.65%  [.] next
     5.64%     +2.10%  [.] sinsp::next
     5.25%     +1.50%  [.] sinsp_evt::get_type
     1.21%     -0.81%  [.] sinsp_filter_check::parse_field_name
     1.75%     +0.70%  [.] scap_event_decode_params
     2.87%     +0.67%  [.] sinsp_thread_manager::get_thread_ref
     1.54%     -0.65%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>
     3.81%     +0.53%  [.] sinsp_parser::process_event
     0.93%     -0.53%  [.] sinsp_threadinfo::~sinsp_threadinfo
     0.72%     -0.46%  [.] std::_Hashtable<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, libsinsp::state::dynamic_struct::field_info>, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, libsinsp::state::dynamic_struct::field_info> >, std::__detail::_Select1st, std::equal_to<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::hash<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::__detail::_Mod_range_hashing, std::__detail::_Default_ranged_hash, std::__detail::_Prime_rehash_policy, std::__detail::_Hashtable_traits<true, false, true> >::_M_emplace<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, libsinsp::state::dynamic_struct::field_info> >

Heap diff from master - unit tests

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

Heap diff from master - scap file

peak heap memory consumption: -131.81K
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.0250         +0.0250           144           148           144           148
BM_sinsp_split_median                                          +0.0288         +0.0288           143           148           143           148
BM_sinsp_split_stddev                                          -0.2273         -0.2277             2             1             2             1
BM_sinsp_split_cv                                              -0.2462         -0.2465             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  +0.0021         +0.0021            57            57            57            57
BM_sinsp_concatenate_paths_relative_path_median                +0.0026         +0.0025            57            57            57            57
BM_sinsp_concatenate_paths_relative_path_stddev                +0.0792         +0.0783             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_cv                    +0.0769         +0.0760             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     +0.0008         +0.0008            24            24            24            24
BM_sinsp_concatenate_paths_empty_path_median                   +0.0005         +0.0005            24            24            24            24
BM_sinsp_concatenate_paths_empty_path_stddev                   -0.0454         -0.0468             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       -0.0462         -0.0476             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  +0.0196         +0.0196            56            57            56            57
BM_sinsp_concatenate_paths_absolute_path_median                +0.0184         +0.0184            56            57            56            57
BM_sinsp_concatenate_paths_absolute_path_stddev                +2.3038         +2.3019             0             1             0             1
BM_sinsp_concatenate_paths_absolute_path_cv                    +2.2403         +2.2384             0             0             0             0
BM_sinsp_split_container_image_mean                            +0.0041         +0.0041           387           388           387           388
BM_sinsp_split_container_image_median                          +0.0074         +0.0074           386           389           386           389
BM_sinsp_split_container_image_stddev                          -0.3290         -0.3282             4             2             4             2
BM_sinsp_split_container_image_cv                              -0.3318         -0.3309             0             0             0             0

[codecov]

Codecov Report

Attention: Patch coverage is 87.78626% with 16 lines in your changes missing coverage. Please review.

Project coverage is 75.29%. Comparing base (bb27230) to head (45e39f0).
Report is 28 commits behind head on master.

Files with missing lines	Patch %	Lines
...serspace/libsinsp/test/scap_files/scap_file_test.h	78.57%	15 Missing ⚠️
userspace/libsinsp/event.h	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2176      +/-   ##
==========================================
+ Coverage   75.07%   75.29%   +0.22%     
==========================================
  Files         256      259       +3     
  Lines       33690    33836     +146     
  Branches     5768     5769       +1     
==========================================
+ Hits        25293    25478     +185     
+ Misses       8397     8358      -39     

Flag	Coverage Δ
libsinsp	75.29% <87.78%> (+0.22%)	⬆️

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]

Shouldn't we manage EF_LARGE_PAYLOAD here? (scap_event_has_large_payload(evt))

[Andreagit97]

Yes, but I would add it only if we really need a conversion for at least one EF_LARGE_PAYLOAD event in order to not increase complexity without reason. Maybe I can add an exception so that at the time we will need a conversion for an EF_LARGE_PAYLOAD event we will face a clear exception

[FedeDP]

It should not be that hard to implement (2-3 locs?) but i am fine with throwing an exception too. Whatever you feel more comfortable with; no syscall-based plugin will ever need a large payload i guess, but i'd like to at least get notified if we ever fall in that issue.

[Andreagit97]

yep I added the error, the point is that looking at our table today we don't have EF_LARGE_PAYLOAD events to convert and I'm not sure we will ever have them in future

[Andreagit97]

/hold we need to complete the gvisor changes

[Andreagit97]

/unhold after looking a little bit into it the current gvisor changes are enough. The exit event always contains the same parameters of the enter one + the return value. See this method https://github.com/google/gvisor/blob/b78f2ee7c4c393990a84298dd0f200e927b18dab/pkg/sentry/kernel/task_syscall.go#L84

[LucaGuerra]

Thanks for checking the gvisor change!

[poiana]

LGTM label has been added.

Git tree hash: cf498b2293b1b8b24bf80f42980c0e032deadc79

[FedeDP]

/approve

[poiana]

[APPROVALNOTIFIER] This PR is APPROVED

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

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