admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "dcsctp: Negotiate zero checksum"

Browse Source 
This reverts commit a736f30a5fddfa9a6af02a0a916da09bcac49d0d.

Due to a downstream project not supporting this
new handover state, it fails. Handover state needs
to be submitted first, then downstream project needs
to be updated, and finally the code changes can be
submitted.

Bug: webrtc:14997
Change-Id: I8551e349408d9bf4eb593cb948279d659467fe20
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/302821
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Auto-Submit: Victor Boivie <boivie@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39923}
This commit is contained in:
Victor Boivie 2023-04-23 21:10:09 +00:00 committed by WebRTC LUCI CQ
parent e91c76875a
commit 014cbed9d2
13 changed files with 13 additions and 208 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
net/dcsctp/public/dcsctp_handover_state.h
View File

@ -40,7 +40,6 @@ struct DcSctpSocketHandoverState {
bool partial_reliability = false;
bool message_interleaving = false;
bool reconfig = false;
bool zero_checksum = false;
uint16_t negotiated_maximum_incoming_streams = 0;
uint16_t negotiated_maximum_outgoing_streams = 0;
};

4
net/dcsctp/public/dcsctp_socket.h
View File

@ -245,10 +245,6 @@ struct Metrics {
// peers.
bool uses_message_interleaving = false;
// Indicates if draft-tuexen-tsvwg-sctp-zero-checksum-00 has been negotiated
// by both peers.
bool uses_zero_checksum = false;
// The number of negotiated incoming and outgoing streams, which is configured
// locally as `DcSctpOptions::announced_maximum_incoming_streams` and
// `DcSctpOptions::announced_maximum_outgoing_streams`, and which will be

2
net/dcsctp/socket/capabilities.h
View File

@ -21,8 +21,6 @@ struct Capabilities {
bool message_interleaving = false;
// RFC6525 Stream Reconfiguration
bool reconfig = false;
// https://datatracker.ietf.org/doc/draft-tuexen-tsvwg-sctp-zero-checksum/
bool zero_checksum = false;
// Negotiated maximum incoming and outgoing stream count.
uint16_t negotiated_maximum_incoming_streams = 0;
uint16_t negotiated_maximum_outgoing_streams = 0;

20
net/dcsctp/socket/dcsctp_socket.cc
View File

@ -56,7 +56,6 @@
#include "net/dcsctp/packet/parameter/parameter.h"
#include "net/dcsctp/packet/parameter/state_cookie_parameter.h"
#include "net/dcsctp/packet/parameter/supported_extensions_parameter.h"
#include "net/dcsctp/packet/parameter/zero_checksum_acceptable_chunk_parameter.h"
#include "net/dcsctp/packet/sctp_packet.h"
#include "net/dcsctp/packet/tlv_trait.h"
#include "net/dcsctp/public/dcsctp_message.h"
@ -118,11 +117,6 @@ Capabilities ComputeCapabilities(const DcSctpOptions& options,
capabilities.reconfig = true;
}
if (options.enable_zero_checksum &&
parameters.get<ZeroChecksumAcceptableChunkParameter>().has_value()) {
capabilities.zero_checksum = true;
}
capabilities.negotiated_maximum_incoming_streams = std::min(
options.announced_maximum_incoming_streams, peer_nbr_outbound_streams);
capabilities.negotiated_maximum_outgoing_streams = std::min(
@ -143,9 +137,6 @@ void AddCapabilityParameters(const DcSctpOptions& options,
chunk_types.push_back(IDataChunk::kType);
chunk_types.push_back(IForwardTsnChunk::kType);
}
if (options.enable_zero_checksum) {
builder.Add(ZeroChecksumAcceptableChunkParameter());
}
builder.Add(SupportedExtensionsParameter(std::move(chunk_types)));
}
@ -288,10 +279,7 @@ void DcSctpSocket::SendInit() {
connect_params_.initial_tsn, params_builder.Build());
SctpPacket::Builder b(VerificationTag(0), options_);
b.Add(init);
// https://www.ietf.org/archive/id/draft-tuexen-tsvwg-sctp-zero-checksum-01.html#section-4.2
// "When an end point sends a packet containing an INIT chunk, it MUST include
// a correct CRC32c checksum in the packet containing the INIT chunk."
packet_sender_.Send(b, /*write_checksum=*/true);
packet_sender_.Send(b);
}
void DcSctpSocket::MakeConnectionParameters() {
@ -332,7 +320,6 @@ void DcSctpSocket::CreateTransmissionControlBlock(
size_t a_rwnd,
TieTag tie_tag) {
metrics_.uses_message_interleaving = capabilities.message_interleaving;
metrics_.uses_zero_checksum = capabilities.zero_checksum;
metrics_.negotiated_maximum_incoming_streams =
capabilities.negotiated_maximum_incoming_streams;
metrics_.negotiated_maximum_outgoing_streams =
@ -364,7 +351,6 @@ void DcSctpSocket::RestoreFromState(const DcSctpSocketHandoverState& state) {
capabilities.message_interleaving =
state.capabilities.message_interleaving;
capabilities.reconfig = state.capabilities.reconfig;
capabilities.zero_checksum = state.capabilities.zero_checksum;
capabilities.negotiated_maximum_incoming_streams =
state.capabilities.negotiated_maximum_incoming_streams;
capabilities.negotiated_maximum_outgoing_streams =
@ -1223,9 +1209,7 @@ void DcSctpSocket::HandleInit(const CommonHeader& header,
options_.announced_maximum_incoming_streams,
connect_params_.initial_tsn, params_builder.Build());
b.Add(init_ack);
// If the peer has signaled that it supports zero checksum, INIT-ACK can then
// have its checksum as zero.
packet_sender_.Send(b, /*write_checksum=*/!capabilities.zero_checksum);
packet_sender_.Send(b);
}
void DcSctpSocket::HandleInitAck(

155
net/dcsctp/socket/dcsctp_socket_test.cc
View File

@ -23,7 +23,6 @@
#include "api/array_view.h"
#include "net/dcsctp/common/handover_testing.h"
#include "net/dcsctp/packet/chunk/chunk.h"
#include "net/dcsctp/packet/chunk/cookie_ack_chunk.h"
#include "net/dcsctp/packet/chunk/cookie_echo_chunk.h"
#include "net/dcsctp/packet/chunk/data_chunk.h"
#include "net/dcsctp/packet/chunk/data_common.h"
@ -31,7 +30,6 @@
#include "net/dcsctp/packet/chunk/heartbeat_ack_chunk.h"
#include "net/dcsctp/packet/chunk/heartbeat_request_chunk.h"
#include "net/dcsctp/packet/chunk/idata_chunk.h"
#include "net/dcsctp/packet/chunk/init_ack_chunk.h"
#include "net/dcsctp/packet/chunk/init_chunk.h"
#include "net/dcsctp/packet/chunk/sack_chunk.h"
#include "net/dcsctp/packet/chunk/shutdown_chunk.h"
@ -60,10 +58,8 @@ namespace {
using ::testing::_;
using ::testing::AllOf;
using ::testing::ElementsAre;
using ::testing::Eq;
using ::testing::HasSubstr;
using ::testing::IsEmpty;
using ::testing::Not;
using ::testing::SizeIs;
using ::testing::UnorderedElementsAre;
@ -2868,156 +2864,5 @@ TEST(DcSctpSocketTest, ResetStreamsWithPausedSenderResumesWhenPerformed) {
EXPECT_EQ(msg2->payload().size(), kSmallMessageSize);
}
TEST_P(DcSctpSocketParametrizedTest, ZeroChecksumMetricsAreSet) {
std::vector<std::pair<bool, bool>> combinations = {
{false, false}, {false, true}, {true, false}, {true, true}};
for (const auto& [a_enable, z_enable] : combinations) {
DcSctpOptions a_options = {.enable_zero_checksum = a_enable};
DcSctpOptions z_options = {.enable_zero_checksum = z_enable};
SocketUnderTest a("A", a_options);
auto z = std::make_unique<SocketUnderTest>("Z", z_options);
ConnectSockets(a, *z);
z = MaybeHandoverSocket(std::move(z));
EXPECT_EQ(a.socket.GetMetrics()->uses_zero_checksum, a_enable && z_enable);
EXPECT_EQ(z->socket.GetMetrics()->uses_zero_checksum, a_enable && z_enable);
}
}
TEST(DcSctpSocketTest, AlwaysSendsInitWithNonZeroChecksum) {
DcSctpOptions options = {.enable_zero_checksum = true};
SocketUnderTest a("A", options);
a.socket.Connect();
std::vector<uint8_t> data = a.cb.ConsumeSentPacket();
ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
SctpPacket::Parse(data, options));
EXPECT_THAT(packet.descriptors(),
ElementsAre(testing::Field(&SctpPacket::ChunkDescriptor::type,
InitChunk::kType)));
EXPECT_THAT(packet.common_header().checksum, Not(Eq(0u)));
}
TEST(DcSctpSocketTest, MaySendInitAckWithZeroChecksum) {
DcSctpOptions options = {.enable_zero_checksum = true};
SocketUnderTest a("A", options);
SocketUnderTest z("Z", options);
a.socket.Connect();
z.socket.ReceivePacket(a.cb.ConsumeSentPacket()); // INIT
std::vector<uint8_t> data = z.cb.ConsumeSentPacket();
ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
SctpPacket::Parse(data, options));
EXPECT_THAT(packet.descriptors(),
ElementsAre(testing::Field(&SctpPacket::ChunkDescriptor::type,
InitAckChunk::kType)));
EXPECT_THAT(packet.common_header().checksum, 0u);
}
TEST(DcSctpSocketTest, AlwaysSendsCookieEchoWithNonZeroChecksum) {
DcSctpOptions options = {.enable_zero_checksum = true};
SocketUnderTest a("A", options);
SocketUnderTest z("Z", options);
a.socket.Connect();
z.socket.ReceivePacket(a.cb.ConsumeSentPacket()); // INIT
a.socket.ReceivePacket(z.cb.ConsumeSentPacket()); // INIT-ACK
std::vector<uint8_t> data = a.cb.ConsumeSentPacket();
ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
SctpPacket::Parse(data, options));
EXPECT_THAT(packet.descriptors(),
ElementsAre(testing::Field(&SctpPacket::ChunkDescriptor::type,
CookieEchoChunk::kType)));
EXPECT_THAT(packet.common_header().checksum, Not(Eq(0u)));
}
TEST(DcSctpSocketTest, SendsCookieAckWithZeroChecksum) {
DcSctpOptions options = {.enable_zero_checksum = true};
SocketUnderTest a("A", options);
SocketUnderTest z("Z", options);
a.socket.Connect();
z.socket.ReceivePacket(a.cb.ConsumeSentPacket()); // INIT
a.socket.ReceivePacket(z.cb.ConsumeSentPacket()); // INIT-ACK
z.socket.ReceivePacket(a.cb.ConsumeSentPacket()); // COOKIE-ECHO
std::vector<uint8_t> data = z.cb.ConsumeSentPacket();
ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
SctpPacket::Parse(data, options));
EXPECT_THAT(packet.descriptors(),
ElementsAre(testing::Field(&SctpPacket::ChunkDescriptor::type,
CookieAckChunk::kType)));
EXPECT_THAT(packet.common_header().checksum, 0u);
}
TEST_P(DcSctpSocketParametrizedTest, SendsDataWithZeroChecksum) {
DcSctpOptions options = {.enable_zero_checksum = true};
SocketUnderTest a("A", options);
auto z = std::make_unique<SocketUnderTest>("Z", options);
ConnectSockets(a, *z);
z = MaybeHandoverSocket(std::move(z));
std::vector<uint8_t> payload(a.options.mtu - 100);
a.socket.Send(DcSctpMessage(StreamID(1), PPID(53), payload), {});
std::vector<uint8_t> data = a.cb.ConsumeSentPacket();
z->socket.ReceivePacket(data);
ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
SctpPacket::Parse(data, options));
EXPECT_THAT(packet.descriptors(),
ElementsAre(testing::Field(&SctpPacket::ChunkDescriptor::type,
DataChunk::kType)));
EXPECT_THAT(packet.common_header().checksum, 0u);
MaybeHandoverSocketAndSendMessage(a, std::move(z));
}
TEST_P(DcSctpSocketParametrizedTest, AllPacketsAfterConnectHaveZeroChecksum) {
DcSctpOptions options = {.enable_zero_checksum = true};
SocketUnderTest a("A", options);
auto z = std::make_unique<SocketUnderTest>("Z", options);
ConnectSockets(a, *z);
z = MaybeHandoverSocket(std::move(z));
// Send large messages in both directions, and verify that they arrive and
// that every packet has zero checksum.
std::vector<uint8_t> payload(kLargeMessageSize);
a.socket.Send(DcSctpMessage(StreamID(1), PPID(53), payload), kSendOptions);
z->socket.Send(DcSctpMessage(StreamID(1), PPID(53), payload), kSendOptions);
for (;;) {
if (auto data = a.cb.ConsumeSentPacket(); !data.empty()) {
ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
SctpPacket::Parse(data, options));
EXPECT_THAT(packet.common_header().checksum, 0u);
z->socket.ReceivePacket(std::move(data));
} else if (auto data = z->cb.ConsumeSentPacket(); !data.empty()) {
ASSERT_HAS_VALUE_AND_ASSIGN(SctpPacket packet,
SctpPacket::Parse(data, options));
EXPECT_THAT(packet.common_header().checksum, 0u);
a.socket.ReceivePacket(std::move(data));
} else {
break;
}
}
absl::optional<DcSctpMessage> msg1 = z->cb.ConsumeReceivedMessage();
ASSERT_TRUE(msg1.has_value());
EXPECT_THAT(msg1->payload(), SizeIs(kLargeMessageSize));
absl::optional<DcSctpMessage> msg2 = a.cb.ConsumeReceivedMessage();
ASSERT_TRUE(msg2.has_value());
EXPECT_THAT(msg2->payload(), SizeIs(kLargeMessageSize));
MaybeHandoverSocketAndSendMessage(a, std::move(z));
}
} // namespace
} // namespace dcsctp

