Skip to content

Commit 3f321b6

Browse files
authored
Fix QTIP port selection (#5626)
## Description Fixes #5346. When creating a QTIP listener with a wildcard port: - a UDP socket is created first (to reserve the UDP port as XDP will take it over), and the OS would assign an ephemeral port to it - a TCP socket is created second (to reserve the TCP port as XDP will take it over) - but instead of re-using the same port, the initial config was re-used, and the OS would assign a different ephemeral port - this second port would override the first one in the MsQuic socket config The main consequences of this bug are: - the right UDP port was not reserved when creating a listener with a wildcard port - if later on, the OS assigned the UDP port matching the listener TCP port, MsQuic would fail to create a binding (causing the test failure we observed) Creating a listener without a port set is largely a test scenario. Creating a client connection with an unspecified a port was ok because only a TCP or a UDP socket is created for client connection. ## Testing C/I. We should add some test to validate port reservation in XDP scenarios, but it will take efforts to enable test to create sockets cross-platform. ## Documentation N/A
1 parent 51aabfb commit 3f321b6

File tree

1 file changed

+16
-11
lines changed

1 file changed

+16
-11
lines changed

src/platform/datapath_raw_win.c

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -123,28 +123,33 @@ RawSocketCreateUdp(
123123
memcpy(Socket->CibirId, Config->CibirId, Config->CibirIdLength);
124124
}
125125

126+
//
127+
// The socket addresses have been set in the SocketCreateUdp call earlier,
128+
// either form the config or assigned by the OS (for unspecified ports).
129+
// Do no override them from the config here: we need to keep the same OS assigned ports if the
130+
// config doesn't specify them.
131+
//
132+
CXPLAT_DBG_ASSERT(
133+
Config->RemoteAddress == NULL ||
134+
QuicAddrCompare(&Socket->RemoteAddress, Config->RemoteAddress));
135+
CXPLAT_DBG_ASSERT(
136+
Config->LocalAddress == NULL ||
137+
QuicAddrGetPort(Config->LocalAddress) == 0 ||
138+
QuicAddrGetPort(&Socket->LocalAddress) == QuicAddrGetPort(Config->LocalAddress));
139+
126140
if (Config->RemoteAddress) {
127141
//
128142
// This CxPlatSocket is part of a client connection.
129143
//
130144
CXPLAT_FRE_ASSERT(!QuicAddrIsWildCard(Config->RemoteAddress)); // No wildcard remote addresses allowed.
131-
if (Socket->ReserveAuxTcpSock) {
132-
Socket->RemoteAddress = *Config->RemoteAddress;
133-
if (Config->LocalAddress != NULL) {
134-
Socket->LocalAddress = *Config->LocalAddress;
135-
} else {
136-
QuicAddrSetFamily(&Socket->LocalAddress, QUIC_ADDRESS_FAMILY_INET6);
137-
}
138-
}
145+
139146
Socket->Connected = TRUE;
140147
} else {
141148
//
142149
// This CxPlatSocket is part of a server listener.
143150
//
144151
CXPLAT_FRE_ASSERT(Config->LocalAddress != NULL);
145-
if (Socket->ReserveAuxTcpSock) {
146-
Socket->LocalAddress = *Config->LocalAddress;
147-
}
152+
148153
if (!QuicAddrIsWildCard(Config->LocalAddress)) { // For server listeners, the local address MUST be a wildcard address.
149154
Status = QUIC_STATUS_INVALID_STATE;
150155
goto Error;

0 commit comments

Comments
 (0)