Add an eBPF program to measure synchronous connect() calls latencies #254
Conversation
| }; | ||
|
|
||
| struct { | ||
| __uint(type, BPF_MAP_TYPE_HASH); |
There was a problem hiding this comment.
BPF_MAP_TYPE_LRU_HASH is preferred for resiliency against map getting full.
| metrics: | ||
| histograms: | ||
| - name: connect_latency_seconds | ||
| help: Latency histogram for TCP connect() syscall |
There was a problem hiding this comment.
If it's TCP only, then let's call the metric tcp_connect_latency_seconds and rename the file into tcp-connect-latency.yaml.
There was a problem hiding this comment.
Since we do this:
// Filter out non-blocking sockets and errors
It's probably a good idea to add blocking in the name as well.
| __type(value, u64); | ||
| } connect_latency_seconds SEC(".maps"); | ||
|
|
||
| static inline __u16 ntohs(__u16 value) { |
There was a problem hiding this comment.
Any reason not to use bpf_ntohs?
|
|
||
| bpf_probe_read(&sa, sizeof(sa), addr); | ||
|
|
||
| if (sa.sa_family == 1) { |
There was a problem hiding this comment.
There are lots of families and you probably only care about AF_INET:
I suggest you add #define AF_INET 2 and use it here.
| return ((value & 0x00FF) << 8) | ((value & 0xFF00) >> 8); | ||
| } | ||
|
|
||
| SEC("kprobe/__sys_connect") |
There was a problem hiding this comment.
fentry is a lot faster:
You also get:
- Function arguments in
fexit, allowing you to key onaddrrather than pid - No need for
BPF_CORE_READas BTF allows direct reads
| return 0; | ||
| } | ||
| } else { | ||
| const char debug_str[] = "Unexpected addrlen: %d, address family: %d\n"; |
There was a problem hiding this comment.
We generally don't leave debug statements around.
There was a problem hiding this comment.
I would recommend bpf_printk() when you debug locally:
| start_val.d_ip = BPF_CORE_READ(&v6.sin6_addr.in6_u, u6_addr32[3]); | ||
| start_val.d_port = v6.sin6_port; | ||
| } else { | ||
| const char debug_str[] = "This is native ipv6, I'm giving up!\n"; |
There was a problem hiding this comment.
Please implement IPv6 as well. Normally it's in a separate map: #251.
| } | ||
| const char debug_str[] = "Return code is: %d\n"; | ||
| bpf_trace_printk(debug_str, sizeof(debug_str), ret); | ||
| struct connect_latency_key_t key = {}; |
There was a problem hiding this comment.
Please put all definitions at the top of the function.
| struct connect_start_val_t { | ||
| u64 ts; | ||
| int addrlen; | ||
| u32 d_ip; // Destination IPv4 address |
There was a problem hiding this comment.
clang-format is not happy here
|
|
||
| struct connect_start_val_t { | ||
| u64 ts; | ||
| int addrlen; |
There was a problem hiding this comment.
You aren't really using addrlen.
No description provided.