4
net/dcsctp/socket/packet_sender.cc
View File

@ -21,12 +21,12 @@ PacketSender::PacketSender(DcSctpSocketCallbacks& callbacks,
SendPacketStatus)> on_sent_packet)
: callbacks_(callbacks), on_sent_packet_(std::move(on_sent_packet)) {}
bool PacketSender::Send(SctpPacket::Builder& builder, bool write_checksum) {
bool PacketSender::Send(SctpPacket::Builder& builder) {
if (builder.empty()) {
return false;
}
std::vector<uint8_t> payload = builder.Build(write_checksum);
std::vector<uint8_t> payload = builder.Build();
SendPacketStatus status = callbacks_.SendPacketWithStatus(payload);
on_sent_packet_(payload, status);

2
net/dcsctp/socket/packet_sender.h
View File

@ -25,7 +25,7 @@ class PacketSender {
SendPacketStatus)> on_sent_packet);
// Sends the packet, and returns true if it was sent successfully.
bool Send(SctpPacket::Builder& builder, bool write_checksum = true);
bool Send(SctpPacket::Builder& builder);
private:
DcSctpSocketCallbacks& callbacks_;

2
net/dcsctp/socket/state_cookie.cc
View File

@ -42,7 +42,6 @@ std::vector<uint8_t> StateCookie::Serialize() {
buffer.Store8<30>(capabilities_.reconfig);
buffer.Store16<32>(capabilities_.negotiated_maximum_incoming_streams);
buffer.Store16<34>(capabilities_.negotiated_maximum_outgoing_streams);
buffer.Store8<36>(capabilities_.zero_checksum);
return cookie;
}
@ -75,7 +74,6 @@ absl::optional<StateCookie> StateCookie::Deserialize(
capabilities.reconfig = buffer.Load8<30>() != 0;
capabilities.negotiated_maximum_incoming_streams = buffer.Load16<32>();
capabilities.negotiated_maximum_outgoing_streams = buffer.Load16<34>();
capabilities.zero_checksum = buffer.Load8<36>() != 0;
return StateCookie(verification_tag, initial_tsn, a_rwnd, tie_tag,
capabilities);

2
net/dcsctp/socket/state_cookie.h
View File

@ -27,7 +27,7 @@ namespace dcsctp {
// Do not trust anything in it; no pointers or anything like that.
class StateCookie {
public:
static constexpr size_t kCookieSize = 37;
static constexpr size_t kCookieSize = 36;
StateCookie(VerificationTag initiate_tag,
TSN initial_tsn,

2
net/dcsctp/socket/state_cookie_test.cc
View File

@ -21,7 +21,6 @@ TEST(StateCookieTest, SerializeAndDeserialize) {
Capabilities capabilities = {.partial_reliability = true,
.message_interleaving = false,
.reconfig = true,
.zero_checksum = true,
.negotiated_maximum_incoming_streams = 123,
.negotiated_maximum_outgoing_streams = 234};
StateCookie cookie(VerificationTag(123), TSN(456),
@ -37,7 +36,6 @@ TEST(StateCookieTest, SerializeAndDeserialize) {
EXPECT_TRUE(deserialized.capabilities().partial_reliability);
EXPECT_FALSE(deserialized.capabilities().message_interleaving);
EXPECT_TRUE(deserialized.capabilities().reconfig);
EXPECT_TRUE(deserialized.capabilities().zero_checksum);
EXPECT_EQ(deserialized.capabilities().negotiated_maximum_incoming_streams,
123);
EXPECT_EQ(deserialized.capabilities().negotiated_maximum_outgoing_streams,

16
net/dcsctp/socket/transmission_control_block.cc
View File

@ -163,7 +163,7 @@ void TransmissionControlBlock::MaybeSendForwardTsn(SctpPacket::Builder& builder,
} else {
builder.Add(retransmission_queue_.CreateForwardTsn());
}
Send(builder);
packet_sender_.Send(builder);
// https://datatracker.ietf.org/doc/html/rfc3758
// "IMPLEMENTATION NOTE: An implementation may wish to limit the number of
// duplicate FORWARD TSN chunks it sends by ... waiting a full RTT before
@ -198,7 +198,7 @@ void TransmissionControlBlock::MaybeSendFastRetransmit() {
builder.Add(DataChunk(tsn, std::move(data), false));
}
}
Send(builder);
packet_sender_.Send(builder);
}
void TransmissionControlBlock::SendBufferedPackets(SctpPacket::Builder& builder,
@ -245,13 +245,7 @@ void TransmissionControlBlock::SendBufferedPackets(SctpPacket::Builder& builder,
}
}
// https://www.ietf.org/archive/id/draft-tuexen-tsvwg-sctp-zero-checksum-02.html#section-4.2
// "When an end point sends a packet containing a COOKIE ECHO chunk, it MUST
// include a correct CRC32c checksum in the packet containing the COOKIE
// ECHO chunk."
bool write_checksum =
!capabilities_.zero_checksum || cookie_echo_chunk_.has_value();
if (!packet_sender_.Send(builder, write_checksum)) {
if (!packet_sender_.Send(builder)) {
break;
}
@ -280,9 +274,6 @@ std::string TransmissionControlBlock::ToString() const {
if (capabilities_.reconfig) {
sb << "Reconfig,";
}
if (capabilities_.zero_checksum) {
sb << "ZeroChecksum,";
}
sb << " max_in=" << capabilities_.negotiated_maximum_incoming_streams;
sb << " max_out=" << capabilities_.negotiated_maximum_outgoing_streams;
@ -303,7 +294,6 @@ void TransmissionControlBlock::AddHandoverState(
state.capabilities.partial_reliability = capabilities_.partial_reliability;
state.capabilities.message_interleaving = capabilities_.message_interleaving;
state.capabilities.reconfig = capabilities_.reconfig;
state.capabilities.zero_checksum = capabilities_.zero_checksum;
state.capabilities.negotiated_maximum_incoming_streams =
capabilities_.negotiated_maximum_incoming_streams;
state.capabilities.negotiated_maximum_outgoing_streams =

3
net/dcsctp/socket/transmission_control_block.h
View File

@ -80,8 +80,7 @@ class TransmissionControlBlock : public Context {
return tx_error_counter_.IsExhausted();
}
void Send(SctpPacket::Builder& builder) override {
packet_sender_.Send(builder,
/*write_checksum=*/!capabilities_.zero_checksum);
packet_sender_.Send(builder);
}
// Other accessors

8
net/dcsctp/socket/transmission_control_block_test.cc
View File

@ -92,7 +92,6 @@ TEST_F(TransmissionControlBlockTest, LogsAllCapabilitiesInToSring) {
capabilities_.negotiated_maximum_outgoing_streams = 2000;
capabilities_.message_interleaving = true;
capabilities_.partial_reliability = true;
capabilities_.zero_checksum = true;
capabilities_.reconfig = true;
TransmissionControlBlock tcb(
@ -100,10 +99,9 @@ TEST_F(TransmissionControlBlockTest, LogsAllCapabilitiesInToSring) {
kMyVerificationTag, kMyInitialTsn, kPeerVerificationTag, kPeerInitialTsn,
kArwnd, kTieTag, sender_, on_connection_established.AsStdFunction());
EXPECT_EQ(
tcb.ToString(),
"verification_tag=000001c8, last_cumulative_ack=999, "
"capabilities=PR,IL,Reconfig,ZeroChecksum, max_in=1000 max_out=2000");
EXPECT_EQ(tcb.ToString(),
"verification_tag=000001c8, last_cumulative_ack=999, "
"capabilities=PR,IL,Reconfig, max_in=1000 max_out=2000");
}
TEST_F(TransmissionControlBlockTest, IsInitiallyHandoverReady) {