Deprecate gfe2_flags_reloadable_quic_ask_for_short_header_connection_id_length2. PiperOrigin-RevId: 491630415
diff --git a/quiche/quic/core/quic_buffered_packet_store.cc b/quiche/quic/core/quic_buffered_packet_store.cc index a962499..df028f0 100644 --- a/quiche/quic/core/quic_buffered_packet_store.cc +++ b/quiche/quic/core/quic_buffered_packet_store.cc
@@ -198,15 +198,11 @@ // connection ID length when we buffered the packet and indexed by // connection ID. QuicErrorCode error_code = QuicFramer::ParsePublicHeaderDispatcher( - *packet.packet, - GetQuicReloadableFlag(quic_ask_for_short_header_connection_id_length2) - ? connection_id.length() - : kQuicDefaultConnectionIdLength, - &unused_format, &long_packet_type, &unused_version_flag, - &unused_use_length_prefix, &unused_version_label, - &unused_parsed_version, &unused_destination_connection_id, - &unused_source_connection_id, &unused_retry_token, - &unused_detailed_error); + *packet.packet, connection_id.length(), &unused_format, + &long_packet_type, &unused_version_flag, &unused_use_length_prefix, + &unused_version_label, &unused_parsed_version, + &unused_destination_connection_id, &unused_source_connection_id, + &unused_retry_token, &unused_detailed_error); if (error_code == QUIC_NO_ERROR && long_packet_type == INITIAL) { initial_packets.push_back(std::move(packet));
diff --git a/quiche/quic/core/quic_dispatcher.cc b/quiche/quic/core/quic_dispatcher.cc index c851171..0f8c1ab 100644 --- a/quiche/quic/core/quic_dispatcher.cc +++ b/quiche/quic/core/quic_dispatcher.cc
@@ -306,24 +306,13 @@ ReceivedPacketInfo packet_info(self_address, peer_address, packet); std::string detailed_error; QuicErrorCode error; - if (GetQuicReloadableFlag(quic_ask_for_short_header_connection_id_length2)) { - QUIC_RELOADABLE_FLAG_COUNT(quic_ask_for_short_header_connection_id_length2); - error = QuicFramer::ParsePublicHeaderDispatcherShortHeaderLengthUnknown( - packet, &packet_info.form, &packet_info.long_packet_type, - &packet_info.version_flag, &packet_info.use_length_prefix, - &packet_info.version_label, &packet_info.version, - &packet_info.destination_connection_id, - &packet_info.source_connection_id, &packet_info.retry_token, - &detailed_error, connection_id_generator_); - } else { - error = QuicFramer::ParsePublicHeaderDispatcher( - packet, expected_server_connection_id_length_, &packet_info.form, - &packet_info.long_packet_type, &packet_info.version_flag, - &packet_info.use_length_prefix, &packet_info.version_label, - &packet_info.version, &packet_info.destination_connection_id, - &packet_info.source_connection_id, &packet_info.retry_token, - &detailed_error); - } + error = QuicFramer::ParsePublicHeaderDispatcherShortHeaderLengthUnknown( + packet, &packet_info.form, &packet_info.long_packet_type, + &packet_info.version_flag, &packet_info.use_length_prefix, + &packet_info.version_label, &packet_info.version, + &packet_info.destination_connection_id, &packet_info.source_connection_id, + &packet_info.retry_token, &detailed_error, connection_id_generator_); + if (error != QUIC_NO_ERROR) { // Packet has framing error. SetLastError(error); @@ -361,12 +350,6 @@ // Before introducing the flag, it was impossible for a short header to // update |expected_server_connection_id_length_|. - QUIC_BUG_IF( - quic_bug_480483284_01, - !GetQuicReloadableFlag(quic_ask_for_short_header_connection_id_length2) && - !packet_info.version_flag && - packet_info.destination_connection_id.length() != - expected_server_connection_id_length_); if (should_update_expected_server_connection_id_length_ && packet_info.version_flag) { expected_server_connection_id_length_ = @@ -385,7 +368,6 @@ // where NEW_CONNECTION_IDs are not using the generator, and the dispatcher // is, due to flag misconfiguration. if (!packet_info.version_flag && - GetQuicReloadableFlag(quic_ask_for_short_header_connection_id_length2) && (IsSupportedVersion(ParsedQuicVersion::Q046()) || IsSupportedVersion(ParsedQuicVersion::Q050()))) { ReceivedPacketInfo gquic_packet_info(self_address, peer_address, packet); @@ -1254,9 +1236,6 @@ allow_short_initial_server_connection_ids_) { return false; } - if (!GetQuicReloadableFlag(quic_ask_for_short_header_connection_id_length2)) { - return true; - } uint8_t generator_output = connection_id.IsEmpty() ? connection_id_generator_.ConnectionIdLength(0x00)
diff --git a/quiche/quic/core/quic_dispatcher_test.cc b/quiche/quic/core/quic_dispatcher_test.cc index 9341cde..3ed2330 100644 --- a/quiche/quic/core/quic_dispatcher_test.cc +++ b/quiche/quic/core/quic_dispatcher_test.cc
@@ -326,18 +326,14 @@ ConstructReceivedPacket(*packet, mock_helper_.GetClock()->Now())); // Call ConnectionIdLength if the packet clears the Long Header bit, or // if the test involves sending a connection ID that is too short - if ((!has_version_flag || !version.AllowsVariableLengthConnectionIds() || - server_connection_id.length() == 0 || - server_connection_id_included == CONNECTION_ID_ABSENT) && - GetQuicReloadableFlag( - quic_ask_for_short_header_connection_id_length2)) { + if (!has_version_flag || !version.AllowsVariableLengthConnectionIds() || + server_connection_id.length() == 0 || + server_connection_id_included == CONNECTION_ID_ABSENT) { // Short headers will ask for the length EXPECT_CALL(connection_id_generator_, ConnectionIdLength(_)) .WillRepeatedly(Return(generated_connection_id_.has_value() ? generated_connection_id_->length() : kQuicDefaultConnectionIdLength)); - } else { - EXPECT_CALL(connection_id_generator_, ConnectionIdLength(_)).Times(0); } ProcessReceivedPacket(std::move(received_packet), peer_address, version, server_connection_id); @@ -631,23 +627,9 @@ }))); ProcessFirstFlight(client_address, old_id); - // Send short header packets with the new length and verify they are parsed - // correctly. If flag is set, all versions should succeed. If not, it should - // fail (this is the bugfix!). gQUIC never gets a new connection ID, so it's - // not affected by asking. - if (version_.HasIetfQuicFrames() && - !GetQuicReloadableFlag(quic_ask_for_short_header_connection_id_length2)) { - // Dispatcher issued a longer connection ID if IETF QUIC, but won't ask for - // that length when processing a short header. Thus dispatch will fail. - EXPECT_CALL(*reinterpret_cast<MockQuicConnection*>(session1_->connection()), - ProcessUdpPacket(_, _, _)) - .Times(0); - } else { - // Dispatch succeeds, because it's gQUIC or all the flags are aligned. - EXPECT_CALL(*reinterpret_cast<MockQuicConnection*>(session1_->connection()), - ProcessUdpPacket(_, _, _)) - .Times(1); - } + EXPECT_CALL(*reinterpret_cast<MockQuicConnection*>(session1_->connection()), + ProcessUdpPacket(_, _, _)) + .Times(1); ProcessPacket(client_address, new_id, false, "foo"); } @@ -995,8 +977,7 @@ EXPECT_CALL(*time_wait_list_manager_, AddConnectionIdToTimeWait(_, _)) .Times(0); // Verify small packet is silently dropped. - if (GetQuicReloadableFlag(quic_ask_for_short_header_connection_id_length2) && - version_.HasIetfInvariantHeader()) { + if (version_.HasIetfInvariantHeader()) { EXPECT_CALL(connection_id_generator_, ConnectionIdLength(0xa7)) .WillOnce(Return(kQuicDefaultConnectionIdLength)); } else { @@ -1005,8 +986,7 @@ EXPECT_CALL(*time_wait_list_manager_, SendPublicReset(_, _, _, _, _, _)) .Times(0); dispatcher_->ProcessPacket(server_address_, client_address, packet); - if (GetQuicReloadableFlag(quic_ask_for_short_header_connection_id_length2) && - version_.HasIetfInvariantHeader()) { + if (version_.HasIetfInvariantHeader()) { EXPECT_CALL(connection_id_generator_, ConnectionIdLength(0xa7)) .WillOnce(Return(kQuicDefaultConnectionIdLength)); } else { @@ -1030,12 +1010,8 @@ .Times(0); EXPECT_CALL(*time_wait_list_manager_, SendPublicReset(_, _, _, _, _, _)) .Times(0); - if (GetQuicReloadableFlag(quic_ask_for_short_header_connection_id_length2)) { - EXPECT_CALL(connection_id_generator_, ConnectionIdLength(_)) - .WillOnce(Return(kQuicDefaultConnectionIdLength)); - } else { - EXPECT_CALL(connection_id_generator_, ConnectionIdLength(_)).Times(0); - } + EXPECT_CALL(connection_id_generator_, ConnectionIdLength(_)) + .WillOnce(Return(kQuicDefaultConnectionIdLength)); dispatcher_->ProcessPacket(server_address_, client_address, packet); } @@ -1286,12 +1262,8 @@ QuicSocketAddress client_address(QuicIpAddress::Loopback4(), 1); // dispatcher_ should drop this packet. - if (GetQuicReloadableFlag(quic_ask_for_short_header_connection_id_length2)) { - EXPECT_CALL(connection_id_generator_, ConnectionIdLength(0x00)) - .WillOnce(Return(10)); - } else { - EXPECT_CALL(connection_id_generator_, ConnectionIdLength(_)).Times(0); - } + EXPECT_CALL(connection_id_generator_, ConnectionIdLength(0x00)) + .WillOnce(Return(10)); EXPECT_CALL(*dispatcher_, CreateQuicSession(_, _, _, _, _, _)).Times(0); EXPECT_CALL(*time_wait_list_manager_, ProcessPacket(_, _, _, _, _, _)) .Times(0);
diff --git a/quiche/quic/core/quic_flags_list.h b/quiche/quic/core/quic_flags_list.h index 0b148a1..aeefafe 100644 --- a/quiche/quic/core/quic_flags_list.h +++ b/quiche/quic/core/quic_flags_list.h
@@ -91,8 +91,6 @@ QUIC_FLAG(quic_reloadable_flag_quic_priority_update_structured_headers_parser, false) // If true, uses conservative cwnd gain and pacing gain when cwnd gets bootstrapped. QUIC_FLAG(quic_reloadable_flag_quic_conservative_cwnd_and_pacing_gains, false) -// Instead of assuming an incoming connection ID length for short headers, ask each time. -QUIC_FLAG(quic_reloadable_flag_quic_ask_for_short_header_connection_id_length2, true) // When true, defaults to BBR congestion control instead of Cubic. QUIC_FLAG(quic_reloadable_flag_quic_default_to_bbr, false) // When true, support draft-ietf-quic-v2-01