mirror/xdp-project-bpf-examples
1
0
Fork 0
mirror of https://github.com/xdp-project/bpf-examples.git synced 2024-05-06 15:54:53 +00:00
Code Issues Projects Releases Wiki Activity

Merge pull request #54 from simosund/pping-fix-reorder-issue

Browse Source 
PPing fix reorder issue
This commit is contained in:
Toke Høiland-Jørgensen
2022-10-11 00:56:08 +02:00
committed by GitHub
parent 619adfb6b5 867b659534
commit 7a9db7b08c
2 changed files with 16 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

23
pping/TODO.md
View File

@@ -202,29 +202,6 @@ concurrent packets have different identifiers there may be a lost
update (but for TCP timestamps, concurrent packets would typically be
expected to have the same timestamp).
A possibly more severe issue is out-of-order packets. If a packet with
an old identifier arrives out of order, that identifier could be
detected as a new identifier. If for example the following flow of
four packets with just two different identifiers (id1 and id2) were to
occur:
id1 -> id2 -> id1 -> id2
Then the tc/egress program would consider each of these packets to
have new identifiers and try to create a new timestamp for each of
them if the sampling strategy allows it. However even if the sampling
strategy allows it, the (incorrect) creation of timestamps for id1 and
id2 the second time would only be successful in case the first
timestamps for id1 and id2 have already been matched against (and thus
deleted). Even if that is the case, they would only result in
reporting an incorrect RTT in case there are also new matches against
these identifiers.
This issue could be avoided entirely by requiring that new-id > old-id
instead of simply checking that new-id != old-id, as TCP timestamps
should monotonically increase. That may however not be a suitable
solution for other types of identifiers.
### Rate-limiting new timestamps
In the tc/egress program packets to timestamp are sampled by using a
per-flow rate-limit, which is enforced by storing when the last

19
pping/pping_kern.c
View File

@@ -309,8 +309,8 @@ static int parse_tcp_ts(struct tcphdr *tcph, void *data_end, __u32 *tsval,
if (opt == 8 && opt_size == 10) {
if (pos + 10 > opt_end || pos + 10 > data_end)
return -1;
*tsval = *(__u32 *)(pos + 2);
*tsecr = *(__u32 *)(pos + 6);
*tsval = bpf_ntohl(*(__u32 *)(pos + 2));
*tsecr = bpf_ntohl(*(__u32 *)(pos + 6));
return 0;
}
@@ -885,6 +885,19 @@ static bool is_local_address(struct packet_info *p_info, void *ctx)
ret == BPF_FIB_LKUP_RET_FWD_DISABLED;
}
static bool is_new_identifier(struct packet_id *pid, struct flow_state *f_state)
{
if (pid->flow.proto == IPPROTO_TCP)
/* TCP timestamps should be monotonically non-decreasing
* Check that pid > last_ts (considering wrap around) by
* checking 0 < pid - last_ts < 2^31 as specified by
* RFC7323 Section 5.2*/
return pid->identifier - f_state->last_id > 0 &&
pid->identifier - f_state->last_id < 1UL << 31;
return pid->identifier != f_state->last_id;
}
/*
* Attempt to create a timestamp-entry for packet p_info for flow in f_state
*/
@@ -899,7 +912,7 @@ static void pping_timestamp_packet(struct flow_state *f_state, void *ctx,
return;
// Check if identfier is new
if (!new_flow && f_state->last_id == p_info->pid.identifier)
if (!new_flow && !is_new_identifier(&p_info->pid, f_state))
return;
f_state->last_id = p_info->pid.identifier;