Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear out rebound paths upon path exhaustion #1783

Merged
merged 11 commits into from
Jul 8, 2021
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ option(QUIC_TLS_SECRETS_SUPPORT "Enable export of TLS secrets" OFF)
option(QUIC_TELEMETRY_ASSERTS "Enable telemetry asserts in release builds" OFF)
option(QUIC_USE_SYSTEM_LIBCRYPTO "Use system libcrypto if openssl TLS" OFF)
option(QUIC_DISABLE_POSIX_GSO "Disable GSO for systems that say they support it but don't" OFF)
option(QUIC_FOLDER_PREFIX "Optional prefix for source group folders when using an IDE generator" "")
set(QUIC_FOLDER_PREFIX "" CACHE STRING "Optional prefix for source group folders when using an IDE generator")

set(BUILD_SHARED_LIBS ${QUIC_BUILD_SHARED})

Expand Down Expand Up @@ -628,7 +628,7 @@ if(QUIC_BUILD_TEST)
include(FetchContent)

enable_testing()

# Build the googletest framework.

# Enforce static builds for test artifacts
Expand Down
26 changes: 23 additions & 3 deletions src/core/path.c
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ QuicPathRemove(

#if DEBUG
if (Path->DestCid) {
CXPLAT_DBG_ASSERT(Path->DestCid->AssignedPath != NULL);
nibanks marked this conversation as resolved.
Show resolved Hide resolved
QUIC_CID_CLEAR_PATH(Path->DestCid);
}
#endif
Expand Down Expand Up @@ -196,9 +195,30 @@ QuicConnGetPathForDatagram(

if (Connection->PathsCount == QUIC_MAX_PATH_COUNT) {
//
// Already tracking the maximum number of paths.
// See if any old paths share the same remote address, and is just a rebind.
// If so, remove the old paths.
// NB: Traversing the array backwards is simpler and more efficient here due
// to the array shifting that happens in QuicPathRemove.
//
return NULL;
for (uint8_t i = Connection->PathsCount - 1; i > 0; i--) {
BOOLEAN IsSimilarRemoteAddress =
QuicAddrGetFamily(&Datagram->Tuple->RemoteAddress) == QuicAddrGetFamily(&Connection->Paths[i].RemoteAddress) &&
QuicAddrCompareIp(&Datagram->Tuple->RemoteAddress, &Connection->Paths[i].RemoteAddress);
BOOLEAN IsSameLocalAddress = QuicAddrCompare(&Datagram->Tuple->LocalAddress, &Connection->Paths[i].LocalAddress);
if (IsSimilarRemoteAddress
&& IsSameLocalAddress
&& !Connection->Paths[i].IsActive) {
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved
QuicPathRemove(Connection, i);
}
}

if (Connection->PathsCount == QUIC_MAX_PATH_COUNT) {
//
// Already tracking the maximum number of paths, and can't free
// any more.
//
return NULL;
}
}

if (Connection->PathsCount > 1) {
Expand Down
17 changes: 17 additions & 0 deletions src/inc/msquic.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ class MsQuicSettings : public QUIC_SETTINGS {
MsQuicSettings& SetMinimumMtu(uint16_t Mtu) { MinimumMtu = Mtu; IsSet.MinimumMtu = TRUE; return *this; }
MsQuicSettings& SetMtuDiscoverySearchCompleteTimeoutUs(uint64_t Time) { MtuDiscoverySearchCompleteTimeoutUs = Time; IsSet.MtuDiscoverySearchCompleteTimeoutUs = TRUE; return *this; }
MsQuicSettings& SetMtuDiscoveryMissingProbeCount(uint8_t Count) { MtuDiscoveryMissingProbeCount = Count; IsSet.MtuDiscoveryMissingProbeCount = TRUE; return *this; }
MsQuicSettings& SetKeepAlive(uint32_t Time) { KeepAliveIntervalMs = Time; IsSet.KeepAliveIntervalMs = TRUE; return *this; }

QUIC_STATUS
SetGlobal() const noexcept {
Expand Down Expand Up @@ -779,6 +780,17 @@ struct MsQuicConnection {
&Addr.SockAddr);
}

QUIC_STATUS
GetRemoteAddr(_Out_ QuicAddr& Addr) {
uint32_t Size = sizeof(Addr.SockAddr);
return
GetParam(
QUIC_PARAM_LEVEL_CONNECTION,
QUIC_PARAM_CONN_REMOTE_ADDRESS,
&Size,
&Addr.SockAddr);
}

QUIC_STATUS
SetLocalAddr(_In_ const QuicAddr& Addr) noexcept {
return
Expand All @@ -802,6 +814,11 @@ struct MsQuicConnection {
&Value);
}

QUIC_STATUS
SetKeepAlive(_In_ uint32_t Value) noexcept {
return SetSettings(MsQuicSettings{}.SetKeepAlive(Value));
}

ThadHouse marked this conversation as resolved.
Show resolved Hide resolved
QUIC_STATUS
SetResumptionTicket(_In_reads_(TicketLength) const uint8_t* Ticket, uint32_t TicketLength) noexcept {
return
Expand Down
8 changes: 8 additions & 0 deletions src/test/MsQuicTests.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,14 @@ QuicTestMtuDiscovery(
_In_ BOOLEAN RaiseMinimumMtu
);

//
// Path tests
//
void
QuicTestLocalPathChanges(
_In_ int Family
);

//
// Handshake Tests
//
Expand Down
10 changes: 10 additions & 0 deletions src/test/bin/quic_gtest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,16 @@ TEST_P(WithHandshakeArgs3, AsyncSecurityConfig) {
}
}

TEST_P(WithFamilyArgs, LocalPathChanges) {
TestLoggerT<ParamType> Logger("QuicTestLocalPathChanges", GetParam());
if (TestingKernelMode) {
// TODO
//ASSERT_TRUE(DriverClient.Run(IOCTL_QUIC_RUN_VERSION_NEGOTIATION, GetParam().Family));
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved
} else {
QuicTestLocalPathChanges(GetParam().Family);
}
}

TEST_P(WithFamilyArgs, VersionNegotiation) {
TestLoggerT<ParamType> Logger("QuicTestVersionNegotiation", GetParam());
if (TestingKernelMode) {
Expand Down
1 change: 1 addition & 0 deletions src/test/lib/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ set(SOURCES
EventTest.cpp
HandshakeTest.cpp
MtuTest.cpp
PathTest.cpp
QuicDrill.cpp
TestConnection.cpp
TestListener.cpp
Expand Down
106 changes: 106 additions & 0 deletions src/test/lib/PathTest.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
/*++

Copyright (c) Microsoft Corporation.
Licensed under the MIT License.

Abstract:

MsQuic Path Unittest

--*/

#include "precomp.h"
#ifdef QUIC_CLOG
#include "PathTest.cpp.clog.h"
#endif

struct PathTestContext {
CxPlatEvent ShutdownEvent;
MsQuicConnection* Connection {nullptr};
bool PeerAddrChanged {false};

static QUIC_STATUS ConnCallback(_In_ MsQuicConnection* Conn, _In_opt_ void* Context, _Inout_ QUIC_CONNECTION_EVENT* Event) {
PathTestContext* Ctx = static_cast<PathTestContext*>(Context);
Ctx->Connection = Conn;
if (Event->Type == QUIC_CONNECTION_EVENT_SHUTDOWN_COMPLETE) {
Ctx->Connection = nullptr;
Ctx->ShutdownEvent.Set();
}
else if (Event->Type == QUIC_CONNECTION_EVENT_PEER_ADDRESS_CHANGED) {
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved
Ctx->PeerAddrChanged = true;
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved
}
return QUIC_STATUS_SUCCESS;
}
};

void
QuicTestLocalPathChanges(
_In_ int Family
)
{
MsQuicRegistration Registration;
TEST_QUIC_SUCCEEDED(Registration.GetInitStatus());

MsQuicAlpn Alpn("MsQuicTest");
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved

MsQuicConfiguration ServerConfiguration(Registration, Alpn, ServerSelfSignedCredConfig);
TEST_QUIC_SUCCEEDED(ServerConfiguration.GetInitStatus());

MsQuicCredentialConfig ClientCredConfig;
MsQuicConfiguration ClientConfiguration(Registration, Alpn, ClientCredConfig);
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved
TEST_QUIC_SUCCEEDED(ClientConfiguration.GetInitStatus());

PathTestContext Context;
MsQuicAutoAcceptListener Listener(Registration, ServerConfiguration, PathTestContext::ConnCallback, &Context);
TEST_QUIC_SUCCEEDED(Listener.GetInitStatus());
QUIC_ADDRESS_FAMILY QuicAddrFamily = (Family == 4) ? QUIC_ADDRESS_FAMILY_INET : QUIC_ADDRESS_FAMILY_INET6;
QuicAddr ServerLocalAddr(QuicAddrFamily);
TEST_QUIC_SUCCEEDED(Listener.Start(Alpn, &ServerLocalAddr.SockAddr));
TEST_QUIC_SUCCEEDED(Listener.GetLocalAddr(ServerLocalAddr));

MsQuicConnection Connection(Registration);
TEST_QUIC_SUCCEEDED(Connection.GetInitStatus());

TEST_QUIC_SUCCEEDED(Connection.StartLocalhost(ClientConfiguration, ServerLocalAddr));
TEST_TRUE(Connection.HandshakeCompleteEvent.WaitTimeout(TestWaitTimeout));

CxPlatSleep(1000);
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved
TEST_NOT_EQUAL(nullptr, Context.Connection);

QuicAddr OrigLocalAddr;
TEST_QUIC_SUCCEEDED(Connection.GetLocalAddr(OrigLocalAddr));
QuicAddr NewLocalAddr(OrigLocalAddr);
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved


for (int i = 0; i < 50; i++) {
NewLocalAddr.IncrementPort();
ReplaceAddressHelper AddrHelper(OrigLocalAddr.SockAddr, NewLocalAddr.SockAddr);
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved
Connection.SetKeepAlive(25);

bool ServerAddressUpdated = false;
uint32_t Try = 0;
do {
if (Try != 0) {
CxPlatSleep(200);
}
QuicAddr ServerRemoteAddr;
TEST_QUIC_SUCCEEDED(Context.Connection->GetRemoteAddr(ServerRemoteAddr));
if (Context.PeerAddrChanged &&
QuicAddrCompare(&NewLocalAddr.SockAddr, &ServerRemoteAddr.SockAddr)) {
ServerAddressUpdated = true;
Context.PeerAddrChanged = false;
break;
}
} while (++Try <= 3);
TEST_TRUE(ServerAddressUpdated);
Connection.SetKeepAlive(0);
}

CxPlatSleep(1000);
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved
TEST_NOT_EQUAL(nullptr, Context.Connection);
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved

Connection.Shutdown(1);
Context.Connection->Shutdown(1);

Context.ShutdownEvent.WaitTimeout(2000);
ThadHouse marked this conversation as resolved.
Show resolved Hide resolved
}