From 11f1d88742dae184c4ee67a2129b6efa9a552fac Mon Sep 17 00:00:00 2001 From: Simon Sundberg Date: Wed, 9 Mar 2022 22:19:59 +0100 Subject: [PATCH 1/2] pping: Keep track of outstanding timestamps Add a counter of outstanding (unmatched) timestamped entires in the flow state. Before a timestamp lookup is attempted, check that there are any outstanding timestamps, otherwise avoid the unecessary hash map lookup. Use 32 bit counter for outstanding timestamps to allow atomic increments/decrements using __synch_fetch_and_add. This operation is not supported on smaller integers, which is why such a large counter is used. The atomicity is needed because the counter may be concurrently accessed by both the ingress/egress hook as well as the periodical map cleanup. Signed-off-by: Simon Sundberg --- pping/pping.h | 3 ++- pping/pping_kern.c | 18 +++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/pping/pping.h b/pping/pping.h index ad2b6a5..e923f45 100644 --- a/pping/pping.h +++ b/pping/pping.h @@ -94,9 +94,10 @@ struct flow_state { __u64 rec_pkts; __u64 rec_bytes; __u32 last_id; + __u32 outstanding_timestamps; bool has_opened; enum flow_event_reason opening_reason; - __u16 reserved; + __u8 reserved[6]; }; struct packet_id { diff --git a/pping/pping_kern.c b/pping/pping_kern.c index fb9fb0b..c391b10 100644 --- a/pping/pping_kern.c +++ b/pping/pping_kern.c @@ -687,7 +687,9 @@ static void pping_timestamp_packet(struct flow_state *f_state, void *ctx, f_state->last_timestamp = p_info->time; if (bpf_map_update_elem(&packet_ts, &p_info->pid, &p_info->time, - BPF_NOEXIST) != 0) + BPF_NOEXIST) == 0) + __sync_fetch_and_add(&f_state->outstanding_timestamps, 1); + else send_map_full_event(ctx, p_info, PPING_MAP_PACKETTS); } @@ -704,6 +706,9 @@ static void pping_match_packet(struct flow_state *f_state, void *ctx, if (!f_state || !p_info->reply_pid_valid) return; + if (f_state->outstanding_timestamps == 0) + return; + p_ts = bpf_map_lookup_elem(&packet_ts, &p_info->reply_pid); if (!p_ts || p_info->time < *p_ts) return; @@ -711,8 +716,10 @@ static void pping_match_packet(struct flow_state *f_state, void *ctx, re.rtt = p_info->time - *p_ts; // Delete timestamp entry as soon as RTT is calculated - if (bpf_map_delete_elem(&packet_ts, &p_info->reply_pid) == 0) + if (bpf_map_delete_elem(&packet_ts, &p_info->reply_pid) == 0) { + __sync_fetch_and_add(&f_state->outstanding_timestamps, -1); debug_increment_autodel(PPING_MAP_PACKETTS); + } if (f_state->min_rtt == 0 || re.rtt < f_state->min_rtt) f_state->min_rtt = re.rtt; @@ -836,8 +843,13 @@ int tsmap_cleanup(struct bpf_iter__bpf_map_elem *ctx) if ((rtt && now - *timestamp > rtt * TIMESTAMP_RTT_LIFETIME) || now - *timestamp > TIMESTAMP_LIFETIME) { - if (bpf_map_delete_elem(&packet_ts, &local_pid) == 0) + if (bpf_map_delete_elem(&packet_ts, &local_pid) == 0) { debug_increment_timeoutdel(PPING_MAP_PACKETTS); + + if (f_state) + __sync_fetch_and_add( + &f_state->outstanding_timestamps, -1); + } } return 0; From 8c0084dd0e35103133e81b6141455c3410ba63d5 Mon Sep 17 00:00:00 2001 From: Simon Sundberg Date: Thu, 10 Mar 2022 10:33:18 +0100 Subject: [PATCH 2/2] pping: Update TODO about timestamp ref count Signed-off-by: Simon Sundberg --- pping/TODO.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pping/TODO.md b/pping/TODO.md index 588659d..d0e729a 100644 --- a/pping/TODO.md +++ b/pping/TODO.md @@ -66,6 +66,9 @@ - Keeping entries around for a long time allows the map to grow unnecessarily large, which slows down the cleaning and may block new entries +- [x] Keep track of outstanding timestamps, only match when necessary + - Can avoid doing lookups in timestamp hash map if we know that + there are no outstanding (unmatched) timestamps for the flow # Potential issues