Add TCP out of order or duplicate segments sampler via BPF #255
Conversation
src/samplers/tcp/stat.rs
Outdated
| binary_path: None, | ||
| sub_system: None, | ||
| }; | ||
| let tcp_ooo_probe = Probe { |
There was a problem hiding this comment.
this probe is identical as the above one. We can just introduce a probe called
tcp_validate_incoming_probe and attach it to both telemetries
Self::Duplicate => vec![tcp_validate_incoming_probe],
Self::OutOfOrder => vec![tcp_validate_incoming_probe],
There was a problem hiding this comment.
Cool, I was wondering if I could do that :)
src/samplers/tcp/bpf.c
Outdated
|
|
||
| // Segment sequence before the expected one | ||
| // which means this was a duplicated segment | ||
| if ((end_seq - tp->rcv_wup) < 0) { |
There was a problem hiding this comment.
I feel the starting sequence number TCP_SKB_CB(skb)->seq and the expected sequence number is tp->rcv_nxt is more straightforward to compare
if TCP_SKB_CB(skb)->seq is smaller than tp->rcv_nxt, it's confirmed a duplicate.
There was a problem hiding this comment.
'rcv_wup' records the rcv_nxt on last window update sent, comparing it to the end_seq of this segment does not seem correct to me.
There was a problem hiding this comment.
I'm basing these on this function that is used to validate the segment sequence on the kernel:
https://elixir.bootlin.com/linux/v4.2/source/net/ipv4/tcp_input.c#L3902
From the comments it states:
Also, controls (RST is main one) are accepted using RCV.WUP instead
of RCV.NXT. Peer still did not advance his SND.UNA when we
delayed ACK, so that hisSND.UNA<=ourRCV.WUP.
So to validate it seems better if we use RCV.WUP. But maybe for probing duplicates we don't care about this and should use RCV.NXT anyways?
There was a problem hiding this comment.
I think what
!before(end_seq, tp->rcv_wup) &&
!after(seq, tp->rcv_nxt + tcp_receive_window(tp))
checks eventually is that whether the sender's segment [seq, end_seq] has any overlays with the expected window at all.
The first check finds out if the end_seq is even before the previous rcv_nxt sent, the second check finds out if the start seq is even after the largest possible seq (which is rcv_nxt + window size).
For us we simply wants to check if the segment is a duplicate, and checking end_seq - tp->rcv_wup can't help, for example, when (end_seq - tp->rcv_wup) <0, we can confirm that it's a duplicate, but when (end_seq - tp->rcv_wup) > 0, it can still be a duplicate, as long as seq < tp->rcv_nxt.
So basically to check duplicates, we should be looking seq and tp->rct_nxt. What do you think?
There was a problem hiding this comment.
Yes, this makes sense. I made the changes to use RCV.NXT instead.
src/samplers/tcp/bpf.c
Outdated
| // Segment sequence after the expected receive window | ||
| // which means this segment was received out of order | ||
| u32 window_end = tp->rcv_nxt + tcp_receive_window(tp); | ||
| if ((window_end - seq) < 0) { |
There was a problem hiding this comment.
Similarly here, I think we can simply check whether if seq > tp->rcv_nxt here to see if the segment arrived out of order.
window_end < seq only tell that the current segment is totally out of the window, but when there's an overlay, it can't capture.
|
This LGTM from a Rust perspective. I think it can be marked as ready for review at this point? @WUMUXIAN - please give a shipit for the BCC portion when you're happy with that and I'll give it another look before merging. |
|
@brayniac feel free to merge and close this PR when possible. Thanks! |
Problem
Add two new TCP probes:
Solution
Added two BPF kernel probes to count du and RTO events.
Result
Two new metrics introduced to TCP sampler: