Skip to content

fix wrong SizeOfTCPInfo #643

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

fix wrong SizeOfTCPInfo #643

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pkg/abi/linux/socket.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ type TCPInfo struct {
}

// SizeOfTCPInfo is the binary size of a TCPInfo struct.
const SizeOfTCPInfo = 104
const SizeOfTCPInfo = 192
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reference in the linux source code or something like that that determines this size?

Copy link
Contributor

Choose a reason for hiding this comment

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

On my system:
sizeof(struct tcp_info) = 104

I think the problem is our TCPInfo struct, not this constant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO, even we got 104 bytes from system, this still breaks the Unmarshal as we have a larger size of TCPInfo struct.

According to kernel (https://github.com/torvalds/linux/blob/master/net/ipv4/tcp.c#L3431), and ss (https://github.com/sivasankariit/iproute2/blob/master/misc/ss.c#L1378), it seems that we shall have a similar workaround.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe that 104 bytes is the new larger size and that struct tcp_info was previously smaller. It would appear that the problem is that our TCPInfo struct has a bunch of extra fields.

Copy link
Contributor

Choose a reason for hiding this comment

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

Nevermind. It looks like the struct was correct at a time and that the newest one is even larger still. What really confuses me is that version in my glibc seems to have fields missing from the middle as well. It appears that they added an extra field based on the assumption that it wouldn't change the size due to the space previously having been there as padding.

The code change looks good to me.

Could you please also:
Add a test which verifies that this constant contains the correct value.
Add a comment that this struct grew before this snapshot of it was take and has grown since.
Add a comment that this snapshot of the struct is from Linux 4.18.

Copy link
Contributor

Choose a reason for hiding this comment

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

I effectively merged this in 691c2f8

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I effectively merged this in 691c2f8

Would you mind adding the comments as suggested by Ian (see above)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Done. 6cfc767


// Control message types, from linux/socket.h.
const (
Expand Down