Skip to content

Commit

Permalink
fixup: don't set pmtud_probe_pending_id if he_internal_send_message f…
Browse files Browse the repository at this point in the history
…ailed
  • Loading branch information
expressvpn-tom-l committed Oct 18, 2023
1 parent 4021537 commit a09d040
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 12 deletions.
5 changes: 3 additions & 2 deletions src/he/pmtud.c
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,11 @@ he_return_code_t he_internal_pmtud_send_probe(he_conn_t *conn, uint16_t probe_mt
// If he_internal_send_message failed to send the probe message, trigger the pmtud timeout with
// a tiny delay
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
if(conn->pmtud_time_cb) {
conn->pmtud_time_cb(conn, timeout_ms, conn->data);
Expand Down
44 changes: 34 additions & 10 deletions test/he/test_pmtud.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@
#include "mock_conn.h"

// Helper macros
#define EXPECT_HE_INTERNAL_SEND_MESSAGE() \
do { \
he_internal_send_message_ExpectAndReturn(&conn, NULL, 0, HE_SUCCESS); \
he_internal_send_message_IgnoreArg_message(); \
he_internal_send_message_IgnoreArg_length(); \
#define EXPECT_HE_INTERNAL_SEND_MESSAGE(rc) \
do { \
he_internal_send_message_ExpectAndReturn(&conn, NULL, 0, (rc)); \
he_internal_send_message_IgnoreArg_message(); \
he_internal_send_message_IgnoreArg_length(); \
} while(0)

static he_conn_t conn = {0};
Expand All @@ -51,6 +51,7 @@ void test_he_internal_pmtud_send_probe(void) {
conn.state = HE_STATE_ONLINE;
conn.pmtud_state = HE_PMTUD_STATE_BASE;
conn.pmtud_time_cb = pmtud_time_cb;
conn.ping_next_id = 42;

uint16_t probe_mtu = 1212;

Expand All @@ -63,6 +64,29 @@ void test_he_internal_pmtud_send_probe(void) {
he_return_code_t res = he_internal_pmtud_send_probe(&conn, probe_mtu);
TEST_ASSERT_EQUAL(HE_SUCCESS, res);
TEST_ASSERT_EQUAL(1, call_counter);
TEST_ASSERT_EQUAL(probe_mtu, conn.pmtud_probing_size);
TEST_ASSERT_EQUAL(42, conn.pmtud_probe_pending_id);
}

void test_he_internal_pmtud_send_probe_failed(void) {
conn.state = HE_STATE_ONLINE;
conn.pmtud_state = HE_PMTUD_STATE_BASE;
conn.pmtud_time_cb = pmtud_time_cb;
conn.ping_next_id = 42;

uint16_t probe_mtu = 1212;

// The expected ping message length should be equal to the length of the of data message of the
// given payload size
uint16_t expected_length = probe_mtu + sizeof(he_msg_data_t);
he_internal_send_message_ExpectAndReturn(&conn, NULL, expected_length, HE_ERR_SSL_ERROR);
he_internal_send_message_IgnoreArg_message();

he_return_code_t res = he_internal_pmtud_send_probe(&conn, probe_mtu);
TEST_ASSERT_EQUAL(HE_SUCCESS, res);
TEST_ASSERT_EQUAL(1, call_counter);
TEST_ASSERT_EQUAL(probe_mtu, conn.pmtud_probing_size);
TEST_ASSERT_EQUAL(0, conn.pmtud_probe_pending_id);
}

void test_he_internal_pmtud_send_probe_nulls(void) {
Expand Down Expand Up @@ -98,7 +122,7 @@ void test_he_internal_pmtud_handle_probe_ack_from_base(void) {
conn.pmtud_time_cb = pmtud_time_cb;
conn.pmtud_base = HE_MAX_MTU;

EXPECT_HE_INTERNAL_SEND_MESSAGE();
EXPECT_HE_INTERNAL_SEND_MESSAGE(HE_SUCCESS);

TEST_ASSERT_EQUAL(HE_SUCCESS, he_internal_pmtud_handle_probe_ack(&conn, 123));

Expand All @@ -122,7 +146,7 @@ void test_he_internal_pmtud_handle_probe_ack_from_error(void) {
conn.pmtud_time_cb = pmtud_time_cb;
conn.pmtud_base = HE_MAX_MTU;

EXPECT_HE_INTERNAL_SEND_MESSAGE();
EXPECT_HE_INTERNAL_SEND_MESSAGE(HE_SUCCESS);

TEST_ASSERT_EQUAL(HE_SUCCESS, he_internal_pmtud_handle_probe_ack(&conn, 123));

Expand All @@ -149,7 +173,7 @@ static void test_handle_probe_ack_from_searching(uint16_t probe_size, bool use_b
conn.pmtud_is_using_big_step = use_big_step;

if(probe_size < MAX_PLPMTU) {
EXPECT_HE_INTERNAL_SEND_MESSAGE();
EXPECT_HE_INTERNAL_SEND_MESSAGE(HE_SUCCESS);
}

TEST_ASSERT_EQUAL(HE_SUCCESS, he_internal_pmtud_handle_probe_ack(&conn, 123));
Expand Down Expand Up @@ -196,7 +220,7 @@ void test_he_internal_pmtud_handle_probe_timeout_try_again(void) {
conn.pmtud_probing_size = 1212;

// It should send probe again when probe count hasn't reached MAX_PROBES
EXPECT_HE_INTERNAL_SEND_MESSAGE();
EXPECT_HE_INTERNAL_SEND_MESSAGE(HE_SUCCESS);

TEST_ASSERT_EQUAL(HE_SUCCESS, he_internal_pmtud_handle_probe_timeout(&conn));

Expand Down Expand Up @@ -249,7 +273,7 @@ void test_he_internal_pmtud_handle_probe_timeout_blackhole_detected(void) {
conn.pmtud_is_using_big_step = false;
conn.pmtud_time_cb = pmtud_time_cb;

EXPECT_HE_INTERNAL_SEND_MESSAGE();
EXPECT_HE_INTERNAL_SEND_MESSAGE(HE_SUCCESS);

// Probe count reached MAX_PROBES,
TEST_ASSERT_EQUAL(HE_SUCCESS, he_internal_pmtud_handle_probe_timeout(&conn));
Expand Down

0 comments on commit a09d040

Please sign in to comment.