Change initial DTLS retransmission timer from 1 second to 50ms.

This will help ensure a timely DTLS handshake when there's packet
loss. It will likely result in spurious retransmissions (since the
RTT is usually > 50ms), but since exponential backoff is still used,
there will at most be ~4 extra retransmissions. For a time-sensitive
application like WebRTC this seems like a reasonable tradeoff.

R=pthatcher@webrtc.org, juberti@chromium.org, juberti@webrtc.org

Review URL: https://codereview.webrtc.org/1981463002 .

Committed: https://crrev.com/1e435628366fb9fed71632369f05928ed857d8ef
Cr-Original-Commit-Position: refs/heads/master@{#12853}
Cr-Commit-Position: refs/heads/master@{#13159}
This commit is contained in:
Taylor Brandstetter 2016-06-15 17:15:23 -07:00
parent 947c02d444
commit 4f0dfbd213
9 changed files with 118 additions and 28 deletions

View File

@ -21,25 +21,25 @@
#endif
// Wait until "ex" is true, or "timeout" expires.
#define WAIT(ex, timeout) \
for (int64_t start = rtc::TimeMillis(); \
!(ex) && rtc::TimeMillis() < start + timeout;) { \
rtc::Thread::Current()->ProcessMessages(0); \
rtc::Thread::Current()->SleepMs(1); \
#define WAIT(ex, timeout) \
for (int64_t start = rtc::SystemTimeMillis(); \
!(ex) && rtc::SystemTimeMillis() < start + timeout;) { \
rtc::Thread::Current()->ProcessMessages(0); \
rtc::Thread::Current()->SleepMs(1); \
}
// This returns the result of the test in res, so that we don't re-evaluate
// the expression in the XXXX_WAIT macros below, since that causes problems
// when the expression is only true the first time you check it.
#define WAIT_(ex, timeout, res) \
do { \
int64_t start = rtc::TimeMillis(); \
res = (ex); \
while (!res && rtc::TimeMillis() < start + timeout) { \
rtc::Thread::Current()->ProcessMessages(0); \
rtc::Thread::Current()->SleepMs(1); \
res = (ex); \
} \
#define WAIT_(ex, timeout, res) \
do { \
int64_t start = rtc::SystemTimeMillis(); \
res = (ex); \
while (!res && rtc::SystemTimeMillis() < start + timeout) { \
rtc::Thread::Current()->ProcessMessages(0); \
rtc::Thread::Current()->SleepMs(1); \
res = (ex); \
} \
} while (0)
// The typical EXPECT_XXXX and ASSERT_XXXXs, but done until true or a timeout.

View File

@ -124,7 +124,9 @@ LogMessage::LogMessage(const char* file,
const char* module)
: severity_(sev), tag_(kLibjingle) {
if (timestamp_) {
int64_t time = TimeSince(LogStartTime());
// Use SystemTimeMillis so that even if tests use fake clocks, the timestamp
// in log messages represents the real system time.
int64_t time = TimeDiff(SystemTimeMillis(), LogStartTime());
// Also ensure WallClockStartTime is initialized, so that it matches
// LogStartTime.
WallClockStartTime();
@ -210,7 +212,7 @@ LogMessage::~LogMessage() {
}
int64_t LogMessage::LogStartTime() {
static const int64_t g_start = TimeMillis();
static const int64_t g_start = SystemTimeMillis();
return g_start;
}

View File

@ -34,6 +34,7 @@
#include "webrtc/base/openssldigest.h"
#include "webrtc/base/opensslidentity.h"
#include "webrtc/base/stringutils.h"
#include "webrtc/base/timeutils.h"
#include "webrtc/base/thread.h"
namespace rtc {
@ -58,7 +59,13 @@ static SrtpCipherMapEntry SrtpCipherMap[] = {
{nullptr, 0}};
#endif
#ifndef OPENSSL_IS_BORINGSSL
#ifdef OPENSSL_IS_BORINGSSL
static void TimeCallback(const SSL* ssl, struct timeval* out_clock) {
uint64_t time = TimeNanos();
out_clock->tv_sec = time / kNumNanosecsPerSec;
out_clock->tv_usec = time / kNumNanosecsPerMicrosec;
}
#else // #ifdef OPENSSL_IS_BORINGSSL
// Cipher name table. Maps internal OpenSSL cipher ids to the RFC name.
struct SslCipherMapEntry {
@ -771,13 +778,19 @@ int OpenSSLStreamAdapter::BeginSSL() {
SSL_set_app_data(ssl_, this);
SSL_set_bio(ssl_, bio, bio); // the SSL object owns the bio now.
#ifndef OPENSSL_IS_BORINGSSL
if (ssl_mode_ == SSL_MODE_DTLS) {
#ifdef OPENSSL_IS_BORINGSSL
// Change the initial retransmission timer from 1 second to 50ms.
// This will likely result in some spurious retransmissions, but
// it's useful for ensuring a timely handshake when there's packet
// loss.
DTLSv1_set_initial_timeout_duration(ssl_, 50);
#else
// Enable read-ahead for DTLS so whole packets are read from internal BIO
// before parsing. This is done internally by BoringSSL for DTLS.
SSL_set_read_ahead(ssl_, 1);
}
#endif
}
SSL_set_mode(ssl_, SSL_MODE_ENABLE_PARTIAL_WRITE |
SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
@ -985,6 +998,11 @@ SSL_CTX* OpenSSLStreamAdapter::SetupSSLContext() {
DTLS1_2_VERSION : TLS1_2_VERSION);
break;
}
// Set a time callback for BoringSSL because:
// 1. Our time function is more accurate (doesn't just use gettimeofday).
// 2. This allows us to inject a fake clock for testing.
// SSL_CTX_set_current_time_cb(ctx, &TimeCallback);
ctx->current_time_cb = &TimeCallback;
#endif
if (identity_ && !identity_->ConfigureIdentity(ctx)) {
@ -1127,6 +1145,14 @@ bool OpenSSLStreamAdapter::HaveExporter() {
#endif
}
bool OpenSSLStreamAdapter::IsBoringSsl() {
#ifdef OPENSSL_IS_BORINGSSL
return true;
#else
return false;
#endif
}
#define CDEF(X) \
{ static_cast<uint16_t>(TLS1_CK_##X & 0xffff), "TLS_" #X }

View File

@ -111,6 +111,7 @@ class OpenSSLStreamAdapter : public SSLStreamAdapter {
static bool HaveDtls();
static bool HaveDtlsSrtp();
static bool HaveExporter();
static bool IsBoringSsl();
static bool IsAcceptableCipher(int cipher, KeyType key_type);
static bool IsAcceptableCipher(const std::string& cipher, KeyType key_type);

View File

@ -82,6 +82,9 @@ bool SSLStreamAdapter::HaveDtlsSrtp() {
bool SSLStreamAdapter::HaveExporter() {
return OpenSSLStreamAdapter::HaveExporter();
}
bool SSLStreamAdapter::IsBoringSsl() {
return OpenSSLStreamAdapter::IsBoringSsl();
}
bool SSLStreamAdapter::IsAcceptableCipher(int cipher, KeyType key_type) {
return OpenSSLStreamAdapter::IsAcceptableCipher(cipher, key_type);
}

View File

@ -195,6 +195,7 @@ class SSLStreamAdapter : public StreamAdapterInterface {
static bool HaveDtls();
static bool HaveDtlsSrtp();
static bool HaveExporter();
static bool IsBoringSsl();
// Returns true iff the supplied cipher is deemed to be strong.
// TODO(torbjorng): Consider removing the KeyType argument.

View File

@ -38,10 +38,7 @@ ClockInterface* SetClockForTesting(ClockInterface* clock) {
return prev;
}
uint64_t TimeNanos() {
if (g_clock) {
return g_clock->TimeNanos();
}
uint64_t SystemTimeNanos() {
int64_t ticks;
#if defined(WEBRTC_MAC)
static mach_timebase_info_data_t timebase;
@ -86,6 +83,17 @@ uint64_t TimeNanos() {
return ticks;
}
int64_t SystemTimeMillis() {
return static_cast<int64_t>(SystemTimeNanos() / kNumNanosecsPerMillisec);
}
uint64_t TimeNanos() {
if (g_clock) {
return g_clock->TimeNanos();
}
return SystemTimeNanos();
}
uint32_t Time32() {
return static_cast<uint32_t>(TimeNanos() / kNumNanosecsPerMillisec);
}

View File

@ -53,6 +53,11 @@ class ClockInterface {
// that uses it, eliminating the need for a global variable and this function.
ClockInterface* SetClockForTesting(ClockInterface* clock);
// Returns the actual system time, even if a clock is set for testing.
// Useful for timeouts while using a test clock, or for logging.
uint64_t SystemTimeNanos();
int64_t SystemTimeMillis();
// Returns the current time in milliseconds in 32 bits.
uint32_t Time32();

View File

@ -22,10 +22,10 @@
#include "webrtc/base/sslstreamadapter.h"
#include "webrtc/base/stringutils.h"
#define MAYBE_SKIP_TEST(feature) \
if (!(rtc::SSLStreamAdapter::feature())) { \
LOG(LS_INFO) << "Feature disabled... skipping"; \
return; \
#define MAYBE_SKIP_TEST(feature) \
if (!(rtc::SSLStreamAdapter::feature())) { \
LOG(LS_INFO) << #feature " feature disabled... skipping"; \
return; \
}
static const char kIceUfrag1[] = "TESTICEUFRAG0001";
@ -413,7 +413,8 @@ class DtlsTestClient : public sigslot::has_slots<> {
rtc::SentPacket sent_packet_;
};
// Note that this test always uses a FakeClock, due to the |fake_clock_| member
// variable.
class DtlsTransportChannelTest : public testing::Test {
public:
DtlsTransportChannelTest()
@ -561,6 +562,7 @@ class DtlsTransportChannelTest : public testing::Test {
}
protected:
rtc::ScopedFakeClock fake_clock_;
DtlsTestClient client1_;
DtlsTestClient client2_;
int channel_ct_;
@ -974,3 +976,45 @@ TEST_F(DtlsTransportChannelTest,
kTimeout);
EXPECT_EQ(1, client1_.received_dtls_client_hellos());
}
// Test that packets are retransmitted according to the expected schedule.
// Each time a timeout occurs, the retransmission timer should be doubled up to
// 60 seconds. The timer defaults to 1 second, but for WebRTC we should be
// initializing it to 50ms.
TEST_F(DtlsTransportChannelTest, TestRetransmissionSchedule) {
MAYBE_SKIP_TEST(HaveDtls);
// We can only change the retransmission schedule with a recently-added
// BoringSSL API. Skip the test if not built with BoringSSL.
MAYBE_SKIP_TEST(IsBoringSsl);
PrepareDtls(true, true, rtc::KT_DEFAULT);
// Exchange transport descriptions.
Negotiate(cricket::CONNECTIONROLE_ACTPASS, cricket::CONNECTIONROLE_ACTIVE);
// Make client2_ writable, but not client1_.
// This means client1_ will send DTLS client hellos but get no response.
EXPECT_TRUE(client2_.Connect(&client1_, true));
EXPECT_TRUE_WAIT(client2_.all_raw_channels_writable(), kTimeout);
// Wait for the first client hello to be sent.
EXPECT_EQ_WAIT(1, client1_.received_dtls_client_hellos(), kTimeout);
EXPECT_FALSE(client1_.all_raw_channels_writable());
static int timeout_schedule_ms[] = {50, 100, 200, 400, 800, 1600,
3200, 6400, 12800, 25600, 51200, 60000};
int expected_hellos = 1;
for (size_t i = 0;
i < (sizeof(timeout_schedule_ms) / sizeof(timeout_schedule_ms[0]));
++i) {
// For each expected retransmission time, advance the fake clock a
// millisecond before the expected time and verify that no unexpected
// retransmissions were sent. Then advance it the final millisecond and
// verify that the expected retransmission was sent.
fake_clock_.AdvanceTime(
rtc::TimeDelta::FromMilliseconds(timeout_schedule_ms[i] - 1));
EXPECT_EQ(expected_hellos, client1_.received_dtls_client_hellos());
fake_clock_.AdvanceTime(rtc::TimeDelta::FromMilliseconds(1));
EXPECT_EQ(++expected_hellos, client1_.received_dtls_client_hellos());
}
}