Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix tcp/connection/accepted|initiated metrics for kernel 5.10 #266

Merged

Conversation

WUMUXIAN
Copy link
Contributor

@WUMUXIAN WUMUXIAN commented Jan 5, 2022

Problem

We have deployed Rezolus 2.15 on T3 canary instances and it showed that
tcp/connection/accepted
tcp/connection/initiated
are not working

I upgraded my VM to the same kernel version as T3 instances and confirmed that these two metrics stops working on kernel 5.10.

Solution

Made changes to the code to make sure these two metrics are compatible to 5.10.

Result

The two metrics work correctly on my 5.4 kernel as well as 5.10 kernel now.

@@ -206,7 +206,7 @@ impl TcpStatistic {
name: "tcp_finish_connect".to_string(),
handler: "trace_finish_connect".to_string(),
probe_type: ProbeType::Kernel,
probe_location: ProbeLocation::Return,
probe_location: ProbeLocation::Entry,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Kernel 5.4, it does not matter if we attach this at Entry or Return.
However on Kernel 5.10, if we attach it to Return, the sock* sk can't be used to get the stats from the map as the address are moved 128 bits. Now sure about why.
But using Entry here fix the issue.

@WUMUXIAN WUMUXIAN changed the title Muxianw/fix connection io metrics for 5.10 Fix tcp/connection/accepted|initiated metrics for kernel 5.10 Jan 5, 2022
Copy link
Contributor

@brayniac brayniac left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@brayniac brayniac merged commit b092048 into twitter:master Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants