Skip to content

Commit

Permalink
Refactor pmtud variables in struct
Browse files Browse the repository at this point in the history
  • Loading branch information
@kp-mariappan-ramasamy
kp-mariappan-ramasamy committed Jan 3, 2025
1 parent 360e30b commit 4f1f9d3
Show file tree
Hide file tree
Showing 9 changed files with 143 additions and 139 deletions.
6 changes: 3 additions & 3 deletions src/he/conn.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1213,7 +1213,7 @@ he_return_code_t he_conn_start_pmtu_discovery(he_conn_t *conn) {
if(conn->pmtud_state_change_cb == NULL || conn->pmtud_time_cb == NULL) {
return HE_ERR_PMTUD_CALLBACKS_NOT_SET;
}
if(conn->pmtud_state != HE_PMTUD_STATE_DISABLED) {
if(conn->pmtud.state != HE_PMTUD_STATE_DISABLED) {
// PMTUD is already started
return HE_SUCCESS;
}
Expand All @@ -1223,10 +1223,10 @@ he_return_code_t he_conn_start_pmtu_discovery(he_conn_t *conn) {
}

uint16_t he_conn_get_effective_pmtu(he_conn_t *conn) {
if(!conn || conn->effective_pmtu == 0) {
if(!conn || conn->pmtud.effective_pmtu == 0) {
return HE_MAX_MTU;
}
return conn->effective_pmtu;
return conn->pmtud.effective_pmtu;
}

he_return_code_t he_conn_pmtud_probe_timeout(he_conn_t *conn) {
Expand Down
4 changes: 2 additions & 2 deletions src/he/flow.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

bool he_internal_flow_should_fragment(he_conn_t *conn, uint16_t effective_pmtu, uint16_t length) {
return conn->connection_type == HE_CONNECTION_TYPE_DATAGRAM &&
conn->pmtud_state == HE_PMTUD_STATE_SEARCH_COMPLETE && length > effective_pmtu;
conn->pmtud.state == HE_PMTUD_STATE_SEARCH_COMPLETE && length > effective_pmtu;
}

he_return_code_t he_conn_inside_packet_received(he_conn_t *conn, uint8_t *packet, size_t length) {
Expand Down Expand Up @@ -70,7 +70,7 @@ he_return_code_t he_conn_inside_packet_received(he_conn_t *conn, uint8_t *packet
// Clamp the MSS if PMTU has been fixed
he_return_code_t ret = HE_SUCCESS;
uint16_t effective_pmtu = he_conn_get_effective_pmtu(conn);
if(conn->pmtud_state == HE_PMTUD_STATE_SEARCH_COMPLETE) {
if(conn->pmtud.state == HE_PMTUD_STATE_SEARCH_COMPLETE) {
ret = he_internal_clamp_mss(packet, length, effective_pmtu - HE_MSS_OVERHEAD);
if(ret != HE_SUCCESS) {
return ret;
Expand Down
28 changes: 16 additions & 12 deletions src/he/he_internal.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,21 @@ struct he_ssl_ctx {
size_t max_frag_entries;
};

typedef struct he_internal_pmtud_ {
/// Path MTU Discovery state
he_pmtud_state_t state;

/// Current effective PMTU
uint16_t effective_pmtu;

/// PMTUD internal data
uint16_t base;
uint8_t probe_count;
uint16_t probing_size;
bool is_using_big_step;
uint16_t probe_pending_id;
} he_internal_pmtud_t;

typedef struct he_fragment_table he_fragment_table_t;

struct he_conn {
Expand Down Expand Up @@ -258,18 +273,7 @@ struct he_conn {
/// Identifier of the ping message pending reply
uint16_t ping_pending_id;

/// Path MTU Discovery state
he_pmtud_state_t pmtud_state;

/// Current effective PMTU
uint16_t effective_pmtu;

/// PMTUD internal data
uint16_t pmtud_base;
uint8_t pmtud_probe_count;
uint16_t pmtud_probing_size;
bool pmtud_is_using_big_step;
uint16_t pmtud_probe_pending_id;
he_internal_pmtud_t pmtud;

/// UDP Fragmentation
_Atomic uint16_t frag_next_id;
Expand Down
2 changes: 1 addition & 1 deletion src/he/msg_handlers.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ he_return_code_t he_handle_msg_pong(he_conn_t *conn, uint8_t *packet, int length
he_internal_generate_event(conn, HE_EVENT_PONG);
return HE_SUCCESS;
}
if(id == conn->pmtud_probe_pending_id) {
if(id == conn->pmtud.probe_pending_id) {
// Received ack of a pmtud probe
he_internal_pmtud_handle_probe_ack(conn, id);
return HE_SUCCESS;
Expand Down
76 changes: 38 additions & 38 deletions src/he/pmtud.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ static void he_internal_pmtud_change_state(he_conn_t *conn, he_pmtud_state_t sta
if(conn->pmtud_state_change_cb) {
conn->pmtud_state_change_cb(conn, state, conn->data);
}
conn->pmtud_state = state;
conn->pmtud.state = state;
}

he_return_code_t he_internal_pmtud_send_probe(he_conn_t *conn, uint16_t probe_mtu) {
Expand Down Expand Up @@ -70,8 +70,8 @@ he_return_code_t he_internal_pmtud_send_probe(he_conn_t *conn, uint16_t probe_mt
uint16_t payload_size = probe_mtu + sizeof(he_msg_data_t) - sizeof(he_msg_ping_t);
probe->length = htons(payload_size);

conn->pmtud_probe_count++;
conn->pmtud_probing_size = probe_mtu;
conn->pmtud.probe_count++;
conn->pmtud.probing_size = probe_mtu;

// Send it
he_return_code_t res = he_internal_send_message(conn, buf, sizeof(he_msg_ping_t) + payload_size);
Expand All @@ -82,7 +82,7 @@ he_return_code_t he_internal_pmtud_send_probe(he_conn_t *conn, uint16_t probe_mt
timeout_ms = 10;
} else {
// Set the probe pending id for verifying the ack message
conn->pmtud_probe_pending_id = id;
conn->pmtud.probe_pending_id = id;
}

// Start the probe timer
Expand All @@ -97,7 +97,7 @@ he_return_code_t he_internal_pmtud_handle_probe_ack(he_conn_t *conn, uint16_t pr
if(!conn) {
return HE_ERR_NULL_POINTER;
}
if(conn->pmtud_probe_pending_id != probe_id) {
if(conn->pmtud.probe_pending_id != probe_id) {
// Receives an outdated probe id, ignore.
return HE_SUCCESS;
}
Expand All @@ -108,26 +108,26 @@ he_return_code_t he_internal_pmtud_handle_probe_ack(he_conn_t *conn, uint16_t pr
}

// Reset the pending probe id
conn->pmtud_probe_pending_id = 0;
conn->pmtud.probe_pending_id = 0;

// Reset the probe count for each ack
conn->pmtud_probe_count = 0;
conn->pmtud.probe_count = 0;

// Handle the ack based on current state
switch(conn->pmtud_state) {
switch(conn->pmtud.state) {
case HE_PMTUD_STATE_BASE:
// Base MTU confirmed
return he_internal_pmtud_base_confirmed(conn);
case HE_PMTUD_STATE_SEARCHING:
// Current probe acked
if(conn->pmtud_probing_size >= MAX_PLPMTU) {
if(conn->pmtud.probing_size >= MAX_PLPMTU) {
// Search completed if the MAX_PLPMTU acked
return he_internal_pmtud_search_completed(conn);
} else {
// Increment probing size and send next probe
uint16_t probe_size = conn->pmtud_probing_size;
uint16_t probe_size = conn->pmtud.probing_size;
probe_size +=
(conn->pmtud_is_using_big_step) ? PMTUD_PROBE_BIG_STEP : PMTUD_PROBE_SMALL_STEP;
(conn->pmtud.is_using_big_step) ? PMTUD_PROBE_BIG_STEP : PMTUD_PROBE_SMALL_STEP;
probe_size = MIN(probe_size, MAX_PLPMTU);
he_internal_pmtud_send_probe(conn, probe_size);
}
Expand All @@ -151,18 +151,18 @@ he_return_code_t he_internal_pmtud_handle_probe_timeout(he_conn_t *conn) {
}

// Try again with the same probe size
if(conn->pmtud_probe_count < MAX_PROBES) {
return he_internal_pmtud_send_probe(conn, conn->pmtud_probing_size);
if(conn->pmtud.probe_count < MAX_PROBES) {
return he_internal_pmtud_send_probe(conn, conn->pmtud.probing_size);
}

// Reset probe count
conn->pmtud_probe_pending_id = 0;
conn->pmtud_probe_count = 0;
conn->pmtud.probe_pending_id = 0;
conn->pmtud.probe_count = 0;

// PROBE_COUNT reaches MAX_PROBES, decide what to do based on state
switch(conn->pmtud_state) {
switch(conn->pmtud.state) {
case HE_PMTUD_STATE_BASE:
if(conn->pmtud_probing_size == INITIAL_PLPMTU) {
if(conn->pmtud.probing_size == INITIAL_PLPMTU) {
// Try again using MIN_PLPMTU
return he_internal_pmtud_send_probe(conn, MIN_PLPMTU);
} else {
Expand All @@ -171,18 +171,18 @@ he_return_code_t he_internal_pmtud_handle_probe_timeout(he_conn_t *conn) {
}
break;
case HE_PMTUD_STATE_SEARCHING:
if(conn->pmtud_is_using_big_step) {
if(conn->pmtud.is_using_big_step) {
// Try probing with small step
conn->pmtud_is_using_big_step = false;
uint16_t probe_size = conn->pmtud_probing_size;
conn->pmtud.is_using_big_step = false;
uint16_t probe_size = conn->pmtud.probing_size;
probe_size -= PMTUD_PROBE_BIG_STEP;
probe_size += PMTUD_PROBE_SMALL_STEP;
return he_internal_pmtud_send_probe(conn, probe_size);
} else {
// Search completed
// Set the probing size to the previous successful one
assert(conn->pmtud_probing_size > conn->pmtud_base);
conn->pmtud_probing_size -= PMTUD_PROBE_SMALL_STEP;
assert(conn->pmtud.probing_size > conn->pmtud.base);
conn->pmtud.probing_size -= PMTUD_PROBE_SMALL_STEP;
return he_internal_pmtud_search_completed(conn);
}
break;
Expand All @@ -205,7 +205,7 @@ he_return_code_t he_internal_pmtud_start_base_probing(he_conn_t *conn) {
}

// Check current state
switch(conn->pmtud_state) {
switch(conn->pmtud.state) {
case HE_PMTUD_STATE_DISABLED:
case HE_PMTUD_STATE_SEARCHING:
case HE_PMTUD_STATE_SEARCH_COMPLETE:
Expand All @@ -223,19 +223,19 @@ he_return_code_t he_internal_pmtud_start_base_probing(he_conn_t *conn) {
he_internal_pmtud_change_state(conn, HE_PMTUD_STATE_BASE);

// Initialize PMTUD internal state
conn->pmtud_base = INITIAL_PLPMTU;
conn->pmtud_probe_count = 0;
conn->pmtud.base = INITIAL_PLPMTU;
conn->pmtud.probe_count = 0;

// Start probing base mtu
return he_internal_pmtud_send_probe(conn, conn->pmtud_base);
return he_internal_pmtud_send_probe(conn, conn->pmtud.base);
}

he_return_code_t he_internal_pmtud_base_confirmed(he_conn_t *conn) {
if(!conn) {
return HE_ERR_NULL_POINTER;
}
// Check current state
switch(conn->pmtud_state) {
switch(conn->pmtud.state) {
case HE_PMTUD_STATE_BASE:
case HE_PMTUD_STATE_ERROR:
// Valid states
Expand All @@ -249,9 +249,9 @@ he_return_code_t he_internal_pmtud_base_confirmed(he_conn_t *conn) {
he_internal_pmtud_change_state(conn, HE_PMTUD_STATE_SEARCHING);

// Start searching
conn->pmtud_probe_count = 0;
conn->pmtud_is_using_big_step = true;
uint16_t probe_size = conn->pmtud_base + PMTUD_PROBE_BIG_STEP;
conn->pmtud.probe_count = 0;
conn->pmtud.is_using_big_step = true;
uint16_t probe_size = conn->pmtud.base + PMTUD_PROBE_BIG_STEP;

return he_internal_pmtud_send_probe(conn, probe_size);
}
Expand All @@ -261,7 +261,7 @@ he_return_code_t he_internal_pmtud_confirm_base_failed(he_conn_t *conn) {
return HE_ERR_NULL_POINTER;
}
// Check current state
switch(conn->pmtud_state) {
switch(conn->pmtud.state) {
case HE_PMTUD_STATE_BASE:
// Valid states
break;
Expand All @@ -281,7 +281,7 @@ he_return_code_t he_internal_pmtud_search_completed(he_conn_t *conn) {
return HE_ERR_NULL_POINTER;
}
// Check current state
switch(conn->pmtud_state) {
switch(conn->pmtud.state) {
case HE_PMTUD_STATE_SEARCHING:
// Valid states
break;
Expand All @@ -291,7 +291,7 @@ he_return_code_t he_internal_pmtud_search_completed(he_conn_t *conn) {
}

// Set the effective pmtu
conn->effective_pmtu = MIN(conn->pmtud_probing_size, MAX_PLPMTU);
conn->pmtud.effective_pmtu = MIN(conn->pmtud.probing_size, MAX_PLPMTU);

// Change state
he_internal_pmtud_change_state(conn, HE_PMTUD_STATE_SEARCH_COMPLETE);
Expand All @@ -305,7 +305,7 @@ he_return_code_t he_internal_pmtud_blackhole_detected(he_conn_t *conn) {
return HE_ERR_NULL_POINTER;
}
// Check current state
switch(conn->pmtud_state) {
switch(conn->pmtud.state) {
case HE_PMTUD_STATE_SEARCHING:
case HE_PMTUD_STATE_SEARCH_COMPLETE:
// Valid states
Expand All @@ -319,11 +319,11 @@ he_return_code_t he_internal_pmtud_blackhole_detected(he_conn_t *conn) {
he_internal_pmtud_change_state(conn, HE_PMTUD_STATE_BASE);

// Set the base pmtu
conn->pmtud_base = MIN_PLPMTU;
conn->pmtud_probe_count = 0;
conn->pmtud.base = MIN_PLPMTU;
conn->pmtud.probe_count = 0;

// Start probing base mtu
return he_internal_pmtud_send_probe(conn, conn->pmtud_base);
return he_internal_pmtud_send_probe(conn, conn->pmtud.base);
}

he_return_code_t he_internal_pmtud_retry_probe(he_conn_t *conn, int delay_ms) {
Expand All @@ -334,6 +334,6 @@ he_return_code_t he_internal_pmtud_retry_probe(he_conn_t *conn, int delay_ms) {
ret = conn->pmtud_time_cb(conn, delay_ms, conn->data);
}

conn->pmtud_probe_count = 0;
conn->pmtud.probe_count = 0;
return ret;
}
4 changes: 2 additions & 2 deletions test/he/test_conn.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1728,7 +1728,7 @@ void test_he_conn_start_pmtu_discovery_do_nothing_when_already_started(void) {
conn.pmtud_time_cb = pmtud_time_cb;

// Do nothing if the pmtud is already started
conn.pmtud_state = HE_PMTUD_STATE_BASE;
conn.pmtud.state = HE_PMTUD_STATE_BASE;
TEST_ASSERT_EQUAL(HE_SUCCESS, he_conn_start_pmtu_discovery(&conn));
}

Expand Down Expand Up @@ -1763,7 +1763,7 @@ void test_he_conn_get_effective_pmtu(void) {
TEST_ASSERT_EQUAL(HE_MAX_MTU, he_conn_get_effective_pmtu(NULL));
TEST_ASSERT_EQUAL(HE_MAX_MTU, he_conn_get_effective_pmtu(&conn));

conn.effective_pmtu = 1212;
conn.pmtud.effective_pmtu = 1212;
TEST_ASSERT_EQUAL(1212, he_conn_get_effective_pmtu(&conn));
}

Expand Down
8 changes: 4 additions & 4 deletions test/he/test_flow.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,12 @@ void test_he_internal_flow_should_fragment(void) {

// Don't frag if PMTUD search hasn't completed
conn->connection_type = HE_CONNECTION_TYPE_DATAGRAM;
conn->pmtud_state = HE_PMTUD_STATE_SEARCHING;
conn->pmtud.state = HE_PMTUD_STATE_SEARCHING;
TEST_ASSERT_FALSE(he_internal_flow_should_fragment(conn, 1200, 1350));

// Don't frag if the packet length is exactly effective_pmtu
conn->connection_type = HE_CONNECTION_TYPE_DATAGRAM;
conn->pmtud_state = HE_PMTUD_STATE_SEARCH_COMPLETE;
conn->pmtud.state = HE_PMTUD_STATE_SEARCH_COMPLETE;
TEST_ASSERT_FALSE(he_internal_flow_should_fragment(conn, 1200, 1200));

// Should frag if packet length is greater than effective_pmtu
Expand All @@ -194,7 +194,7 @@ void test_he_internal_flow_should_fragment(void) {

void test_inside_pkt_good_packet_clamp_mss_success(void) {
conn->state = HE_STATE_ONLINE;
conn->pmtud_state = HE_PMTUD_STATE_SEARCH_COMPLETE;
conn->pmtud.state = HE_PMTUD_STATE_SEARCH_COMPLETE;
he_internal_is_ipv4_packet_valid_ExpectAndReturn(fake_ipv4_packet, sizeof(fake_ipv4_packet),
true);
he_conn_get_effective_pmtu_ExpectAndReturn(conn, 100);
Expand All @@ -211,7 +211,7 @@ void test_inside_pkt_good_packet_clamp_mss_success(void) {

void test_inside_pkt_good_packet_clamp_mss_failed(void) {
conn->state = HE_STATE_ONLINE;
conn->pmtud_state = HE_PMTUD_STATE_SEARCH_COMPLETE;
conn->pmtud.state = HE_PMTUD_STATE_SEARCH_COMPLETE;
he_internal_is_ipv4_packet_valid_ExpectAndReturn(fake_ipv4_packet, sizeof(fake_ipv4_packet),
true);
he_conn_get_effective_pmtu_ExpectAndReturn(conn, 100);
Expand Down
2 changes: 1 addition & 1 deletion test/he/test_msg_handlers.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ void test_msg_handler_pong(void) {

void test_msg_handler_pong_pmtud_ack(void) {
conn->ping_pending_id = 42;
conn->pmtud_probe_pending_id = 999;
conn->pmtud.probe_pending_id = 999;

he_internal_pmtud_handle_probe_ack_ExpectAndReturn(conn, 999, HE_SUCCESS);

Expand Down
Loading

0 comments on commit 4f1f9d3

Please sign in to comment.