Merge pull request #165 from expressvpn/CVPN-165

CVPN-165 Start PMTUD from 1200 MTU

src/he/pmtud.c

@@ -162,8 +162,14 @@ he_return_code_t he_internal_pmtud_handle_probe_timeout(he_conn_t *conn) {
   // PROBE_COUNT reaches MAX_PROBES, decide what to do based on state
   switch(conn->pmtud_state) {
     case HE_PMTUD_STATE_BASE:
-      // Unable to confirm the base PMTU, entering error state.
-      return he_internal_pmtud_confirm_base_failed(conn);
+      if(conn->pmtud_probing_size == INITIAL_PLPMTU) {
+        // Try again using MIN_PLPMTU
+        return he_internal_pmtud_send_probe(conn, MIN_PLPMTU);
+      } else {
+        // Unable to confirm the base PMTU, entering error state.
+        return he_internal_pmtud_confirm_base_failed(conn);
+      }
+      break;
     case HE_PMTUD_STATE_SEARCHING:
       if(conn->pmtud_is_using_big_step) {
         // Try probing with small step
@@ -217,7 +223,7 @@ 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 = MIN_PLPMTU;
+  conn->pmtud_base = INITIAL_PLPMTU;
   conn->pmtud_probe_count = 0;
 
   // Start probing base mtu
@@ -324,7 +330,7 @@ he_return_code_t he_internal_pmtud_retry_probe(he_conn_t *conn, int delay_ms) {
   he_return_code_t ret = HE_ERR_PMTUD_CALLBACKS_NOT_SET;
 
   // Retry PMTUD after a delay
-  if (conn->pmtud_time_cb) {
+  if(conn->pmtud_time_cb) {
     ret = conn->pmtud_time_cb(conn, delay_ms, conn->data);
   }
 

src/he/pmtud.h

@@ -40,6 +40,9 @@
   (HE_MAX_WIRE_MTU - HE_IPV4_HEADER_SIZE - HE_UDP_HEADER_SIZE - sizeof(he_wire_hdr_t) - \
    HE_WOLF_MAX_HEADER_SIZE - sizeof(he_msg_data_t))
 
+/// The initial PMTU the discovery process will attempt to use first
+#define INITIAL_PLPMTU 1200
+
 /// The default timeout for waiting for an acknowledgement to a probe packet, in milliseconds.
 #define PMTUD_PROBE_TIMEOUT_MS 5000
 

test/he/test_pmtud.c

@@ -250,9 +250,11 @@ void test_he_internal_pmtud_handle_probe_timeout_confirm_base_failed(void) {
   conn.pmtud_state = HE_PMTUD_STATE_BASE;
   conn.pmtud_probe_count = MAX_PROBES;
   conn.pmtud_time_cb = pmtud_time_cb;
+  conn.pmtud_probing_size = MIN_PLPMTU;
 
   // Probe count reached MAX_PROBES,
-  // it should call he_internal_pmtud_confirm_base_failed if current state is Base.
+  // it should call he_internal_pmtud_confirm_base_failed if current state is BASE
+  // and the probe size is NOT INITIAL_PLPMTU.
   TEST_ASSERT_EQUAL(HE_SUCCESS, he_internal_pmtud_handle_probe_timeout(&conn));
 
   // The new state should be Error
@@ -266,6 +268,60 @@ void test_he_internal_pmtud_handle_probe_timeout_confirm_base_failed(void) {
   TEST_ASSERT_EQUAL(1, call_counter);
 }
 
+void test_he_internal_pmtud_handle_probe_timeout_confirm_base_retry_with_min_plpmtu(void) {
+  conn.state = HE_STATE_ONLINE;
+  conn.pmtud_state = HE_PMTUD_STATE_BASE;
+  conn.pmtud_probe_count = MAX_PROBES;
+  conn.pmtud_time_cb = pmtud_time_cb;
+  conn.pmtud_probing_size = INITIAL_PLPMTU;
+  conn.ping_next_id = 42;
+
+  EXPECT_HE_INTERNAL_SEND_MESSAGE(HE_SUCCESS);
+
+  // Probe count reached MAX_PROBES when confirming base.
+  // It should try again using MIN_PLPMTU.
+  TEST_ASSERT_EQUAL(HE_SUCCESS, he_internal_pmtud_handle_probe_timeout(&conn));
+
+  // The new state should still be BASE
+  TEST_ASSERT_EQUAL(HE_PMTUD_STATE_BASE, conn.pmtud_state);
+
+  // The probe count and pending id should be set
+  TEST_ASSERT_EQUAL(1, conn.pmtud_probe_count);
+  TEST_ASSERT_EQUAL(42, conn.pmtud_probe_pending_id);
+
+  // The new probe size should be MIN_PLPMTU
+  TEST_ASSERT_EQUAL(MIN_PLPMTU, conn.pmtud_probing_size);
+
+  // pmtud_time_cb should be called to retry the error
+  TEST_ASSERT_EQUAL(1, call_counter);
+}
+
+void test_he_internal_pmtud_start_base_probing(void) {
+  conn.state = HE_STATE_ONLINE;
+  conn.pmtud_state = HE_PMTUD_STATE_DISABLED;
+  conn.pmtud_time_cb = pmtud_time_cb;
+  conn.ping_next_id = 42;
+
+  EXPECT_HE_INTERNAL_SEND_MESSAGE(HE_SUCCESS);
+
+  he_return_code_t rc = he_internal_pmtud_start_base_probing(&conn);
+
+  // The new state should be BASE
+  TEST_ASSERT_EQUAL(HE_PMTUD_STATE_BASE, conn.pmtud_state);
+
+  // The probe count and pending id should be set
+  TEST_ASSERT_EQUAL(1, conn.pmtud_probe_count);
+  TEST_ASSERT_EQUAL(42, conn.pmtud_probe_pending_id);
+
+  // The probe size should be INITIAL_PLPMTU
+  TEST_ASSERT_EQUAL(INITIAL_PLPMTU, conn.pmtud_probing_size);
+
+  // pmtud_time_cb should be called to retry the error
+  TEST_ASSERT_EQUAL(1, call_counter);
+
+  TEST_ASSERT_EQUAL(HE_SUCCESS, rc);
+}
+
 void test_he_internal_pmtud_handle_probe_timeout_search_completed(void) {
   conn.state = HE_STATE_ONLINE;
   conn.pmtud_state = HE_PMTUD_STATE_SEARCHING;