lib/mvfst
1
0
Fork 0
mirror of https://github.com/facebookincubator/mvfst.git synced 2025-08-08 09:42:06 +03:00
Code Activity

Stop setting clientConnId in ServerStateMachine [2/x]

Browse Source 
Summary:
Removes clientConnId completely from ServerConnectionIdParams.

This diff first fixes an incorrect assumption; it calls
`shortHeader.getConnectionId()` which is actually the destination id (server)
not the client connection id.

Next, this entire block is unnecessary, because this will be called after
the transport is created, so the clientConnectionId will always be set.

This also setsconn.serverConnectionId earlier (shouldn't depend on
connClientId).

Reviewed By: yangchi

Differential Revision: D16792866

fbshipit-source-id: 537ba12baa9939c9d5512e46eb914c1d3a7a9aa2
This commit is contained in:
Viktor Chynarov
2019-08-23 15:03:10 -07:00
committed by Facebook Github Bot
parent 196dd99308
commit 594a98be7a
8 changed files with 4 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
quic/api/test/QuicTransportBaseTest.cpp
View File

@@ -163,7 +163,6 @@ class TestQuicTransport
while (!cursor.isAtEnd()) {
// create server chosen connId with processId = 0 and workerId = 0
ServerConnectionIdParams params(0, 0, 0);
params.clientConnId = *conn_->clientConnectionId;
conn_->serverConnectionId = connIdAlgo_->encodeConnectionId(params);
auto type = static_cast<TestFrameType>(cursor.readBE<uint8_t>());
if (type == TestFrameType::CRYPTO) {
@@ -312,7 +311,6 @@ class TestQuicTransport
void setServerConnectionId() {
// create server chosen connId with processId = 0 and workerId = 0
ServerConnectionIdParams params(0, 0, 0);
params.clientConnId = *conn_->clientConnectionId;
conn_->serverConnectionId = connIdAlgo_->encodeConnectionId(params);
}

2
quic/client/test/QuicClientTransportTest.cpp
View File

@@ -1298,7 +1298,6 @@ class QuicClientTransportTest : public Test {
void setConnectionIds() {
originalConnId = client->getConn().clientConnectionId;
ServerConnectionIdParams params(0, 0, 0);
params.clientConnId = *client->getConn().clientConnectionId;
serverChosenConnId = connIdAlgo_->encodeConnectionId(params);
}
@@ -2471,7 +2470,6 @@ class QuicClientTransportVersionAndRetryTest
originalConnId = client->getConn().clientConnectionId;
// create server chosen connId with processId = 0 and workerId = 0
ServerConnectionIdParams params(0, 0, 0);
params.clientConnId = *client->getConn().clientConnectionId;
serverChosenConnId = connIdAlgo_->encodeConnectionId(params);
// The tests that we do here create streams before crypto is finished,
// so we initialize the peer streams, to allow for this behavior. TODO: when

1
quic/codec/DefaultConnectionIdAlgo.cpp
View File

@@ -180,7 +180,6 @@ ServerConnectionIdParams DefaultConnectionIdAlgo::parseConnectionId(
getHostIdBitsInConnId(id),
getProcessIdBitsFromConnId(id),
getWorkerIdFromConnId(id));
serverConnIdParams.clientConnId.assign(id);
return serverConnIdParams;
}

1
quic/codec/QuicConnectionId.h
View File

@@ -110,7 +110,6 @@ struct ServerConnectionIdParams {
*/
void setWorkerId(uint8_t workerIdIn);
folly::Optional<ConnectionId> clientConnId;
// Quic connection-id short version
uint8_t version{0};
// Quic Host id

2
quic/codec/test/TypesTest.cpp
View File

@@ -199,13 +199,11 @@ TEST_F(TypesTest, KeyPhase) {
TEST_F(TypesTest, TestConnIdWorkerId) {
std::vector<uint8_t> connIdData(kDefaultConnectionIdSize);
folly::Random::secureRandom(connIdData.data(), connIdData.size());
ConnectionId randConnId(connIdData);
auto connIdAlgo = std::make_unique<DefaultConnectionIdAlgo>();
for (uint8_t i = 0; i <= 254; i++) {
uint8_t processId = i % 2;
uint16_t hostId = folly::Random::rand32() % 4095;
ServerConnectionIdParams params(hostId, processId, i);
params.clientConnId = randConnId;
auto paramsAfterEncode =
connIdAlgo->parseConnectionId(connIdAlgo->encodeConnectionId(params));
EXPECT_TRUE(connIdAlgo->canParse(connIdAlgo->encodeConnectionId(params)));

2
quic/loss/test/QuicLossFunctionsTest.cpp
View File

@@ -80,7 +80,6 @@ class QuicLossFunctionsTest : public TestWithParam<PacketNumberSpace> {
// create a serverConnectionId that is different from the client connId
// with bits for processId and workerId set to 0
ServerConnectionIdParams params(0, 0, 0);
params.clientConnId = *conn->clientConnectionId;
conn->connIdAlgo = connIdAlgo_.get();
conn->serverConnectionId = connIdAlgo_->encodeConnectionId(params);
return conn;
@@ -105,7 +104,6 @@ class QuicLossFunctionsTest : public TestWithParam<PacketNumberSpace> {
// create a serverConnectionId that is different from the client connId
// with bits for processId and workerId set to 0
ServerConnectionIdParams params(0, 0, 0);
params.clientConnId = *conn->clientConnectionId;
conn->connIdAlgo = connIdAlgo_.get();
conn->serverConnectionId = connIdAlgo_->encodeConnectionId(params);
return conn;

27
quic/server/state/ServerStateMachine.cpp
View File

@@ -501,6 +501,7 @@ void onServerReadDataFromOpen(
FizzCryptoFactory(&fizzFactory)
.getClientInitialCipher(initialDestinationConnectionId, version));
conn.readCodec->setClientConnectionId(clientConnectionId);
conn.readCodec->setServerConnectionId(*conn.serverConnectionId);
if (conn.qLogger) {
conn.qLogger->scid = conn.serverConnectionId;
conn.qLogger->dcid = clientConnectionId;
@@ -686,31 +687,11 @@ void onServerReadDataFromOpen(
}
}
// TODO: remove this when we actually negotiate connid and version
if (!conn.clientConnectionId) {
conn.clientConnectionId = folly::variant_match(
regularPacket.header,
[](const LongHeader& longHeader) {
return longHeader.getSourceConnId();
},
[](const ShortHeader& shortHeader) {
return shortHeader.getConnectionId();
});
// change the connection id when we switch
CHECK(conn.clientConnectionId);
// TODO: if conn.serverConnIdParams->clientConnId != conn.clientConnId,
// we need to update sourceAddressMap_.
// TODO: need to remove ServerConnectionIdParams::clientConnId, it is no
// longer needed.
conn.serverConnIdParams->clientConnId = *conn.clientConnectionId;
conn.readCodec->setServerConnectionId(*conn.serverConnectionId);
if (conn.qLogger) {
conn.qLogger->dcid = conn.clientConnectionId;
conn.qLogger->scid = conn.serverConnectionId;
}
}
CHECK(conn.clientConnectionId);
if (conn.qLogger) {
conn.qLogger->addPacket(regularPacket, packetSize);
conn.qLogger->dcid = conn.clientConnectionId;
conn.qLogger->scid = conn.serverConnectionId;
}
QUIC_TRACE(packet_recvd, conn, packetNum, packetSize);
// We assume that the higher layer takes care of validating that the version

2
quic/state/stream/test/StreamStateMachineTest.cpp
View File

@@ -430,7 +430,6 @@ TEST_F(QuicHalfClosedRemoteStateTest, AckStream) {
auto conn = createConn();
// create server chosen connId with processId = 0 and workerId = 0
ServerConnectionIdParams params(0, 0, 0);
params.clientConnId = *conn->clientConnectionId;
auto connIdAlgo = std::make_unique<DefaultConnectionIdAlgo>();
folly::Optional<ConnectionId> serverChosenConnId =
connIdAlgo->encodeConnectionId(params);
@@ -470,7 +469,6 @@ TEST_F(QuicHalfClosedRemoteStateTest, AckStreamAfterSkip) {
auto conn = createConn();
// create server chosen connId with processId = 0 and workerId = 0
ServerConnectionIdParams params(0, 0, 0);
params.clientConnId = *conn->clientConnectionId;
auto connIdAlgo = std::make_unique<DefaultConnectionIdAlgo>();
folly::Optional<ConnectionId> serverChosenConnId =
connIdAlgo->encodeConnectionId(params);