diff --git a/p2p/base/dtlstransport.cc b/p2p/base/dtlstransport.cc index c1b65eec73..0520a8517b 100644 --- a/p2p/base/dtlstransport.cc +++ b/p2p/base/dtlstransport.cc @@ -124,6 +124,7 @@ DtlsTransport::DtlsTransport(IceTransportInternal* ice_transport, ssl_role_(rtc::SSL_CLIENT), ssl_max_version_(rtc::SSL_PROTOCOL_DTLS_12), crypto_options_(crypto_options) { + RTC_DCHECK(ice_transport_); ice_transport_->SignalWritableState.connect(this, &DtlsTransport::OnWritableState); ice_transport_->SignalReadPacket.connect(this, &DtlsTransport::OnReadPacket); @@ -132,6 +133,8 @@ DtlsTransport::DtlsTransport(IceTransportInternal* ice_transport, &DtlsTransport::OnReadyToSend); ice_transport_->SignalReceivingState.connect( this, &DtlsTransport::OnReceivingState); + ice_transport_->SignalNetworkRouteChanged.connect( + this, &DtlsTransport::OnNetworkRouteChanged); } DtlsTransport::~DtlsTransport() = default; @@ -428,6 +431,10 @@ int DtlsTransport::GetError() { return ice_transport_->GetError(); } +rtc::Optional DtlsTransport::network_route() const { + return ice_transport_->network_route(); +} + bool DtlsTransport::GetOption(rtc::Socket::Option opt, int* value) { return ice_transport_->GetOption(opt, value); } @@ -631,6 +638,11 @@ void DtlsTransport::OnDtlsEvent(rtc::StreamInterface* dtls, int sig, int err) { } } +void DtlsTransport::OnNetworkRouteChanged( + rtc::Optional network_route) { + SignalNetworkRouteChanged(network_route); +} + void DtlsTransport::MaybeStartDtls() { if (dtls_ && ice_transport_->writable()) { ConfigureHandshakeTimeout(); diff --git a/p2p/base/dtlstransport.h b/p2p/base/dtlstransport.h index eb86deb337..4dd9bc257d 100644 --- a/p2p/base/dtlstransport.h +++ b/p2p/base/dtlstransport.h @@ -156,6 +156,8 @@ class DtlsTransport : public DtlsTransportInternal { int GetError() override; + rtc::Optional network_route() const override; + int SetOption(rtc::Socket::Option opt, int value) override; std::string ToString() const { @@ -179,6 +181,7 @@ class DtlsTransport : public DtlsTransportInternal { void OnReadyToSend(rtc::PacketTransportInternal* transport); void OnReceivingState(rtc::PacketTransportInternal* transport); void OnDtlsEvent(rtc::StreamInterface* stream_, int sig, int err); + void OnNetworkRouteChanged(rtc::Optional network_route); bool SetupDtls(); void MaybeStartDtls(); bool HandleDtlsPacket(const char* data, size_t size); diff --git a/p2p/base/dtlstransport_unittest.cc b/p2p/base/dtlstransport_unittest.cc index eaa0cecd3a..ed010e02c4 100644 --- a/p2p/base/dtlstransport_unittest.cc +++ b/p2p/base/dtlstransport_unittest.cc @@ -101,11 +101,11 @@ class DtlsTestClient : public sigslot::has_slots<> { (role == cricket::ICEROLE_CONTROLLING) ? 1 : 2); dtls->SetSslMaxProtocolVersion(ssl_max_version_); dtls->SignalWritableState.connect( - this, &DtlsTestClient::OnTransportChannelWritableState); - dtls->SignalReadPacket.connect( - this, &DtlsTestClient::OnTransportChannelReadPacket); - dtls->SignalSentPacket.connect( - this, &DtlsTestClient::OnTransportChannelSentPacket); + this, &DtlsTestClient::OnTransportWritableState); + dtls->SignalReadPacket.connect(this, + &DtlsTestClient::OnTransportReadPacket); + dtls->SignalSentPacket.connect(this, + &DtlsTestClient::OnTransportSentPacket); dtls_transports_.push_back(std::unique_ptr(dtls)); fake_ice_transports_.push_back( std::unique_ptr(fake_ice_channel)); @@ -356,18 +356,17 @@ class DtlsTestClient : public sigslot::has_slots<> { return (num_matches < ((static_cast(size) - 5) / 10)); } - // Transport channel callbacks - void OnTransportChannelWritableState( - rtc::PacketTransportInternal* transport) { - RTC_LOG(LS_INFO) << name_ << ": Channel '" << transport->debug_name() + // Transport callbacks + void OnTransportWritableState(rtc::PacketTransportInternal* transport) { + RTC_LOG(LS_INFO) << name_ << ": Transport '" << transport->transport_name() << "' is writable"; } - void OnTransportChannelReadPacket(rtc::PacketTransportInternal* transport, - const char* data, - size_t size, - const rtc::PacketTime& packet_time, - int flags) { + void OnTransportReadPacket(rtc::PacketTransportInternal* transport, + const char* data, + size_t size, + const rtc::PacketTime& packet_time, + int flags) { uint32_t packet_num = 0; ASSERT_TRUE(VerifyPacket(data, size, &packet_num)); received_.insert(packet_num); @@ -377,8 +376,8 @@ class DtlsTestClient : public sigslot::has_slots<> { ASSERT_EQ(expected_flags, flags); } - void OnTransportChannelSentPacket(rtc::PacketTransportInternal* transport, - const rtc::SentPacket& sent_packet) { + void OnTransportSentPacket(rtc::PacketTransportInternal* transport, + const rtc::SentPacket& sent_packet) { sent_packet_ = sent_packet; } diff --git a/p2p/base/dtlstransportinternal.cc b/p2p/base/dtlstransportinternal.cc index 5e28cb0022..a294ccbc4e 100644 --- a/p2p/base/dtlstransportinternal.cc +++ b/p2p/base/dtlstransportinternal.cc @@ -16,8 +16,4 @@ DtlsTransportInternal::DtlsTransportInternal() = default; DtlsTransportInternal::~DtlsTransportInternal() = default; -std::string DtlsTransportInternal::debug_name() const { - return transport_name() + " " + rtc::ToString(component()); -} - } // namespace cricket diff --git a/p2p/base/dtlstransportinternal.h b/p2p/base/dtlstransportinternal.h index be3db6b663..b08868fde5 100644 --- a/p2p/base/dtlstransportinternal.h +++ b/p2p/base/dtlstransportinternal.h @@ -43,8 +43,6 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal { virtual DtlsTransportState dtls_state() const = 0; - virtual const std::string& transport_name() const = 0; - virtual int component() const = 0; virtual bool IsDtlsActive() const = 0; @@ -93,9 +91,6 @@ class DtlsTransportInternal : public rtc::PacketTransportInternal { // Emitted whenever the Dtls handshake failed on some transport channel. sigslot::signal1 SignalDtlsHandshakeError; - // Debugging description of this transport. - std::string debug_name() const override; - protected: DtlsTransportInternal(); diff --git a/p2p/base/fakedtlstransport.h b/p2p/base/fakedtlstransport.h index 01a4ed8dfb..508158f678 100644 --- a/p2p/base/fakedtlstransport.h +++ b/p2p/base/fakedtlstransport.h @@ -31,8 +31,11 @@ class FakeDtlsTransport : public DtlsTransportInternal { transport_name_(ice_transport->transport_name()), component_(ice_transport->component()), dtls_fingerprint_("", nullptr, 0) { + RTC_DCHECK(ice_transport_); ice_transport_->SignalReadPacket.connect( this, &FakeDtlsTransport::OnIceTransportReadPacket); + ice_transport_->SignalNetworkRouteChanged.connect( + this, &FakeDtlsTransport::OnNetworkRouteChanged); } // If this constructor is called, a new fake ICE transport will be created, @@ -45,6 +48,8 @@ class FakeDtlsTransport : public DtlsTransportInternal { ice_transport_ = owned_ice_transport_.get(); ice_transport_->SignalReadPacket.connect( this, &FakeDtlsTransport::OnIceTransportReadPacket); + ice_transport_->SignalNetworkRouteChanged.connect( + this, &FakeDtlsTransport::OnNetworkRouteChanged); } ~FakeDtlsTransport() override { @@ -201,6 +206,10 @@ class FakeDtlsTransport : public DtlsTransportInternal { } int GetError() override { return ice_transport_->GetError(); } + rtc::Optional network_route() const override { + return ice_transport_->network_route(); + } + private: void OnIceTransportReadPacket(PacketTransportInternal* ice_, const char* data, @@ -229,6 +238,10 @@ class FakeDtlsTransport : public DtlsTransportInternal { SignalWritableState(this); } + void OnNetworkRouteChanged(rtc::Optional network_route) { + SignalNetworkRouteChanged(network_route); + } + FakeIceTransport* ice_transport_; std::unique_ptr owned_ice_transport_; std::string transport_name_; diff --git a/p2p/base/fakeicetransport.h b/p2p/base/fakeicetransport.h index 7b6f78a33e..cc6cec99d8 100644 --- a/p2p/base/fakeicetransport.h +++ b/p2p/base/fakeicetransport.h @@ -190,6 +190,7 @@ class FakeIceTransport : public IceTransportInternal { SignalSentPacket(this, sent_packet); return static_cast(len); } + int SetOption(rtc::Socket::Option opt, int value) override { socket_options_[opt] = value; return true; @@ -203,8 +204,16 @@ class FakeIceTransport : public IceTransportInternal { return false; } } + int GetError() override { return 0; } + rtc::Optional network_route() const override { + return network_route_; + } + void SetNetworkRoute(rtc::Optional network_route) { + network_route_ = network_route; + } + private: void set_writable(bool writable) { if (writable_ == writable) { @@ -255,6 +264,7 @@ class FakeIceTransport : public IceTransportInternal { bool receiving_ = false; bool combine_outgoing_packets_ = false; rtc::CopyOnWriteBuffer send_packet_; + rtc::Optional network_route_; std::map socket_options_; }; diff --git a/p2p/base/fakepackettransport.h b/p2p/base/fakepackettransport.h index 2af14c1bda..cbff5110ed 100644 --- a/p2p/base/fakepackettransport.h +++ b/p2p/base/fakepackettransport.h @@ -23,8 +23,8 @@ namespace rtc { // Used to simulate a packet-based transport. class FakePacketTransport : public PacketTransportInternal { public: - explicit FakePacketTransport(const std::string& debug_name) - : debug_name_(debug_name) {} + explicit FakePacketTransport(const std::string& transport_name) + : transport_name_(transport_name) {} ~FakePacketTransport() override { if (dest_ && dest_->dest_ == this) { dest_->dest_ = nullptr; @@ -59,7 +59,7 @@ class FakePacketTransport : public PacketTransportInternal { } // Fake PacketTransportInternal implementation. - std::string debug_name() const override { return debug_name_; } + const std::string& transport_name() const override { return transport_name_; } bool writable() const override { return writable_; } bool receiving() const override { return receiving_; } int SendPacket(const char* data, @@ -88,6 +88,13 @@ class FakePacketTransport : public PacketTransportInternal { const CopyOnWriteBuffer* last_sent_packet() { return &last_sent_packet_; } + Optional network_route() const override { + return network_route_; + } + void SetNetworkRoute(Optional network_route) { + network_route_ = network_route; + } + private: void set_writable(bool writable) { if (writable_ == writable) { @@ -118,12 +125,14 @@ class FakePacketTransport : public PacketTransportInternal { CopyOnWriteBuffer last_sent_packet_; AsyncInvoker invoker_; - std::string debug_name_; + std::string transport_name_; FakePacketTransport* dest_ = nullptr; bool async_ = false; int async_delay_ms_ = 0; bool writable_ = false; bool receiving_ = false; + + Optional network_route_; }; } // namespace rtc diff --git a/p2p/base/icetransportinternal.cc b/p2p/base/icetransportinternal.cc index a741205c21..d5fa989613 100644 --- a/p2p/base/icetransportinternal.cc +++ b/p2p/base/icetransportinternal.cc @@ -26,8 +26,4 @@ void IceTransportInternal::SetRemoteIceCredentials(const std::string& ice_ufrag, SetRemoteIceParameters(IceParameters(ice_ufrag, ice_pwd, false)); } -std::string IceTransportInternal::debug_name() const { - return transport_name() + " " + rtc::ToString(component()); -} - } // namespace cricket diff --git a/p2p/base/icetransportinternal.h b/p2p/base/icetransportinternal.h index b7dfc42fed..0a71162eea 100644 --- a/p2p/base/icetransportinternal.h +++ b/p2p/base/icetransportinternal.h @@ -52,8 +52,6 @@ class IceTransportInternal : public rtc::PacketTransportInternal { virtual IceTransportState GetState() const = 0; - virtual const std::string& transport_name() const = 0; - virtual int component() const = 0; virtual IceRole GetIceRole() const = 0; @@ -111,21 +109,14 @@ class IceTransportInternal : public rtc::PacketTransportInternal { sigslot::signal2 SignalCandidatesRemoved; - // Deprecated by SignalSelectedCandidatePairChanged + // Deprecated by PacketTransportInternal::SignalNetworkRouteChanged. // This signal occurs when there is a change in the way that packets are // being routed, i.e. to a different remote location. The candidate // indicates where and how we are currently sending media. + // TODO(zhihuang): Update the Chrome remoting to use the new + // SignalNetworkRouteChanged. sigslot::signal2 SignalRouteChange; - // Signalled when the current selected candidate pair has changed. - // The first parameter is the transport that signals the event. - // The second parameter is the new selected candidate pair. The third - // parameter is the last packet id sent on the previous candidate pair. - // The fourth parameter is a boolean which is true if the Transport - // is ready to send with this candidate pair. - sigslot::signal4 - SignalSelectedCandidatePairChanged; - // Invoked when there is conflict in the ICE role between local and remote // agents. sigslot::signal1 SignalRoleConflict; @@ -135,9 +126,6 @@ class IceTransportInternal : public rtc::PacketTransportInternal { // Invoked when the transport is being destroyed. sigslot::signal1 SignalDestroyed; - - // Debugging description of this transport. - std::string debug_name() const override; }; } // namespace cricket diff --git a/p2p/base/jseptransport_unittest.cc b/p2p/base/jseptransport_unittest.cc index 644bcd90b0..8d09e0f9bd 100644 --- a/p2p/base/jseptransport_unittest.cc +++ b/p2p/base/jseptransport_unittest.cc @@ -17,7 +17,6 @@ #include "rtc_base/network.h" using cricket::JsepTransport; -using cricket::TransportChannel; using cricket::FakeDtlsTransport; using cricket::FakeIceTransport; using cricket::IceRole; diff --git a/p2p/base/p2ptransportchannel.cc b/p2p/base/p2ptransportchannel.cc index 71af75c7d2..19abf38dcb 100644 --- a/p2p/base/p2ptransportchannel.cc +++ b/p2p/base/p2ptransportchannel.cc @@ -23,6 +23,7 @@ #include "rtc_base/checks.h" #include "rtc_base/crc32.h" #include "rtc_base/logging.h" +#include "rtc_base/nethelper.h" #include "rtc_base/stringencode.h" #include "rtc_base/timeutils.h" #include "system_wrappers/include/field_trial.h" @@ -1082,6 +1083,10 @@ bool P2PTransportChannel::GetStats(ConnectionInfos *infos) { return true; } +rtc::Optional P2PTransportChannel::network_route() const { + return network_route_; +} + rtc::DiffServCodePoint P2PTransportChannel::DefaultDscpValue() const { OptionMap::const_iterator it = options_.find(rtc::Socket::OPT_DSCP); if (it == options_.end()) { @@ -1467,6 +1472,7 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn) { // destroyed, so don't use it. Connection* old_selected_connection = selected_connection_; selected_connection_ = conn; + network_route_.reset(); if (selected_connection_) { ++nomination_; if (old_selected_connection) { @@ -1485,12 +1491,23 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn) { PresumedWritable(selected_connection_)) { SignalReadyToSend(this); } + + network_route_.emplace(rtc::NetworkRoute()); + network_route_->connected = ReadyToSend(selected_connection_); + network_route_->local_network_id = + selected_connection_->local_candidate().network_id(); + network_route_->remote_network_id = + selected_connection_->remote_candidate().network_id(); + network_route_->last_sent_packet_id = last_sent_packet_id_; + network_route_->packet_overhead = + GetIpOverhead( + selected_connection_->local_candidate().address().family()) + + GetProtocolOverhead(selected_connection_->local_candidate().protocol()); } else { LOG_J(LS_INFO, this) << "No selected connection"; } - SignalSelectedCandidatePairChanged(this, selected_connection_, - last_sent_packet_id_, - ReadyToSend(selected_connection_)); + + SignalNetworkRouteChanged(network_route_); } // Warning: UpdateState should eventually be called whenever a connection diff --git a/p2p/base/p2ptransportchannel.h b/p2p/base/p2ptransportchannel.h index f9e9a53761..188f60c4f1 100644 --- a/p2p/base/p2ptransportchannel.h +++ b/p2p/base/p2ptransportchannel.h @@ -127,6 +127,8 @@ class P2PTransportChannel : public IceTransportInternal, int receiving_timeout() const { return config_.receiving_timeout; } int check_receiving_interval() const { return check_receiving_interval_; } + rtc::Optional network_route() const override; + // Helper method used only in unittest. rtc::DiffServCodePoint DefaultDscpValue() const; @@ -400,6 +402,8 @@ class P2PTransportChannel : public IceTransportInternal, webrtc::MetricsObserverInterface* metrics_observer_ = nullptr; + rtc::Optional network_route_; + RTC_DISALLOW_COPY_AND_ASSIGN(P2PTransportChannel); }; diff --git a/p2p/base/p2ptransportchannel_unittest.cc b/p2p/base/p2ptransportchannel_unittest.cc index 326cafa0bc..37f68073e1 100644 --- a/p2p/base/p2ptransportchannel_unittest.cc +++ b/p2p/base/p2ptransportchannel_unittest.cc @@ -379,8 +379,8 @@ class P2PTransportChannelTestBase : public testing::Test, this, &P2PTransportChannelTestBase::OnReadPacket); channel->SignalRoleConflict.connect( this, &P2PTransportChannelTestBase::OnRoleConflict); - channel->SignalSelectedCandidatePairChanged.connect( - this, &P2PTransportChannelTestBase::OnSelectedCandidatePairChanged); + channel->SignalNetworkRouteChanged.connect( + this, &P2PTransportChannelTestBase::OnNetworkRouteChanged); channel->SetIceParameters(local_ice); if (remote_ice_parameter_source_ == FROM_SETICEPARAMETERS) { channel->SetRemoteIceParameters(remote_ice); @@ -697,14 +697,12 @@ class P2PTransportChannelTestBase : public testing::Test, new CandidatesData(ch, c)); } } - void OnSelectedCandidatePairChanged( - IceTransportInternal* transport_channel, - CandidatePairInterface* selected_candidate_pair, - int last_sent_packet_id, - bool ready_to_send) { - // Do not count if it switches to nullptr. This may happen if all - // connections timed out. - if (selected_candidate_pair != nullptr) { + + void OnNetworkRouteChanged(rtc::Optional network_route) { + // If the |network_route| is unset, don't count. This is used in the case + // when the network on remote side is down, the signal will be fired with an + // unset network route and it shouldn't trigger a connection switch. + if (network_route) { ++selected_candidate_pair_switches_; } } @@ -3055,8 +3053,8 @@ class P2PTransportChannelPingTest : public testing::Test, ch->SetIceRole(ICEROLE_CONTROLLING); ch->SetIceParameters(kIceParams[0]); ch->SetRemoteIceParameters(kIceParams[1]); - ch->SignalSelectedCandidatePairChanged.connect( - this, &P2PTransportChannelPingTest::OnSelectedCandidatePairChanged); + ch->SignalNetworkRouteChanged.connect( + this, &P2PTransportChannelPingTest::OnNetworkRouteChanged); ch->SignalReadyToSend.connect(this, &P2PTransportChannelPingTest::OnReadyToSend); ch->SignalStateChanged.connect( @@ -3142,13 +3140,11 @@ class P2PTransportChannelPingTest : public testing::Test, conn->SignalNominated(conn); } - void OnSelectedCandidatePairChanged( - IceTransportInternal* transport_channel, - CandidatePairInterface* selected_candidate_pair, - int last_sent_packet_id, - bool ready_to_send) { - last_selected_candidate_pair_ = selected_candidate_pair; - last_sent_packet_id_ = last_sent_packet_id; + void OnNetworkRouteChanged(rtc::Optional network_route) { + last_network_route_ = network_route; + if (last_network_route_) { + last_sent_packet_id_ = last_network_route_->last_sent_packet_id; + } ++selected_candidate_pair_switches_; } @@ -3182,9 +3178,6 @@ class P2PTransportChannelPingTest : public testing::Test, channel_state_ = channel->GetState(); } - CandidatePairInterface* last_selected_candidate_pair() { - return last_selected_candidate_pair_; - } int last_sent_packet_id() { return last_sent_packet_id_; } bool channel_ready_to_send() { return channel_ready_to_send_; } void reset_channel_ready_to_send() { channel_ready_to_send_ = false; } @@ -3195,14 +3188,26 @@ class P2PTransportChannelPingTest : public testing::Test, return switches; } + // Return true if the |pair| matches the last network route. + bool CandidatePairMatchesNetworkRoute(CandidatePairInterface* pair) { + if (!pair) { + return !last_network_route_.has_value(); + } else { + return pair->local_candidate().network_id() == + last_network_route_->local_network_id && + pair->remote_candidate().network_id() == + last_network_route_->remote_network_id; + } + } + private: std::unique_ptr vss_; rtc::AutoSocketServerThread thread_; - CandidatePairInterface* last_selected_candidate_pair_ = nullptr; int selected_candidate_pair_switches_ = 0; int last_sent_packet_id_ = -1; bool channel_ready_to_send_ = false; IceTransportState channel_state_ = IceTransportState::STATE_INIT; + rtc::Optional last_network_route_; }; TEST_F(P2PTransportChannelPingTest, TestTriggeredChecks) { @@ -3580,7 +3585,7 @@ TEST_F(P2PTransportChannelPingTest, TestReceivingStateChange) { // The controlled side will select a connection as the "selected connection" // based on priority until the controlling side nominates a connection, at which // point the controlled side will select that connection as the -// "selected connection". Plus, SignalSelectedCandidatePair will be fired if the +// "selected connection". Plus, SignalNetworkRouteChanged will be fired if the // selected connection changes and SignalReadyToSend will be fired if the new // selected connection is writable. TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) { @@ -3603,7 +3608,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) { // A connection needs to be writable before it is selected for transmission. conn1->ReceivedPingResponse(LOW_RTT, "id"); EXPECT_EQ_WAIT(conn1, ch.selected_connection(), kDefaultTimeout); - EXPECT_EQ(conn1, last_selected_candidate_pair()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1)); EXPECT_EQ(len, SendData(ch, data, len, ++last_packet_id)); // When a higher priority candidate comes in, the new connection is chosen @@ -3613,7 +3618,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) { ASSERT_TRUE(conn2 != nullptr); conn2->ReceivedPingResponse(LOW_RTT, "id"); EXPECT_EQ_WAIT(conn2, ch.selected_connection(), kDefaultTimeout); - EXPECT_EQ(conn2, last_selected_candidate_pair()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2)); EXPECT_TRUE(channel_ready_to_send()); EXPECT_EQ(last_packet_id, last_sent_packet_id()); @@ -3631,7 +3636,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) { // connection. NominateConnection(conn3); EXPECT_EQ(conn3, ch.selected_connection()); - EXPECT_EQ(conn3, last_selected_candidate_pair()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn3)); EXPECT_EQ(last_packet_id, last_sent_packet_id()); EXPECT_TRUE(channel_ready_to_send()); @@ -3652,7 +3657,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBeforeNomination) { // The selected connection switches after conn4 becomes writable. conn4->ReceivedPingResponse(LOW_RTT, "id"); EXPECT_EQ_WAIT(conn4, ch.selected_connection(), kDefaultTimeout); - EXPECT_EQ(conn4, last_selected_candidate_pair()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn4)); EXPECT_EQ(last_packet_id, last_sent_packet_id()); // SignalReadyToSend is fired again because conn4 is writable. EXPECT_TRUE(channel_ready_to_send()); @@ -3814,7 +3819,7 @@ TEST_F(P2PTransportChannelPingTest, // Initially, connections are selected based on priority. EXPECT_EQ(1, reset_selected_candidate_pair_switches()); - EXPECT_EQ(conn1, last_selected_candidate_pair()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1)); // conn2 receives data; it becomes selected. // Advance the clock by 1ms so that the last data receiving timestamp of @@ -3822,12 +3827,12 @@ TEST_F(P2PTransportChannelPingTest, SIMULATED_WAIT(false, 1, clock); conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0)); EXPECT_EQ(1, reset_selected_candidate_pair_switches()); - EXPECT_EQ(conn2, last_selected_candidate_pair()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2)); // conn1 also receives data; it becomes selected due to priority again. conn1->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0)); EXPECT_EQ(1, reset_selected_candidate_pair_switches()); - EXPECT_EQ(conn1, last_selected_candidate_pair()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2)); // conn2 received data more recently; it is selected now because it // received data more recently. @@ -3836,7 +3841,7 @@ TEST_F(P2PTransportChannelPingTest, conn2->ReceivedPingResponse(LOW_RTT, "id"); conn2->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0)); EXPECT_EQ(1, reset_selected_candidate_pair_switches()); - EXPECT_EQ(conn2, last_selected_candidate_pair()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2)); // Make sure sorting won't reselect candidate pair. SIMULATED_WAIT(false, 10, clock); @@ -3866,17 +3871,17 @@ TEST_F(P2PTransportChannelPingTest, SIMULATED_WAIT(false, 1, clock); conn1->OnReadPacket("XYZ", 3, rtc::CreatePacketTime(0)); EXPECT_EQ(1, reset_selected_candidate_pair_switches()); - EXPECT_EQ(conn1, last_selected_candidate_pair()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1)); // conn2 is nominated; it becomes the selected connection. NominateConnection(conn2); EXPECT_EQ(1, reset_selected_candidate_pair_switches()); - EXPECT_EQ(conn2, last_selected_candidate_pair()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2)); // conn1 is selected because it has higher priority and also nominated. NominateConnection(conn1); EXPECT_EQ(1, reset_selected_candidate_pair_switches()); - EXPECT_EQ(conn1, last_selected_candidate_pair()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2)); // Make sure sorting won't reselect candidate pair. SIMULATED_WAIT(false, 10, clock); @@ -3902,24 +3907,28 @@ TEST_F(P2PTransportChannelPingTest, ASSERT_TRUE(conn2 != nullptr); // conn1 is the selected connection because it has a higher priority, - EXPECT_EQ_SIMULATED_WAIT(conn1, last_selected_candidate_pair(), - kDefaultTimeout, clock); + EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), kDefaultTimeout, + clock); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1)); reset_selected_candidate_pair_switches(); // conn2 is nominated; it becomes selected. NominateConnection(conn2); EXPECT_EQ(1, reset_selected_candidate_pair_switches()); - EXPECT_EQ(conn2, last_selected_candidate_pair()); + EXPECT_EQ(conn2, ch.selected_connection()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2)); // conn1 is selected because of its priority. NominateConnection(conn1); EXPECT_EQ(1, reset_selected_candidate_pair_switches()); - EXPECT_EQ(conn1, last_selected_candidate_pair()); + EXPECT_EQ(conn1, ch.selected_connection()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1)); // conn2 gets higher remote nomination; it is selected again. NominateConnection(conn2, 2U); EXPECT_EQ(1, reset_selected_candidate_pair_switches()); - EXPECT_EQ(conn2, last_selected_candidate_pair()); + EXPECT_EQ(conn2, ch.selected_connection()); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2)); // Make sure sorting won't reselect candidate pair. SIMULATED_WAIT(false, 100, clock); @@ -3970,15 +3979,17 @@ TEST_F(P2PTransportChannelPingTest, conn2->ReceivedPingResponse(LOW_RTT, "id"); EXPECT_EQ_SIMULATED_WAIT(1, reset_selected_candidate_pair_switches(), kDefaultTimeout, clock); - EXPECT_EQ_SIMULATED_WAIT(conn2, last_selected_candidate_pair(), - kDefaultTimeout, clock); + EXPECT_EQ_SIMULATED_WAIT(conn2, ch.selected_connection(), kDefaultTimeout, + clock); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn2)); // If conn1 is also writable, it will become selected. conn1->ReceivedPingResponse(LOW_RTT, "id"); EXPECT_EQ_SIMULATED_WAIT(1, reset_selected_candidate_pair_switches(), kDefaultTimeout, clock); - EXPECT_EQ_SIMULATED_WAIT(conn1, last_selected_candidate_pair(), - kDefaultTimeout, clock); + EXPECT_EQ_SIMULATED_WAIT(conn1, ch.selected_connection(), kDefaultTimeout, + clock); + EXPECT_TRUE(CandidatePairMatchesNetworkRoute(conn1)); // Make sure sorting won't reselect candidate pair. SIMULATED_WAIT(false, 10, clock); diff --git a/p2p/base/packettransportinternal.cc b/p2p/base/packettransportinternal.cc index 0ecb2d1aed..2d28ffbd1e 100644 --- a/p2p/base/packettransportinternal.cc +++ b/p2p/base/packettransportinternal.cc @@ -24,4 +24,8 @@ bool PacketTransportInternal::GetOption(rtc::Socket::Option opt, int* value) { return false; } +rtc::Optional PacketTransportInternal::network_route() const { + return rtc::Optional(); +} + } // namespace rtc diff --git a/p2p/base/packettransportinternal.h b/p2p/base/packettransportinternal.h index 41e9fbcfb9..a5b9772c27 100644 --- a/p2p/base/packettransportinternal.h +++ b/p2p/base/packettransportinternal.h @@ -14,16 +14,15 @@ #include #include +#include "api/optional.h" // This is included for PacketOptions. #include "api/ortc/packettransportinterface.h" +#include "p2p/base/port.h" #include "rtc_base/asyncpacketsocket.h" +#include "rtc_base/networkroute.h" #include "rtc_base/sigslot.h" #include "rtc_base/socket.h" -namespace cricket { -class TransportChannel; -} - namespace rtc { struct PacketOptions; struct PacketTime; @@ -32,8 +31,7 @@ struct SentPacket; class PacketTransportInternal : public virtual webrtc::PacketTransportInterface, public sigslot::has_slots<> { public: - // Identify the object for logging and debug purpose. - virtual std::string debug_name() const = 0; + virtual const std::string& transport_name() const = 0; // The transport has been established. virtual bool writable() const = 0; @@ -66,6 +64,10 @@ class PacketTransportInternal : public virtual webrtc::PacketTransportInterface, // Returns the most recent error that occurred on this channel. virtual int GetError() = 0; + // Returns the current network route with transport overhead. + // TODO(zhihuang): Make it pure virtual once the Chrome/remoting is updated. + virtual rtc::Optional network_route() const; + // Emitted when the writable state, represented by |writable()|, changes. sigslot::signal1 SignalWritableState; @@ -91,6 +93,9 @@ class PacketTransportInternal : public virtual webrtc::PacketTransportInterface, sigslot::signal2 SignalSentPacket; + // Signalled when the current network route has changed. + sigslot::signal1> SignalNetworkRouteChanged; + protected: PacketTransportInternal(); ~PacketTransportInternal() override; diff --git a/p2p/base/port.cc b/p2p/base/port.cc index 8ffc68866b..1c316d2cb2 100644 --- a/p2p/base/port.cc +++ b/p2p/base/port.cc @@ -99,11 +99,6 @@ const char STUN_PORT_TYPE[] = "stun"; const char PRFLX_PORT_TYPE[] = "prflx"; const char RELAY_PORT_TYPE[] = "relay"; -const char UDP_PROTOCOL_NAME[] = "udp"; -const char TCP_PROTOCOL_NAME[] = "tcp"; -const char SSLTCP_PROTOCOL_NAME[] = "ssltcp"; -const char TLS_PROTOCOL_NAME[] = "tls"; - static const char* const PROTO_NAMES[] = {UDP_PROTOCOL_NAME, TCP_PROTOCOL_NAME, SSLTCP_PROTOCOL_NAME, TLS_PROTOCOL_NAME}; diff --git a/p2p/base/port.h b/p2p/base/port.h index 3bcfae26be..2957fd8b74 100644 --- a/p2p/base/port.h +++ b/p2p/base/port.h @@ -28,6 +28,7 @@ #include "p2p/base/stunrequest.h" #include "rtc_base/asyncpacketsocket.h" #include "rtc_base/checks.h" +#include "rtc_base/nethelper.h" #include "rtc_base/network.h" #include "rtc_base/proxyinfo.h" #include "rtc_base/ratetracker.h" @@ -45,11 +46,6 @@ extern const char STUN_PORT_TYPE[]; extern const char PRFLX_PORT_TYPE[]; extern const char RELAY_PORT_TYPE[]; -extern const char UDP_PROTOCOL_NAME[]; -extern const char TCP_PROTOCOL_NAME[]; -extern const char SSLTCP_PROTOCOL_NAME[]; -extern const char TLS_PROTOCOL_NAME[]; - // RFC 6544, TCP candidate encoding rules. extern const int DISCARD_PORT; extern const char TCPTYPE_ACTIVE_STR[]; diff --git a/p2p/base/udptransport.cc b/p2p/base/udptransport.cc index 9f3d7d0990..8b4ce00689 100644 --- a/p2p/base/udptransport.cc +++ b/p2p/base/udptransport.cc @@ -16,6 +16,7 @@ #include "rtc_base/asyncpacketsocket.h" #include "rtc_base/asyncudpsocket.h" #include "rtc_base/logging.h" +#include "rtc_base/nethelper.h" #include "rtc_base/socketaddress.h" #include "rtc_base/thread.h" #include "rtc_base/thread_checker.h" @@ -63,7 +64,7 @@ rtc::SocketAddress UdpTransport::GetRemoteAddress() const { return remote_address_; } -std::string UdpTransport::debug_name() const { +const std::string& UdpTransport::transport_name() const { return transport_name_; } @@ -95,6 +96,13 @@ int UdpTransport::SendPacket(const char* data, return result; } +rtc::Optional UdpTransport::network_route() const { + rtc::NetworkRoute network_route; + network_route.packet_overhead = + /*kUdpOverhead=*/8 + GetIpOverhead(GetLocalAddress().family()); + return rtc::Optional(network_route); +} + int UdpTransport::SetOption(rtc::Socket::Option opt, int value) { return 0; } diff --git a/p2p/base/udptransport.h b/p2p/base/udptransport.h index 6a9d211ab9..118b596e91 100644 --- a/p2p/base/udptransport.h +++ b/p2p/base/udptransport.h @@ -46,7 +46,7 @@ class UdpTransport : public rtc::PacketTransportInternal, rtc::SocketAddress GetRemoteAddress() const override; // Overrides of PacketTransportInternal, used by webrtc internally. - std::string debug_name() const override; + const std::string& transport_name() const override; bool receiving() const override; @@ -61,6 +61,8 @@ class UdpTransport : public rtc::PacketTransportInternal, int GetError() override; + rtc::Optional network_route() const override; + protected: PacketTransportInternal* GetInternal() override; @@ -73,6 +75,7 @@ class UdpTransport : public rtc::PacketTransportInternal, void OnSocketSentPacket(rtc::AsyncPacketSocket* socket, const rtc::SentPacket& packet); bool IsLocalConsistent(); + std::string transport_name_; int send_error_ = 0; std::unique_ptr socket_; diff --git a/pc/channel.cc b/pc/channel.cc index 3950c03a10..4318aaec3a 100644 --- a/pc/channel.cc +++ b/pc/channel.cc @@ -174,6 +174,8 @@ BaseChannel::BaseChannel(rtc::Thread* worker_thread, // channel to signal. rtp_transport_->SignalPacketReceived.connect(this, &BaseChannel::OnPacketReceived); + rtp_transport_->SignalNetworkRouteChanged.connect( + this, &BaseChannel::OnNetworkRouteChanged); RTC_LOG(LS_INFO) << "Created channel for " << content_name; } @@ -301,7 +303,7 @@ void BaseChannel::SetTransports_n( transport_name_ = rtp_dtls_transport->transport_name(); debug_name = transport_name_; } else { - debug_name = rtp_packet_transport->debug_name(); + debug_name = rtp_packet_transport->transport_name(); } if (rtp_packet_transport == rtp_transport_->rtp_packet_transport()) { // Nothing to do if transport isn't changing. @@ -344,6 +346,9 @@ void BaseChannel::SetTransport_n( DtlsTransportInternal* new_dtls_transport, rtc::PacketTransportInternal* new_packet_transport) { RTC_DCHECK(network_thread_->IsCurrent()); + if (new_dtls_transport) { + RTC_DCHECK(new_dtls_transport == new_packet_transport); + } DtlsTransportInternal*& old_dtls_transport = rtcp ? rtcp_dtls_transport_ : rtp_dtls_transport_; rtc::PacketTransportInternal* old_packet_transport = @@ -398,21 +403,14 @@ void BaseChannel::ConnectToDtlsTransport(DtlsTransportInternal* transport) { transport->SignalWritableState.connect(this, &BaseChannel::OnWritableState); transport->SignalDtlsState.connect(this, &BaseChannel::OnDtlsState); transport->SignalSentPacket.connect(this, &BaseChannel::SignalSentPacket_n); - transport->ice_transport()->SignalSelectedCandidatePairChanged.connect( - this, &BaseChannel::OnSelectedCandidatePairChanged); } void BaseChannel::DisconnectFromDtlsTransport( DtlsTransportInternal* transport) { RTC_DCHECK(network_thread_->IsCurrent()); - OnSelectedCandidatePairChanged(transport->ice_transport(), nullptr, -1, - false); - transport->SignalWritableState.disconnect(this); transport->SignalDtlsState.disconnect(this); transport->SignalSentPacket.disconnect(this); - transport->ice_transport()->SignalSelectedCandidatePairChanged.disconnect( - this); } void BaseChannel::ConnectToPacketTransport( @@ -592,29 +590,24 @@ void BaseChannel::OnDtlsState(DtlsTransportInternal* transport, } } -void BaseChannel::OnSelectedCandidatePairChanged( - IceTransportInternal* ice_transport, - CandidatePairInterface* selected_candidate_pair, - int last_sent_packet_id, - bool ready_to_send) { - RTC_DCHECK((rtp_dtls_transport_ && - ice_transport == rtp_dtls_transport_->ice_transport()) || - (rtcp_dtls_transport_ && - ice_transport == rtcp_dtls_transport_->ice_transport())); +void BaseChannel::OnNetworkRouteChanged( + rtc::Optional network_route) { RTC_DCHECK(network_thread_->IsCurrent()); - selected_candidate_pair_ = selected_candidate_pair; - std::string transport_name = ice_transport->transport_name(); - rtc::NetworkRoute network_route; - if (selected_candidate_pair) { - network_route = rtc::NetworkRoute( - ready_to_send, selected_candidate_pair->local_candidate().network_id(), - selected_candidate_pair->remote_candidate().network_id(), - last_sent_packet_id); - - UpdateTransportOverhead(); + rtc::NetworkRoute new_route; + if (network_route) { + invoker_.AsyncInvoke(RTC_FROM_HERE, worker_thread_, [=] { + media_channel_->OnTransportOverheadChanged( + network_route->packet_overhead); + }); + new_route = *(network_route); } + + // Note: When the RTCP-muxing is not enabled, RTCP transport and RTP transport + // use the same transport name and MediaChannel::OnNetworkRouteChanged cannot + // work correctly. Intentionally leave it broken to simplify the code and + // encourage the users to stop using non-muxing RTCP. invoker_.AsyncInvoke(RTC_FROM_HERE, worker_thread_, [=] { - media_channel_->OnNetworkRouteChanged(transport_name, network_route); + media_channel_->OnNetworkRouteChanged(transport_name_, new_route); }); } @@ -915,9 +908,8 @@ bool BaseChannel::SetupDtlsSrtp_n(bool rtcp) { if (!ret) { RTC_LOG(LS_WARNING) << "DTLS-SRTP key installation failed"; - } else { - UpdateTransportOverhead(); } + return ret; } @@ -1021,6 +1013,7 @@ void BaseChannel::EnableSrtpTransport_n() { if (srtp_transport_ == nullptr) { rtp_transport_->SignalReadyToSend.disconnect(this); rtp_transport_->SignalPacketReceived.disconnect(this); + rtp_transport_->SignalNetworkRouteChanged.disconnect(this); auto transport = rtc::MakeUnique( std::move(rtp_transport_), content_name_); @@ -1031,6 +1024,8 @@ void BaseChannel::EnableSrtpTransport_n() { this, &BaseChannel::OnTransportReadyToSend); rtp_transport_->SignalPacketReceived.connect( this, &BaseChannel::OnPacketReceived); + rtp_transport_->SignalNetworkRouteChanged.connect( + this, &BaseChannel::OnNetworkRouteChanged); RTC_LOG(LS_INFO) << "Wrapping RtpTransport in SrtpTransport."; } } @@ -1160,7 +1155,7 @@ bool BaseChannel::SetRtcpMux_n(bool enable, // the RTCP transport. std::string debug_name = transport_name_.empty() - ? rtp_transport_->rtp_packet_transport()->debug_name() + ? rtp_transport_->rtp_packet_transport()->transport_name() : transport_name_; RTC_LOG(LS_INFO) << "Enabling rtcp-mux for " << content_name() << "; no longer need RTCP transport for " @@ -1679,47 +1674,6 @@ void BaseChannel::UpdateMediaSendRecvState() { Bind(&BaseChannel::UpdateMediaSendRecvState_w, this)); } -int BaseChannel::GetTransportOverheadPerPacket() const { - RTC_DCHECK(network_thread_->IsCurrent()); - - if (!selected_candidate_pair_) - return 0; - - int transport_overhead_per_packet = 0; - - constexpr int kIpv4Overhaed = 20; - constexpr int kIpv6Overhaed = 40; - transport_overhead_per_packet += - selected_candidate_pair_->local_candidate().address().family() == AF_INET - ? kIpv4Overhaed - : kIpv6Overhaed; - - constexpr int kUdpOverhaed = 8; - constexpr int kTcpOverhaed = 20; - transport_overhead_per_packet += - selected_candidate_pair_->local_candidate().protocol() == - TCP_PROTOCOL_NAME - ? kTcpOverhaed - : kUdpOverhaed; - - if (srtp_active()) { - int srtp_overhead = 0; - if (srtp_transport_->GetSrtpOverhead(&srtp_overhead)) - transport_overhead_per_packet += srtp_overhead; - } - - return transport_overhead_per_packet; -} - -void BaseChannel::UpdateTransportOverhead() { - int transport_overhead_per_packet = GetTransportOverheadPerPacket(); - if (transport_overhead_per_packet) - invoker_.AsyncInvoke( - RTC_FROM_HERE, worker_thread_, - Bind(&MediaChannel::OnTransportOverheadChanged, media_channel_.get(), - transport_overhead_per_packet)); -} - void VoiceChannel::UpdateMediaSendRecvState_w() { // Render incoming data if we're the active call, and we have the local // content. We receive data on the default channel and multiplexed streams. diff --git a/pc/channel.h b/pc/channel.h index ec13f07cf3..352fda4dcd 100644 --- a/pc/channel.h +++ b/pc/channel.h @@ -243,11 +243,7 @@ class BaseChannel void OnDtlsState(DtlsTransportInternal* transport, DtlsTransportState state); - void OnSelectedCandidatePairChanged( - IceTransportInternal* ice_transport, - CandidatePairInterface* selected_candidate_pair, - int last_sent_packet_id, - bool ready_to_send); + void OnNetworkRouteChanged(rtc::Optional network_route); bool PacketIsRtcp(const rtc::PacketTransportInternal* transport, const char* data, @@ -363,8 +359,6 @@ class BaseChannel void SignalSentPacket_w(const rtc::SentPacket& sent_packet); bool IsReadyToSendMedia_n() const; void CacheRtpAbsSendTimeHeaderExtension_n(int rtp_abs_sendtime_extn_id); - int GetTransportOverheadPerPacket() const; - void UpdateTransportOverhead(); // Wraps the existing RtpTransport in an SrtpTransport. void EnableSrtpTransport_n(); diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc index 5736bb6afa..36d7b0026e 100644 --- a/pc/channel_unittest.cc +++ b/pc/channel_unittest.cc @@ -1202,6 +1202,8 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { static constexpr uint16_t kLocalNetId = 1; static constexpr uint16_t kRemoteNetId = 2; static constexpr int kLastPacketId = 100; + // Ipv4(20) + UDP(8). + static constexpr int kTransportOverheadPerPacket = 28; CreateChannels(0, 0); @@ -1209,12 +1211,17 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { static_cast(channel1_->media_channel()); ASSERT_TRUE(media_channel1); + // Need to wait for the threads before calling + // |set_num_network_route_changes| because the network route would be set + // when creating the channel. + WaitForThreads(); media_channel1->set_num_network_route_changes(0); network_thread_->Invoke(RTC_FROM_HERE, [this] { + rtc::NetworkRoute network_route; // The transport channel becomes disconnected. - fake_rtp_dtls_transport1_->ice_transport() - ->SignalSelectedCandidatePairChanged( - fake_rtp_dtls_transport1_->ice_transport(), nullptr, -1, false); + fake_rtp_dtls_transport1_->ice_transport()->SignalNetworkRouteChanged( + + rtc::Optional(network_route)); }); WaitForThreads(); EXPECT_EQ(1, media_channel1->num_network_route_changes()); @@ -1222,15 +1229,16 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { media_channel1->set_num_network_route_changes(0); network_thread_->Invoke(RTC_FROM_HERE, [this] { + rtc::NetworkRoute network_route; + network_route.connected = true; + network_route.local_network_id = kLocalNetId; + network_route.remote_network_id = kRemoteNetId; + network_route.last_sent_packet_id = kLastPacketId; + network_route.packet_overhead = kTransportOverheadPerPacket; // The transport channel becomes connected. - rtc::SocketAddress local_address("192.168.1.1", 1000 /* port number */); - rtc::SocketAddress remote_address("192.168.1.2", 2000 /* port number */); - auto candidate_pair = cricket::FakeCandidatePair::Create( - local_address, kLocalNetId, remote_address, kRemoteNetId); - fake_rtp_dtls_transport1_->ice_transport() - ->SignalSelectedCandidatePairChanged( - fake_rtp_dtls_transport1_->ice_transport(), candidate_pair.get(), - kLastPacketId, true); + fake_rtp_dtls_transport1_->ice_transport()->SignalNetworkRouteChanged( + + rtc::Optional(network_route)); }); WaitForThreads(); EXPECT_EQ(1, media_channel1->num_network_route_changes()); @@ -1239,7 +1247,6 @@ class ChannelTest : public testing::Test, public sigslot::has_slots<> { EXPECT_EQ(expected_network_route, media_channel1->last_network_route()); EXPECT_EQ(kLastPacketId, media_channel1->last_network_route().last_sent_packet_id); - constexpr int kTransportOverheadPerPacket = 28; // Ipv4(20) + UDP(8). EXPECT_EQ(kTransportOverheadPerPacket, media_channel1->transport_overhead_per_packet()); } diff --git a/pc/rtptransport.cc b/pc/rtptransport.cc index 1701c4df16..d2604f9eae 100644 --- a/pc/rtptransport.cc +++ b/pc/rtptransport.cc @@ -11,6 +11,7 @@ #include "pc/rtptransport.h" #include "media/base/rtputils.h" +#include "p2p/base/p2pconstants.h" #include "p2p/base/packettransportinterface.h" #include "rtc_base/checks.h" #include "rtc_base/copyonwritebuffer.h" @@ -31,15 +32,22 @@ void RtpTransport::SetRtpPacketTransport( if (rtp_packet_transport_) { rtp_packet_transport_->SignalReadyToSend.disconnect(this); rtp_packet_transport_->SignalReadPacket.disconnect(this); + rtp_packet_transport_->SignalNetworkRouteChanged.disconnect(this); + // Reset the network route of the old transport. + SignalNetworkRouteChanged(rtc::Optional()); } if (new_packet_transport) { new_packet_transport->SignalReadyToSend.connect( this, &RtpTransport::OnReadyToSend); new_packet_transport->SignalReadPacket.connect(this, &RtpTransport::OnReadPacket); + new_packet_transport->SignalNetworkRouteChanged.connect( + this, &RtpTransport::OnNetworkRouteChange); + // Set the network route for the new transport. + SignalNetworkRouteChanged(new_packet_transport->network_route()); } - rtp_packet_transport_ = new_packet_transport; + rtp_packet_transport_ = new_packet_transport; // Assumes the transport is ready to send if it is writable. If we are wrong, // ready to send will be updated the next time we try to send. SetReadyToSend(false, @@ -54,12 +62,19 @@ void RtpTransport::SetRtcpPacketTransport( if (rtcp_packet_transport_) { rtcp_packet_transport_->SignalReadyToSend.disconnect(this); rtcp_packet_transport_->SignalReadPacket.disconnect(this); + rtcp_packet_transport_->SignalNetworkRouteChanged.disconnect(this); + // Reset the network route of the old transport. + SignalNetworkRouteChanged(rtc::Optional()); } if (new_packet_transport) { new_packet_transport->SignalReadyToSend.connect( this, &RtpTransport::OnReadyToSend); new_packet_transport->SignalReadPacket.connect(this, &RtpTransport::OnReadPacket); + new_packet_transport->SignalNetworkRouteChanged.connect( + this, &RtpTransport::OnNetworkRouteChange); + // Set the network route for the new transport. + SignalNetworkRouteChanged(new_packet_transport->network_route()); } rtcp_packet_transport_ = new_packet_transport; @@ -161,6 +176,11 @@ void RtpTransport::OnReadyToSend(rtc::PacketTransportInternal* transport) { SetReadyToSend(transport == rtcp_packet_transport_, true); } +void RtpTransport::OnNetworkRouteChange( + rtc::Optional network_route) { + SignalNetworkRouteChanged(network_route); +} + void RtpTransport::SetReadyToSend(bool rtcp, bool ready) { if (rtcp) { rtcp_ready_to_send_ = ready; diff --git a/pc/rtptransport.h b/pc/rtptransport.h index 5e1aa20a1f..34ec1b46b9 100644 --- a/pc/rtptransport.h +++ b/pc/rtptransport.h @@ -11,6 +11,8 @@ #ifndef PC_RTPTRANSPORT_H_ #define PC_RTPTRANSPORT_H_ +#include + #include "pc/bundlefilter.h" #include "pc/rtptransportinternal.h" #include "rtc_base/sigslot.h" @@ -76,6 +78,7 @@ class RtpTransport : public RtpTransportInternal { bool HandlesPacket(const uint8_t* data, size_t len); void OnReadyToSend(rtc::PacketTransportInternal* transport); + void OnNetworkRouteChange(rtc::Optional network_route); // Updates "ready to send" for an individual channel and fires // SignalReadyToSend. diff --git a/pc/rtptransport_unittest.cc b/pc/rtptransport_unittest.cc index 1e1657d20c..d6eb336309 100644 --- a/pc/rtptransport_unittest.cc +++ b/pc/rtptransport_unittest.cc @@ -19,6 +19,10 @@ namespace webrtc { constexpr bool kMuxDisabled = false; constexpr bool kMuxEnabled = true; +constexpr uint16_t kLocalNetId = 1; +constexpr uint16_t kRemoteNetId = 2; +constexpr int kLastPacketId = 100; +constexpr int kTransportOverheadPerPacket = 28; // Ipv4(20) + UDP(8). TEST(RtpTransportTest, SetRtcpParametersCantDisableRtcpMux) { RtpTransport transport(kMuxDisabled); @@ -56,12 +60,21 @@ class SignalObserver : public sigslot::has_slots<> { public: explicit SignalObserver(RtpTransport* transport) { transport->SignalReadyToSend.connect(this, &SignalObserver::OnReadyToSend); + transport->SignalNetworkRouteChanged.connect( + this, &SignalObserver::OnNetworkRouteChanged); } + bool ready() const { return ready_; } void OnReadyToSend(bool ready) { ready_ = ready; } + rtc::Optional network_route() { return network_route_; } + void OnNetworkRouteChanged(rtc::Optional network_route) { + network_route_ = network_route; + } + private: bool ready_ = false; + rtc::Optional network_route_; }; TEST(RtpTransportTest, SettingRtcpAndRtpSignalsReady) { @@ -128,6 +141,61 @@ TEST(RtpTransportTest, EnablingRtcpMuxSignalsReady) { EXPECT_TRUE(observer.ready()); } +// Tests the SignalNetworkRoute is fired when setting a packet transport. +TEST(RtpTransportTest, SetRtpTransportWithNetworkRouteChanged) { + RtpTransport transport(kMuxDisabled); + SignalObserver observer(&transport); + rtc::FakePacketTransport fake_rtp("fake_rtp"); + + EXPECT_FALSE(observer.network_route()); + + rtc::NetworkRoute network_route; + // Set a non-null RTP transport with a new network route. + network_route.connected = true; + network_route.local_network_id = kLocalNetId; + network_route.remote_network_id = kRemoteNetId; + network_route.last_sent_packet_id = kLastPacketId; + network_route.packet_overhead = kTransportOverheadPerPacket; + fake_rtp.SetNetworkRoute(rtc::Optional(network_route)); + transport.SetRtpPacketTransport(&fake_rtp); + ASSERT_TRUE(observer.network_route()); + EXPECT_EQ(network_route, *(observer.network_route())); + EXPECT_EQ(kTransportOverheadPerPacket, + observer.network_route()->packet_overhead); + EXPECT_EQ(kLastPacketId, observer.network_route()->last_sent_packet_id); + + // Set a null RTP transport. + transport.SetRtpPacketTransport(nullptr); + EXPECT_FALSE(observer.network_route()); +} + +TEST(RtpTransportTest, SetRtcpTransportWithNetworkRouteChanged) { + RtpTransport transport(kMuxDisabled); + SignalObserver observer(&transport); + rtc::FakePacketTransport fake_rtcp("fake_rtcp"); + + EXPECT_FALSE(observer.network_route()); + + rtc::NetworkRoute network_route; + // Set a non-null RTCP transport with a new network route. + network_route.connected = true; + network_route.local_network_id = kLocalNetId; + network_route.remote_network_id = kRemoteNetId; + network_route.last_sent_packet_id = kLastPacketId; + network_route.packet_overhead = kTransportOverheadPerPacket; + fake_rtcp.SetNetworkRoute(rtc::Optional(network_route)); + transport.SetRtcpPacketTransport(&fake_rtcp); + ASSERT_TRUE(observer.network_route()); + EXPECT_EQ(network_route, *(observer.network_route())); + EXPECT_EQ(kTransportOverheadPerPacket, + observer.network_route()->packet_overhead); + EXPECT_EQ(kLastPacketId, observer.network_route()->last_sent_packet_id); + + // Set a null RTCP transport. + transport.SetRtcpPacketTransport(nullptr); + EXPECT_FALSE(observer.network_route()); +} + class SignalCounter : public sigslot::has_slots<> { public: explicit SignalCounter(RtpTransport* transport) { diff --git a/pc/rtptransportinternal.h b/pc/rtptransportinternal.h index e58961997b..04d2ef39da 100644 --- a/pc/rtptransportinternal.h +++ b/pc/rtptransportinternal.h @@ -11,7 +11,11 @@ #ifndef PC_RTPTRANSPORTINTERNAL_H_ #define PC_RTPTRANSPORTINTERNAL_H_ +#include + #include "api/ortc/rtptransportinterface.h" +#include "p2p/base/icetransportinternal.h" +#include "rtc_base/networkroute.h" #include "rtc_base/sigslot.h" namespace rtc { @@ -52,6 +56,10 @@ class RtpTransportInternal : public RtpTransportInterface, sigslot::signal3 SignalPacketReceived; + // Called whenever the network route of the P2P layer transport changes. + // The argument is an optional network route. + sigslot::signal1> SignalNetworkRouteChanged; + virtual bool IsWritable(bool rtcp) const = 0; virtual bool SendRtpPacket(rtc::CopyOnWriteBuffer* packet, diff --git a/pc/srtptransport.cc b/pc/srtptransport.cc index 1343fd0cac..bb42ad4d3c 100644 --- a/pc/srtptransport.cc +++ b/pc/srtptransport.cc @@ -42,6 +42,8 @@ void SrtpTransport::ConnectToRtpTransport() { this, &SrtpTransport::OnPacketReceived); rtp_transport_->SignalReadyToSend.connect(this, &SrtpTransport::OnReadyToSend); + rtp_transport_->SignalNetworkRouteChanged.connect( + this, &SrtpTransport::OnNetworkRouteChanged); } bool SrtpTransport::SendRtpPacket(rtc::CopyOnWriteBuffer* packet, @@ -170,6 +172,20 @@ void SrtpTransport::OnPacketReceived(bool rtcp, SignalPacketReceived(rtcp, packet, packet_time); } +void SrtpTransport::OnNetworkRouteChanged( + + rtc::Optional network_route) { + // Only append the SRTP overhead when there is a selected network route. + if (network_route) { + int srtp_overhead = 0; + if (IsActive()) { + GetSrtpOverhead(&srtp_overhead); + } + network_route->packet_overhead += srtp_overhead; + } + SignalNetworkRouteChanged(network_route); +} + bool SrtpTransport::SetRtpParams(int send_cs, const uint8_t* send_key, int send_key_len, diff --git a/pc/srtptransport.h b/pc/srtptransport.h index 13abd6b47d..919d4b95d5 100644 --- a/pc/srtptransport.h +++ b/pc/srtptransport.h @@ -16,6 +16,7 @@ #include #include +#include "p2p/base/icetransportinternal.h" #include "pc/rtptransportinternal.h" #include "pc/srtpfilter.h" #include "pc/srtpsession.h" @@ -158,8 +159,8 @@ class SrtpTransport : public RtpTransportInternal { void OnPacketReceived(bool rtcp, rtc::CopyOnWriteBuffer* packet, const rtc::PacketTime& packet_time); - void OnReadyToSend(bool ready) { SignalReadyToSend(ready); } + void OnNetworkRouteChanged(rtc::Optional network_route); bool ProtectRtp(void* data, int in_len, int max_len, int* out_len); diff --git a/pc/transportcontroller.cc b/pc/transportcontroller.cc index 57f90b7591..7cf9bbd3bd 100644 --- a/pc/transportcontroller.cc +++ b/pc/transportcontroller.cc @@ -761,7 +761,7 @@ void TransportController::SetMetricsObserver_n( void TransportController::OnChannelWritableState_n( rtc::PacketTransportInternal* transport) { RTC_DCHECK(network_thread_->IsCurrent()); - RTC_LOG(LS_INFO) << " TransportChannel " << transport->debug_name() + RTC_LOG(LS_INFO) << " Transport " << transport->transport_name() << " writability changed to " << transport->writable() << "."; UpdateAggregateStates_n(); diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 971d32d779..20797cc0ab 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -525,6 +525,8 @@ rtc_static_library("rtc_base_generic") { "messagehandler.h", "messagequeue.cc", "messagequeue.h", + "nethelper.cc", + "nethelper.h", "nethelpers.cc", "nethelpers.h", "network.cc", diff --git a/rtc_base/nethelper.cc b/rtc_base/nethelper.cc new file mode 100644 index 0000000000..e654fe3925 --- /dev/null +++ b/rtc_base/nethelper.cc @@ -0,0 +1,42 @@ +/* + * Copyright 2017 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#include "rtc_base/nethelper.h" + +#include "rtc_base/checks.h" +#include "rtc_base/ipaddress.h" + +namespace cricket { + +const char UDP_PROTOCOL_NAME[] = "udp"; +const char TCP_PROTOCOL_NAME[] = "tcp"; +const char SSLTCP_PROTOCOL_NAME[] = "ssltcp"; +const char TLS_PROTOCOL_NAME[] = "tls"; + +int GetIpOverhead(int addr_family) { + switch (addr_family) { + case AF_INET: // IPv4 + return 20; + case AF_INET6: // IPv6 + return 40; + default: + RTC_NOTREACHED() << "Invaild address family."; + return 0; + } +} + +int GetProtocolOverhead(const std::string& protocol) { + if (protocol == TCP_PROTOCOL_NAME || protocol == SSLTCP_PROTOCOL_NAME) { + return 20; + } + return 8; +} + +} // namespace cricket diff --git a/rtc_base/nethelper.h b/rtc_base/nethelper.h new file mode 100644 index 0000000000..e86d126cbf --- /dev/null +++ b/rtc_base/nethelper.h @@ -0,0 +1,33 @@ +/* + * Copyright 2017 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ +#ifndef RTC_BASE_NETHELPER_H_ +#define RTC_BASE_NETHELPER_H_ + +#include +#include + +// This header contains helper functions and constants used by different types +// of transports. +namespace cricket { + +extern const char UDP_PROTOCOL_NAME[]; +extern const char TCP_PROTOCOL_NAME[]; +extern const char SSLTCP_PROTOCOL_NAME[]; +extern const char TLS_PROTOCOL_NAME[]; + +// Get the network layer overhead per packet based on the IP address family. +int GetIpOverhead(int addr_family); + +// Get the transport layer overhead per packet based on the protocol. +int GetProtocolOverhead(const std::string& protocol); + +} // namespace cricket + +#endif // RTC_BASE_NETHELPER_H_ diff --git a/rtc_base/networkroute.h b/rtc_base/networkroute.h index f245cb9dc3..07cba6324c 100644 --- a/rtc_base/networkroute.h +++ b/rtc_base/networkroute.h @@ -22,14 +22,17 @@ struct NetworkRoute { uint16_t local_network_id; uint16_t remote_network_id; int last_sent_packet_id; // Last packet id sent on the PREVIOUS route. + int packet_overhead; // The overhead in bytes from IP layer and above. NetworkRoute() : connected(false), local_network_id(0), remote_network_id(0), - last_sent_packet_id(-1) {} + last_sent_packet_id(-1), + packet_overhead(0) {} // The route is connected if the local and remote network ids are provided. + // TODO(zhihuang): Remove this and let the caller set the fields explicitly. NetworkRoute(bool connected, uint16_t local_net_id, uint16_t remote_net_id, @@ -37,9 +40,11 @@ struct NetworkRoute { : connected(connected), local_network_id(local_net_id), remote_network_id(remote_net_id), - last_sent_packet_id(last_packet_id) {} + last_sent_packet_id(last_packet_id), + packet_overhead(0) {} - // |last_sent_packet_id| does not affect the NetworkRoute comparison. + // |last_sent_packet_id| and |packet_overhead| do not affect the NetworkRoute + // comparison. bool operator==(const NetworkRoute& nr) const { return connected == nr.connected && local_network_id == nr.local_network_id &&