update(sinsp/ifinfo): add new public addr_to_string methods #1937

incertum · 2024-06-27T23:47:14Z

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:

Expose proper / public ip4 and ip6 ip addr to string conversion methods, see https://github.com/falcosecurity/falco/pull/3253/files#r1651554317 @sgaist and @FedeDP

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

incertum · 2024-06-27T23:47:25Z

/milestone 0.18.0

incertum · 2024-06-27T23:49:09Z

userspace/libsinsp/ifinfo.h

@@ -41,6 +41,8 @@ class SINSP_PUBLIC sinsp_ipv4_ifinfo
 	sinsp_ipv4_ifinfo(uint32_t addr, uint32_t netmask, uint32_t bcast, const char* name);

 	std::string to_string() const;
+	std::string addr_to_string(const uint32_t addr) const;


The existing to_string method creates a string not suitable for the metrics feature. While here thought it would be good to expose an overloaded method in case you wanted to just convert for example the netmask to a string.

github-actions · 2024-06-27T23:51:43Z

Perf diff from master - unit tests

     4.41%     +1.89%  [.] sinsp_thread_manager::find_thread
     7.65%     -1.21%  [.] sinsp::next
     4.25%     -1.09%  [.] sinsp_evt::load_params
     0.11%     +0.96%  [.] scap_event_has_large_payload
     1.62%     -0.92%  [.] sinsp_parser::event_cleanup
     3.45%     +0.91%  [.] sinsp_parser::process_event
     4.18%     -0.88%  [.] sinsp_thread_manager::get_thread_ref
    10.07%     +0.85%  [.] sinsp_parser::reset
     1.40%     +0.70%  [.] 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> >::_M_find_before_node
     1.40%     -0.53%  [.] sinsp_evt::get_ts

Perf diff from master - scap file

    12.52%     -5.75%  [.] sinsp_thread_manager::get_thread_ref
    19.52%     -4.87%  [.] sinsp_filter_check::extract_nocache
     6.43%     +4.87%  [.] sinsp_evt_formatter::tostring_withformat
     3.19%     +4.31%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>
     3.27%     +2.98%  [.] sinsp_filter_check_event::extract_single
     9.71%     -2.45%  [.] sinsp_filter_check::apply_transformers
     6.56%     -2.23%  [.] 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> >::_M_find_before_node
     3.28%     +1.35%  [.] sinsp_filter_check::rawval_to_string
     9.78%     +0.95%  [.] sinsp_filter_check::tostring
     3.27%     +0.44%  [.] sinsp_evt::get_param_as_str

Heap diff from master - unit tests

total runtime: 0.04s.
calls to allocation functions: -695 (-18783/s)
temporary memory allocations: -207 (-5594/s)
peak heap memory consumption: -800B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

total runtime: -0.00s.
calls to allocation functions: 0 (0/s)
temporary memory allocations: 4 (-4000/s)
peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

FedeDP · 2024-06-28T06:30:30Z

userspace/libsinsp/ifinfo.cpp

@@ -69,6 +69,35 @@ std::string sinsp_ipv4_ifinfo::to_string() const
 	return std::string(s);
 }

+std::string sinsp_ipv4_ifinfo::addr_to_string(const uint32_t addr) const


I think this method could be made static instead!

FedeDP

/approve

poiana · 2024-07-01T07:07:02Z

LGTM label has been added.

Git tree hash: 9d34509b6ae4e23e09063a129bbf5b7d16b6cd3d

Signed-off-by: Melissa Kilby <[email protected]>

Signed-off-by: Melissa Kilby <[email protected]> Co-authored-by: Federico Di Pierro <[email protected]>

incertum · 2024-07-18T21:15:01Z

Rebased as the CI never triggered. What is needed to finish this PR? Thanks!

github-actions · 2024-07-18T21:20:36Z

Perf diff from master - unit tests

     6.63%     +1.04%  [.] sinsp::next
     5.41%     -0.87%  [.] next
     1.44%     -0.57%  [.] 0x00000000000e83b0
     0.84%     +0.55%  [.] sinsp_evt::get_ts
     0.37%     +0.54%  [.] scap_event_has_large_payload
     4.70%     +0.52%  [.] sinsp_evt::get_type
     0.83%     -0.52%  [.] sinsp_fdtable::find_ref
     1.10%     -0.51%  [.] sinsp_evt::get_direction
     1.92%     -0.49%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>
     0.73%     +0.45%  [.] 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

