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

Propagate socket write errors for DtlsTransport

Browse Source 
The UDP sockets in WebRTC are non-blocking, and when writing too much
to them so that their send buffer becomes exhausted, they will return
EAGAIN or EWOULDBLOCK, which indicates that the client will need to
retry a bit later.

Media packets are generally sent by the pacer, which generally avoids
this exhaustion, but for SCTP which has a congestion control algorithm
quite similar to TCP, it may overshoot the amount of data it writes. If
the SCTP library can be notified when writing fails, it can stop writing
for a while until the socket recovers, which will result in less
overshooting and fewer lost packets (possibly even none). But for the
SCTP library to be able to know this, errors must be propagated, which
they weren't with the argument that packets may get lost anyway.

Bug: webrtc:12943
Change-Id: I9244580abf0d48ff810da30a23f995d12be623ed
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228439
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34751}
This commit is contained in:
Victor Boivie 2021-08-11 19:35:11 +02:00 committed by WebRTC LUCI CQ
parent 55a2f770a6
commit edfaaef086
3 changed files with 36 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
p2p/base/dtls_transport.cc
View File

@ -93,12 +93,17 @@ rtc::StreamResult StreamInterfaceChannel::Write(const void* data,
size_t* written,
int* error) {
RTC_DCHECK_RUN_ON(&sequence_checker_);
// Always succeeds, since this is an unreliable transport anyway.
// TODO(zhihuang): Should this block if ice_transport_'s temporarily
// unwritable?
rtc::PacketOptions packet_options;
ice_transport_->SendPacket(static_cast<const char*>(data), data_len,
packet_options);
int sent = ice_transport_->SendPacket(static_cast<const char*>(data),
data_len, packet_options);
if (sent < 0) {
if (written) {
*written = 0;
}
return rtc::IsBlockingError(ice_transport_->GetError()) ? rtc::SR_BLOCK
: rtc::SR_ERROR;
}
if (written) {
*written = data_len;
}

10
p2p/base/dtls_transport_unittest.cc
View File

@ -421,6 +421,16 @@ TEST_F(DtlsTransportTest, TestTransferDtls) {
TestTransfer(1000, 100, /*srtp=*/false);
}
// Connect with DTLS, and fail to write, to ensure errors are propagated.
TEST_F(DtlsTransportTest, TestWriteFailsOverDtls) {
PrepareDtls(rtc::KT_DEFAULT);
ASSERT_TRUE(Connect());
client1_.fake_ice_transport()->SetError(EAGAIN);
int res = client1_.dtls_transport()->SendPacket("hello", 5,
rtc::PacketOptions(), 0);
EXPECT_EQ(res, -1);
}
// Connect with DTLS, combine multiple DTLS records into one packet.
// Our DTLS implementation doesn't do this, but other implementations may;
// see https://tools.ietf.org/html/rfc6347#section-4.1.1.

17
p2p/base/fake_ice_transport.h
View File

@ -302,6 +302,10 @@ class FakeIceTransport : public IceTransportInternal {
return -1;
}
if (error_ != 0) {
return -1;
}
send_packet_.AppendData(data, len);
if (!combine_outgoing_packets_ || send_packet_.size() > len) {
rtc::CopyOnWriteBuffer packet(std::move(send_packet_));
@ -338,7 +342,17 @@ class FakeIceTransport : public IceTransportInternal {
}
}
int GetError() override { return 0; }
// Sets the error that is returned by `GetError`. If an error is set (i.e.
// non-zero), `SendPacket` will return -1.
void SetError(int error) {
RTC_DCHECK_RUN_ON(network_thread_);
error_ = error;
}
int GetError() override {
RTC_DCHECK_RUN_ON(network_thread_);
return error_;
}
rtc::CopyOnWriteBuffer last_sent_packet() {
RTC_DCHECK_RUN_ON(network_thread_);
@ -421,6 +435,7 @@ class FakeIceTransport : public IceTransportInternal {
rtc::CopyOnWriteBuffer last_sent_packet_ RTC_GUARDED_BY(network_thread_);
rtc::Thread* const network_thread_;
webrtc::ScopedTaskSafetyDetached task_safety_;
int error_ RTC_GUARDED_BY(network_thread_) = 0;
};
class FakeIceTransportWrapper : public webrtc::IceTransportInterface {