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

this fixes the net.IP from /proc/net/udp* being broken because they a… #336

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hubt
Copy link
Contributor

@hubt hubt commented Oct 10, 2020

…re in network byte order, not host byte order

While testing and working on #332 I discovered a more serious problem with the implementation of the NetUDP reader code. It has a byte order problem.

/proc/net/udp{,6} have the IP addresses in network byte order. The current code does not correctly transform from network byte order to host byte order, so the LocalAddr and RemAddr are stored improperly.

I wrote this patch to fix that, but with a big caveat: it assumes little endian order like intel/amd architectures. I could write generic code to handle both big and little endian, but I would have no way to test it. If someone with a big-endian environment wanted to take that on, I could help.

For ipv4, I simply reverse the bytes. For ipv6 it's more complicated because the address is four words consisting of four bytes each. In each of those four words, the four bytes are written in reverse order.

The reason that the tests were passing was because the tests were incorrect. For ipv4, I updated the fixture for /proc/net/udp to match what the test 10.0.0.5 address would really generate. For ipv6, I kept the existing fixture but fixed the test comparison address to fe80::56e1:adff:fe7c:6609.

If someone could take a look and validate all of this that'd be good.

I will also fix #332 that I was originally working on to add in a similar patch, once someone validates this.

…re in network byte order, not host byte order
@@ -147,6 +147,33 @@ func newNetUDPSummary(file string) (*NetUDPSummary, error) {
return netUDPSummary, nil
}

// NOTE: this function assumes the architecture is little endian, like x86 and amd64
Copy link
Member

Choose a reason for hiding this comment

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

I'm not really comfortable assuming little endian here unless the kernel is consistent about the file format.

I have systems with big endian (mips64).

Copy link
Member

Choose a reason for hiding this comment

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

This is what chatgpt recommends, seems reasonable: https://chat.openai.com/share/2586e0df-70b5-4b54-a55a-29a60e4b689d

func IsLittleEndian() bool {
    var testValue uint16 = 0xABCD
    testBytes := make([]byte, 2)
    binary.LittleEndian.PutUint16(testBytes, testValue)
    return testBytes[0] == 0xCD
}

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