retina icon indicating copy to clipboard operation
retina copied to clipboard

Don't check endieness on the host

Open anubhabMajumdar opened this issue 1 year ago • 6 comments

Describe the bug Currently Retina checks host endianness to enrich port/IP addresses. That is not correct, network is big-endian. Handle conversion in bpf code.

To Reproduce

Ref: https://github.com/microsoft/retina/blob/61e115278c478191910d3038e07e7137a96b65d2/pkg/utils/utils_linux.go#L78

Expected behavior Network is big-endian. Handle conversion as such bpf code.

Platform (please complete the following information):

  • OS: Linux
  • Kubernetes Version: N/A
  • Host: Linux/all-arch
  • Retina Version: v0.0.1

anubhabMajumdar avatar Mar 26 '24 15:03 anubhabMajumdar

@hainenber can you comment here if you want this issue assigned to you also?

rbtr avatar Mar 27 '24 19:03 rbtr

Yes, please :D

hainenber avatar Mar 28 '24 09:03 hainenber

What we want is to handle host to network conversion in bpf code and not handle it in userspace, making the code more efficient. Ideally, we won't need HostToNetwork and IntToIp won't need endianness check.

Some references for issue owner to consider:

  • https://mozillazg.com/2022/07/ebpf-libbpf-what-is-load_byte-load_half-load_word-en.html (load_half and load_word)

anubhabMajumdar avatar Apr 01 '24 14:04 anubhabMajumdar

Oh that wasn't what I expect at first 😆 . I was just thinking the conversion is already somewhere in eBPF codebase.

Thanks for the lead, I'll try getting my hands wet on this one as I didn't work on low-level code before :D

hainenber avatar Apr 01 '24 14:04 hainenber

Hand-rolled byte order swap for IPv4 (32-bit addresses):

uint32_t s = 0x80010000;
s = (s & 0x0000FFFF) << 16 | (s & 0xFFFF0000) >> 16;
s = (s & 0x00FF00FF) << 8 | (s & 0xFF00FF00) >> 8; 
cout << setw(8) << setfill('0') << s << endl;

rectified95 avatar Apr 11 '24 22:04 rectified95

Hand-rolled byte order swap for IPv4 (32-bit addresses):

uint32_t s = 0x80010000;
s = (s & 0x0000FFFF) << 16 | (s & 0xFFFF0000) >> 16;
s = (s & 0x00FF00FF) << 8 | (s & 0xFF00FF00) >> 8; 
cout << setw(8) << setfill('0') << s << endl;

@rectified95 maybe you can pick this up now? 🙂

rbtr avatar May 15 '24 23:05 rbtr

just want to dust off this issue, when we are getting the src and dst ips from the iphdr image they might already are big-endian? image so we might not need to do conversation at all? correct me if im wrong 🙂 @anubhabMajumdar

nddq avatar May 31 '24 18:05 nddq

@nddq Yes, those values are always big-endian and need to be converted to little-endian so they can be used by the hosts. I'm working on this now. The question is whether doing this simple operation in bpf is much faster than in Go, but it def wouldn't hurt.

rectified95 avatar May 31 '24 18:05 rectified95