solisten: clean endian conversion hack

[yadutaf]

I checked http:https://lxr.free-electrons.com/source/include/uapi/linux/swab.h#L57. If the compiler advertises its built-in byte swap methods, Linux headers will skip the arch specific inline assembly.

This still feels a bit hackish but a cleaner flavor.

How can we make this the default ?

[drzaeus77]

I think the swap in this case should be done in userspace. I will re-propose a patch I had in the original tool review.

[drzaeus77]

diff --git a/tools/solisten.py b/tools/solisten.py
index 6ff3d4d..5213dcd 100644
--- a/tools/solisten.py
+++ b/tools/solisten.py
@@ -23,6 +23,7 @@ import netaddr
 import argparse
 from bcc import BPF
 import ctypes as ct
+from struct import unpack

 # Arguments
 examples = """Examples:
@@ -51,18 +52,6 @@ bpf_text = """
 #include <net/net_namespace.h>
 #include <bcc/proto.h>

-// Endian conversion. We can't use kernel version here as it uses inline
-// assembly, neither libc version as we can't import it here. Adapted from both.
-#if defined(__LITTLE_ENDIAN)
-#define bcc_be32_to_cpu(x) ((u32)(__builtin_bswap32)((x)))
-#define bcc_be64_to_cpu(x) ((u64)(__builtin_bswap64)((x)))
-#elif defined(__BIG_ENDIAN)
-#define bcc_be32_to_cpu(x) (x)
-#define bcc_be64_to_cpu(x) (x)
-#else
-#error Host endianness not defined
-#endif
-
 // Common structure for UDP/TCP IPv4/IPv6
 struct listen_evt_t {
     u64 ts_us;
@@ -71,7 +60,7 @@ struct listen_evt_t {
     u64 netns;
     u64 proto;    // familiy << 16 | type
     u64 lport;    // use only 16 bits
-    u64 laddr[2]; // IPv4: store in laddr[0]
+    u8 rawaddr[16]; // IPv4: store in laddr[0]
     char task[TASK_COMM_LEN];
 };
 BPF_PERF_OUTPUT(listen_evt);
@@ -116,12 +105,9 @@ int kprobe__inet_listen(struct pt_regs *ctx, struct socket *sock, int backlog)

         // Get IP
         if (family == AF_INET) {
-            bpf_probe_read(evt.laddr, sizeof(u32), &(inet->inet_rcv_saddr));
-            evt.laddr[0] = bcc_be32_to_cpu(evt.laddr[0]);
+            bpf_probe_read(evt.rawaddr, sizeof(u32), &(inet->inet_rcv_saddr));
         } else if (family == AF_INET6) {
-            bpf_probe_read(evt.laddr, sizeof(evt.laddr), sk->__sk_common.skc_v6_rcv_saddr.in6_u.u6_addr32);
-            evt.laddr[0] = bcc_be64_to_cpu(evt.laddr[0]);
-            evt.laddr[1] = bcc_be64_to_cpu(evt.laddr[1]);
+            bpf_probe_read(evt.rawaddr, sizeof(evt.rawaddr), sk->__sk_common.skc_v6_rcv_saddr.in6_u.u6_addr32);
         }

         // Send event to userland
@@ -141,7 +127,7 @@ class ListenEvt(ct.Structure):
         ("netns", ct.c_ulonglong),
         ("proto", ct.c_ulonglong),
         ("lport", ct.c_ulonglong),
-        ("laddr", ct.c_ulonglong * 2),
+        ("rawaddr", ct.c_ubyte * 16),
         ("task", ct.c_char * TASK_COMM_LEN)
     ]

@@ -167,9 +153,9 @@ def event_printer(show_netns):
         address = ""
         if proto_type == socket.AF_INET:
             protocol += "v4"
-            address = netaddr.IPAddress(event.laddr[0])
+            address = netaddr.IPAddress(unpack(">I", bytes(event.rawaddr[:4]))[0])
         elif proto_type == socket.AF_INET6:
-            address = netaddr.IPAddress(event.laddr[0]<<64 | event.laddr[1], version=6)
+            address = netaddr.IPAddress(":".join(["%x"%x for x in unpack(">HHHHHHHH", event.rawaddr)]), version=6)
             protocol += "v6"

         # Display

[drzaeus77]

Note that the ">" in the unpack syntax specifies that the value is in big endian (network endian) ordering and should be converted if necessary. Whether this is a nop or a bswap python will correctly determine.

[yadutaf]

Which would save a couple of kernel side cycles. Sounds good.

[brendangregg]

FWIW, with the inet_ntoa support I'm working on, the byteswaps aren't necessary.

[yadutaf]

Do you want to wait for the inet_ntoa before merging ?

[4ast]

I think this is good as-is. @drzaeus77