Perf diff from master - scap file

    15.12%     -9.53%  [.] sinsp_evt_formatter::tostring_withformat
     8.58%     -4.47%  [.] sinsp_filter_check_thread::extract_single
     8.23%     -4.23%  [.] 0x00000000000a77e0
     4.32%     +3.39%  [.] 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> >::_M_find_before_node
     6.45%     -2.83%  [.] sinsp_filter_check::tostring
     4.30%     +2.62%  [.] sinsp_thread_manager::find_thread
     2.10%     +2.22%  [.] sinsp_thread_manager::get_thread_ref
     6.37%     +1.37%  [.] gzfile_read
     6.42%     +1.33%  [.] sinsp_filter_check::extract_nocache
     4.22%     -1.23%  [.] sinsp_filter_check::get_transformed_field_info

Heap diff from master - unit tests

total runtime: 0.05s.
calls to allocation functions: -808 (-15843/s)
temporary memory allocations: -310 (-6078/s)
peak heap memory consumption: -800B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

Heap diff from master - scap file

total runtime: -0.00s.
calls to allocation functions: 0 (0/s)
temporary memory allocations: -9 (2250/s)
peak heap memory consumption: 0B
peak RSS (including heaptrack overhead): 0B
total memory leaked: 0B

FedeDP

/approve

poiana · 2024-07-19T08:01:08Z

LGTM label has been added.

Git tree hash: 24cf9dca4f5c1b057ac1ce60cecace9850a63d74

poiana · 2024-07-19T08:02:58Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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,incertum]

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

incertum · 2024-08-08T06:20:35Z

Would there be feedback from the second reviewer? Thanks.

poiana added release-note-none kind/cleanup dco-signoff: yes kind/feature New feature or request area/libsinsp labels Jun 27, 2024

poiana requested review from hbrueckner and mstemm June 27, 2024 23:47

poiana added the approved label Jun 27, 2024

poiana added this to the 0.18.0 milestone Jun 27, 2024

poiana added the size/M label Jun 27, 2024

incertum commented Jun 27, 2024

View reviewed changes

incertum mentioned this pull request Jun 28, 2024

new(metrics): add host_ifinfo metric falcosecurity/falco#3253

Merged

FedeDP reviewed Jun 28, 2024

View reviewed changes

incertum force-pushed the ip-to-string-public branch from 71ece5e to b707a74 Compare June 28, 2024 18:54

FedeDP approved these changes Jul 1, 2024

View reviewed changes

poiana assigned FedeDP Jul 1, 2024

poiana added the lgtm label Jul 1, 2024

incertum and others added 2 commits July 18, 2024 21:13

update(sinsp/ifinfo): add new public addr_to_string methods

fc4d0cb

Signed-off-by: Melissa Kilby <[email protected]>

update(sinsp/ifinfo): make overloaded addr_to_string static

8bf2f59

Signed-off-by: Melissa Kilby <[email protected]> Co-authored-by: Federico Di Pierro <[email protected]>

incertum force-pushed the ip-to-string-public branch from b707a74 to 8bf2f59 Compare July 18, 2024 21:14

poiana removed the lgtm label Jul 18, 2024

poiana requested a review from FedeDP July 18, 2024 21:14

FedeDP approved these changes Jul 19, 2024

View reviewed changes

poiana added the lgtm label Jul 19, 2024

jasondellaluce approved these changes Aug 19, 2024

View reviewed changes

poiana assigned jasondellaluce Aug 19, 2024

poiana merged commit f19c71a into falcosecurity:master Aug 19, 2024
42 of 43 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

update(sinsp/ifinfo): add new public addr_to_string methods #1937

update(sinsp/ifinfo): add new public addr_to_string methods #1937

incertum commented Jun 27, 2024

incertum commented Jun 27, 2024

incertum Jun 27, 2024

github-actions bot commented Jun 27, 2024

FedeDP Jun 28, 2024

FedeDP left a comment

poiana commented Jul 1, 2024

incertum commented Jul 18, 2024

github-actions bot commented Jul 18, 2024

FedeDP left a comment

poiana commented Jul 19, 2024

poiana commented Jul 19, 2024

incertum commented Aug 8, 2024

update(sinsp/ifinfo): add new public addr_to_string methods #1937

update(sinsp/ifinfo): add new public addr_to_string methods #1937

Conversation

incertum commented Jun 27, 2024

incertum commented Jun 27, 2024

incertum Jun 27, 2024

Choose a reason for hiding this comment

github-actions bot commented Jun 27, 2024

Perf diff from master - unit tests

Perf diff from master - scap file

Heap diff from master - unit tests

Heap diff from master - scap file

FedeDP Jun 28, 2024

Choose a reason for hiding this comment

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented Jul 1, 2024

incertum commented Jul 18, 2024

github-actions bot commented Jul 18, 2024

Perf diff from master - unit tests

Perf diff from master - scap file

Heap diff from master - unit tests

Heap diff from master - scap file

FedeDP left a comment

Choose a reason for hiding this comment

poiana commented Jul 19, 2024

poiana commented Jul 19, 2024

incertum commented Aug 8, 2024