Skip to content

Commit 2e7595b

Browse files
committed
pping: Replace boolean connection state flags with enum
The connection state had 3 boolean flags related to what state it was in (is_empty, has_opened and has_closed). Only specific combinations of these flags really made sense (has_opened/has_closed didn't really mean anything if is_empty, and if has_closed one would expect is_empty to be false and has_opened to be true etc.). Therefore, replace these combinations of boolean values with a singular enum which is used to check if the flow is empty, waiting to open (seen outgoing packet but no response), is open or has closed. Signed-off-by: Simon Sundberg <simon.sundberg@kau.se>
1 parent 4fb1f6d commit 2e7595b

File tree

2 files changed

+22
-15
lines changed

2 files changed

+22
-15
lines changed

pping/pping.h

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,13 @@ enum __attribute__((__packed__)) pping_map {
5050
PPING_MAP_PACKETTS
5151
};
5252

53+
enum __attribute__((__packed__)) connection_state {
54+
CONNECTION_STATE_EMPTY,
55+
CONNECTION_STATE_WAITOPEN,
56+
CONNECTION_STATE_OPEN,
57+
CONNECTION_STATE_CLOSED
58+
};
59+
5360
struct bpf_config {
5461
__u64 rate_limit;
5562
fixpoint64 rtt_rate;
@@ -96,11 +103,9 @@ struct flow_state {
96103
__u64 rec_bytes;
97104
__u32 last_id;
98105
__u32 outstanding_timestamps;
99-
bool has_opened;
106+
enum connection_state conn_state;
100107
enum flow_event_reason opening_reason;
101-
bool is_empty;
102-
bool has_closed;
103-
__u32 reserved;
108+
__u8 reserved[6];
104109
};
105110

106111
/*

pping/pping_kern.c

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -645,17 +645,16 @@ static void send_map_full_event(void *ctx, struct packet_info *p_info,
645645
static void init_flowstate(struct flow_state *f_state,
646646
struct packet_info *p_info)
647647
{
648-
f_state->is_empty = false;
648+
f_state->conn_state = CONNECTION_STATE_WAITOPEN;
649649
f_state->last_timestamp = p_info->time;
650-
f_state->has_opened = false;
651650
f_state->opening_reason = p_info->event_type == FLOW_EVENT_OPENING ?
652651
p_info->event_reason :
653652
EVENT_REASON_FIRST_OBS_PCKT;
654653
}
655654

656655
static void init_empty_flowstate(struct flow_state *f_state)
657656
{
658-
f_state->is_empty = true;
657+
f_state->conn_state = CONNECTION_STATE_EMPTY;
659658
}
660659

661660
/*
@@ -716,14 +715,16 @@ lookup_or_create_dualflow_state(void *ctx, struct packet_info *p_info,
716715

717716
static bool is_flowstate_active(struct flow_state *f_state)
718717
{
719-
return !f_state->is_empty && !f_state->has_closed;
718+
return f_state->conn_state != CONNECTION_STATE_EMPTY &&
719+
f_state->conn_state != CONNECTION_STATE_CLOSED;
720720
}
721721

722722
static void update_forward_flowstate(struct packet_info *p_info,
723723
struct flow_state *f_state, bool *new_flow)
724724
{
725725
// "Create" flowstate if it's empty
726-
if (f_state->is_empty && p_info->pid_valid) {
726+
if (f_state->conn_state == CONNECTION_STATE_EMPTY &&
727+
p_info->pid_valid) {
727728
init_flowstate(f_state, p_info);
728729
if (new_flow)
729730
*new_flow = true;
@@ -742,9 +743,9 @@ static void update_reverse_flowstate(void *ctx, struct packet_info *p_info,
742743
return;
743744

744745
// First time we see reply for flow?
745-
if (!f_state->has_opened &&
746+
if (f_state->conn_state == CONNECTION_STATE_WAITOPEN &&
746747
p_info->event_type != FLOW_EVENT_CLOSING_BOTH) {
747-
f_state->has_opened = true;
748+
f_state->conn_state = CONNECTION_STATE_OPEN;
748749
send_flow_open_event(ctx, p_info, f_state);
749750
}
750751

@@ -754,7 +755,7 @@ static void update_reverse_flowstate(void *ctx, struct packet_info *p_info,
754755

755756
static bool should_notify_closing(struct flow_state *f_state)
756757
{
757-
return f_state->has_opened && !f_state->has_closed;
758+
return f_state->conn_state == CONNECTION_STATE_OPEN;
758759
}
759760

760761
static void close_and_delete_flows(void *ctx, struct packet_info *p_info,
@@ -766,14 +767,14 @@ static void close_and_delete_flows(void *ctx, struct packet_info *p_info,
766767
p_info->event_type == FLOW_EVENT_CLOSING_BOTH) {
767768
if (should_notify_closing(fw_flow))
768769
send_flow_event(ctx, p_info, false);
769-
fw_flow->has_closed = true;
770+
fw_flow->conn_state = CONNECTION_STATE_CLOSED;
770771
}
771772

772773
// Reverse flow closing
773774
if (p_info->event_type == FLOW_EVENT_CLOSING_BOTH) {
774775
if (should_notify_closing(rev_flow))
775776
send_flow_event(ctx, p_info, true);
776-
rev_flow->has_closed = true;
777+
rev_flow->conn_state = CONNECTION_STATE_CLOSED;
777778
}
778779

779780
// Delete flowstate entry if neither flow is open anymore
@@ -956,7 +957,8 @@ static bool is_flow_old(struct network_tuple *flow, struct flow_state *f_state,
956957
return false;
957958
age = time - ts;
958959

959-
return (!f_state->has_opened && age > UNOPENED_FLOW_LIFETIME) ||
960+
return (f_state->conn_state == CONNECTION_STATE_WAITOPEN &&
961+
age > UNOPENED_FLOW_LIFETIME) ||
960962
((flow->proto == IPPROTO_ICMP ||
961963
flow->proto == IPPROTO_ICMPV6) &&
962964
age > ICMP_FLOW_LIFETIME) ||

0 commit comments

Comments
 (0)