zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

dcsctp: Add API to indicate packet send status

Browse Source 
Before this change, there was no way for a client to indicate to the
dcSCTP library if a packet that was supposed to be sent, was actually
sent. It was assumed that it always was.

To handle temporary failures better, such as retrying to send packets
that failed to be sent when the send buffer was full, this information
is propagated to the library.

Note that this change only covers the API and adaptations to clients.
The actual implementation to make use of this information is done as a
follow-up change.

Bug: webrtc:12943
Change-Id: I8f9c62e17f1de1566fa6b0f13a57a3db9f4e7684
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/228563
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#34767}
This commit is contained in:
Victor Boivie
2021-08-12 15:21:25 +02:00
committed by WebRTC LUCI CQ
parent 19214818d7
commit 8df32eb0e1
9 changed files with 59 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
media/BUILD.gn
View File

@ -411,6 +411,7 @@ if (rtc_build_dcsctp) {
"../p2p:rtc_p2p",
"../rtc_base:checks",
"../rtc_base:rtc_base_approved",
"../rtc_base:socket",
"../rtc_base:threading",
"../rtc_base/task_utils:pending_task_safety_flag",
"../rtc_base/task_utils:to_queued_task",

15
media/sctp/dcsctp_transport.cc
View File

@ -26,6 +26,7 @@
#include "p2p/base/packet_transport_internal.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/socket.h"
#include "rtc_base/strings/string_builder.h"
#include "rtc_base/thread.h"
#include "rtc_base/trace_event.h"
@ -34,6 +35,7 @@
namespace webrtc {
namespace {
using ::dcsctp::SendPacketStatus;
enum class WebrtcPPID : dcsctp::PPID::UnderlyingType {
// https://www.rfc-editor.org/rfc/rfc8832.html#section-8.1
@ -308,7 +310,8 @@ void DcSctpTransport::set_debug_name_for_testing(const char* debug_name) {
debug_name_ = debug_name;
}
void DcSctpTransport::SendPacket(rtc::ArrayView<const uint8_t> data) {
SendPacketStatus DcSctpTransport::SendPacketWithStatus(
rtc::ArrayView<const uint8_t> data) {
RTC_DCHECK_RUN_ON(network_thread_);
RTC_DCHECK(socket_);
@ -318,12 +321,12 @@ void DcSctpTransport::SendPacket(rtc::ArrayView<const uint8_t> data) {
"SCTP seems to have made a packet that is bigger "
"than its official MTU: "
<< data.size() << " vs max of " << socket_->options().mtu;
return;
return SendPacketStatus::kError;
}
TRACE_EVENT0("webrtc", "DcSctpTransport::SendPacket");
if (!transport_ || !transport_->writable())
return;
return SendPacketStatus::kError;
RTC_LOG(LS_VERBOSE) << debug_name_ << "->SendPacket(length=" << data.size()
<< ")";
@ -336,7 +339,13 @@ void DcSctpTransport::SendPacket(rtc::ArrayView<const uint8_t> data) {
RTC_LOG(LS_WARNING) << debug_name_ << "->SendPacket(length=" << data.size()
<< ") failed with error: " << transport_->GetError()
<< ".";
if (rtc::IsBlockingError(transport_->GetError())) {
return SendPacketStatus::kTemporaryFailure;
}
return SendPacketStatus::kError;
}
return SendPacketStatus::kSuccess;
}
std::unique_ptr<dcsctp::Timeout> DcSctpTransport::CreateTimeout() {

3
media/sctp/dcsctp_transport.h
View File

@ -59,7 +59,8 @@ class DcSctpTransport : public cricket::SctpTransportInternal,
private:
// dcsctp::DcSctpSocketCallbacks
void SendPacket(rtc::ArrayView<const uint8_t> data) override;
dcsctp::SendPacketStatus SendPacketWithStatus(
rtc::ArrayView<const uint8_t> data) override;
std::unique_ptr<dcsctp::Timeout> CreateTimeout() override;
dcsctp::TimeMs TimeMillis() override;
uint32_t GetRandomInt(uint32_t low, uint32_t high) override;

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

@ -155,6 +155,19 @@ inline constexpr absl::string_view ToString(ResetStreamsStatus error) {
}
}
// Return value of DcSctpSocketCallbacks::SendPacketWithStatus.
enum class SendPacketStatus {
// Indicates that the packet was successfully sent. As sending is unreliable,
// there are no guarantees that the packet was actually delivered.
kSuccess,
// The packet was not sent due to a temporary failure, such as the local send
// buffer becoming exhausted. This return value indicates that the socket will
// recover and sending that packet can be retried at a later time.
kTemporaryFailure,
// The packet was not sent due to other reasons.
kError,
};
// Tracked metrics, which is the return value of GetMetrics. Optional members
// will be unset when they are not yet known.
struct Metrics {
@ -205,9 +218,22 @@ class DcSctpSocketCallbacks {
// Called when the library wants the packet serialized as `data` to be sent.
//
// TODO(bugs.webrtc.org/12943): This method is deprecated, see
// `SendPacketWithStatus`.
//
// Note that it's NOT ALLOWED to call into this library from within this
// callback.
virtual void SendPacket(rtc::ArrayView<const uint8_t> data) = 0;
virtual void SendPacket(rtc::ArrayView<const uint8_t> data) {}
// Called when the library wants the packet serialized as `data` to be sent.
//
// Note that it's NOT ALLOWED to call into this library from within this
// callback.
virtual SendPacketStatus SendPacketWithStatus(
rtc::ArrayView<const uint8_t> data) {
SendPacket(data);
return SendPacketStatus::kSuccess;
}
// Called when the library wants to create a Timeout. The callback must return
// an object that implements that interface.

5
net/dcsctp/socket/callback_deferrer.h
View File

@ -59,9 +59,10 @@ class CallbackDeferrer : public DcSctpSocketCallbacks {
}
}
void SendPacket(rtc::ArrayView<const uint8_t> data) override {
SendPacketStatus SendPacketWithStatus(
rtc::ArrayView<const uint8_t> data) override {
// Will not be deferred - call directly.
underlying_.SendPacket(data);
return underlying_.SendPacketWithStatus(data);
}
std::unique_ptr<Timeout> CreateTimeout() override {

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

@ -858,7 +858,7 @@ void DcSctpSocket::SendPacket(SctpPacket::Builder& builder) {
packet_observer_->OnSentPacket(callbacks_.TimeMillis(), payload);
}
++metrics_.tx_packets_count;
callbacks_.SendPacket(payload);
callbacks_.SendPacketWithStatus(payload);
}
bool DcSctpSocket::ValidateHasTCB() {

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

@ -42,7 +42,7 @@ class MockContext : public Context {
ON_CALL(*this, callbacks).WillByDefault(testing::ReturnRef(callbacks_));
ON_CALL(*this, current_rto).WillByDefault(testing::Return(DurationMs(123)));
ON_CALL(*this, Send).WillByDefault([this](SctpPacket::Builder& builder) {
callbacks_.SendPacket(builder.Build());
callbacks_.SendPacketWithStatus(builder.Build());
});
}

8
net/dcsctp/socket/mock_dcsctp_socket_callbacks.h
View File

@ -56,10 +56,11 @@ class MockDcSctpSocketCallbacks : public DcSctpSocketCallbacks {
: log_prefix_(name.empty() ? "" : std::string(name) + ": "),
random_(internal::GetUniqueSeed()),
timeout_manager_([this]() { return now_; }) {
ON_CALL(*this, SendPacket)
ON_CALL(*this, SendPacketWithStatus)
.WillByDefault([this](rtc::ArrayView<const uint8_t> data) {
sent_packets_.emplace_back(
std::vector<uint8_t>(data.begin(), data.end()));
return SendPacketStatus::kSuccess;
});
ON_CALL(*this, OnMessageReceived)
.WillByDefault([this](DcSctpMessage message) {
@ -80,8 +81,9 @@ class MockDcSctpSocketCallbacks : public DcSctpSocketCallbacks {
});
ON_CALL(*this, TimeMillis).WillByDefault([this]() { return now_; });
}
MOCK_METHOD(void,
SendPacket,
MOCK_METHOD(SendPacketStatus,
SendPacketWithStatus,
(rtc::ArrayView<const uint8_t> data),
(override));

14
net/dcsctp/socket/stream_reset_handler_test.cc
View File

@ -183,7 +183,7 @@ class StreamResetHandlerTest : public testing::Test {
};
TEST_F(StreamResetHandlerTest, ChunkWithNoParametersReturnsError) {
EXPECT_CALL(callbacks_, SendPacket).Times(0);
EXPECT_CALL(callbacks_, SendPacketWithStatus).Times(0);
EXPECT_CALL(callbacks_, OnError).Times(1);
handler_.HandleReConfig(ReConfigChunk(Parameters()));
}
@ -198,7 +198,7 @@ TEST_F(StreamResetHandlerTest, ChunkWithInvalidParametersReturnsError) {
ReconfigRequestSN(10),
kPeerInitialTsn, {StreamID(2)}));
EXPECT_CALL(callbacks_, SendPacket).Times(0);
EXPECT_CALL(callbacks_, SendPacketWithStatus).Times(0);
EXPECT_CALL(callbacks_, OnError).Times(1);
handler_.HandleReConfig(ReConfigChunk(builder.Build()));
}
@ -375,7 +375,7 @@ TEST_F(StreamResetHandlerTest, SendOutgoingResettingOnPositiveResponse) {
// Processing a response shouldn't result in sending anything.
EXPECT_CALL(callbacks_, OnError).Times(0);
EXPECT_CALL(callbacks_, SendPacket).Times(0);
EXPECT_CALL(callbacks_, SendPacketWithStatus).Times(0);
handler_.HandleReConfig(std::move(response_reconfig));
}
@ -401,7 +401,7 @@ TEST_F(StreamResetHandlerTest, SendOutgoingResetRollbackOnError) {
// Only requests should result in sending responses.
EXPECT_CALL(callbacks_, OnError).Times(0);
EXPECT_CALL(callbacks_, SendPacket).Times(0);
EXPECT_CALL(callbacks_, SendPacketWithStatus).Times(0);
handler_.HandleReConfig(std::move(response_reconfig));
}
@ -430,12 +430,12 @@ TEST_F(StreamResetHandlerTest, SendOutgoingResetRetransmitOnInProgress) {
// Processing a response shouldn't result in sending anything.
EXPECT_CALL(callbacks_, OnError).Times(0);
EXPECT_CALL(callbacks_, SendPacket).Times(0);
EXPECT_CALL(callbacks_, SendPacketWithStatus).Times(0);
handler_.HandleReConfig(std::move(response_reconfig));
// Let some time pass, so that the reconfig timer expires, and retries the
// same request.
EXPECT_CALL(callbacks_, SendPacket).Times(1);
EXPECT_CALL(callbacks_, SendPacketWithStatus).Times(1);
AdvanceTime(kRto);
std::vector<uint8_t> payload = callbacks_.ConsumeSentPacket();
@ -486,7 +486,7 @@ TEST_F(StreamResetHandlerTest, ResetWhileRequestIsSentWillQueue) {
// Processing a response shouldn't result in sending anything.
EXPECT_CALL(callbacks_, OnError).Times(0);
EXPECT_CALL(callbacks_, SendPacket).Times(0);
EXPECT_CALL(callbacks_, SendPacketWithStatus).Times(0);
handler_.HandleReConfig(std::move(response_reconfig));
// Response has been processed. A new request can be sent.