Extend NetworkRoute with more info about local/remote endpoints
This patch extends the NetworkRoute struct with more information
about local/remote endpoints. It adds
- adapter type
- adapter id
- relay
(previously it was "only" network_id)
The patch leaves the {local/remote}_network_id fields
around and populated since downstream projects depend
on them. They will be removed once they have migrated.
OWNER: srte@ call/ test/
OWNER: asapersson@ video/
OWNER: hta@ p2p/ pc/ rtc_base/
BUG: webrtc:11434
Change-Id: I9bcec385b40d707db385fef40b2c7a315dd35dd0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/170628
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Åsa Persson <asapersson@webrtc.org>
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30848}
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index fb684ca..f219c21 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -249,8 +249,11 @@
const std::string& transport_name,
const rtc::NetworkRoute& network_route) {
// Check if the network route is connected.
+
+ RTC_LOG(LS_INFO) << "Network route changed on transport " << transport_name
+ << ": new_route = " << network_route.DebugString();
+
if (!network_route.connected) {
- RTC_LOG(LS_INFO) << "Transport " << transport_name << " is disconnected";
// TODO(honghaiz): Perhaps handle this in SignalChannelNetworkState and
// consider merging these two methods.
return;
@@ -269,17 +272,23 @@
// No need to reset BWE if this is the first time the network connects.
return;
}
- if (kv->second.connected != network_route.connected ||
- kv->second.local_network_id != network_route.local_network_id ||
- kv->second.remote_network_id != network_route.remote_network_id) {
- kv->second = network_route;
+ //
+ auto old_route = kv->second;
+ kv->second = network_route;
+ RTC_LOG(LS_INFO) << "old_route = " << old_route.DebugString();
+
+ // Check if enough conditions of the new/old route has changed
+ // to trigger resetting of bitrates (and a probe).
+ // Currently we only check local/remote network id (i.e IP address) and
+ // connected state and do not consider if we change route due to TURN.
+ //
+ // TODO(bugs.webrtc.org/11438) : Experiment with using more information/
+ // other conditions.
+ if (old_route.connected != network_route.connected ||
+ old_route.local.network_id() != network_route.local.network_id() ||
+ old_route.remote.network_id() != network_route.remote.network_id()) {
BitrateConstraints bitrate_config = bitrate_configurator_.GetConfig();
- RTC_LOG(LS_INFO) << "Network route changed on transport " << transport_name
- << ": new local network id "
- << network_route.local_network_id
- << " new remote network id "
- << network_route.remote_network_id
- << " Reset bitrates to min: "
+ RTC_LOG(LS_INFO) << "Reset bitrates to min: "
<< bitrate_config.min_bitrate_bps
<< " bps, start: " << bitrate_config.start_bitrate_bps
<< " bps, max: " << bitrate_config.max_bitrate_bps
@@ -297,8 +306,11 @@
RTC_DCHECK_RUN_ON(&task_queue_);
transport_overhead_bytes_per_packet_ = network_route.packet_overhead;
if (reset_feedback_on_route_change_) {
+ // TODO(bugs.webrtc.org/11438) : Consider if transport_feedback_adapter
+ // should have a real "route" rather than just local/remote network_id.
transport_feedback_adapter_.SetNetworkIds(
- network_route.local_network_id, network_route.remote_network_id);
+ network_route.local.network_id(),
+ network_route.remote.network_id());
}
if (controller_) {
PostUpdates(controller_->OnNetworkRouteChange(msg));
diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index 906aa59..d935a45 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -57,6 +57,43 @@
return cricket::WEAK_PING_INTERVAL;
}
+rtc::AdapterType GuessAdapterTypeFromNetworkCost(int network_cost) {
+ // The current network costs have been unchanged since they were added
+ // to webrtc. If they ever were to change we would need to reconsider
+ // this method.
+ switch (network_cost) {
+ case rtc::kNetworkCostMin:
+ return rtc::ADAPTER_TYPE_ETHERNET;
+ case rtc::kNetworkCostLow:
+ return rtc::ADAPTER_TYPE_WIFI;
+ case rtc::kNetworkCostHigh:
+ return rtc::ADAPTER_TYPE_CELLULAR;
+ case rtc::kNetworkCostUnknown:
+ return rtc::ADAPTER_TYPE_UNKNOWN;
+ case rtc::kNetworkCostMax:
+ return rtc::ADAPTER_TYPE_ANY;
+ }
+ return rtc::ADAPTER_TYPE_UNKNOWN;
+}
+
+rtc::RouteEndpoint CreateRouteEndpointFromCandidate(
+ bool local,
+ const cricket::Candidate& candidate,
+ bool uses_turn) {
+ auto adapter_type = candidate.network_type();
+ if (!local && adapter_type == rtc::ADAPTER_TYPE_UNKNOWN) {
+ adapter_type = GuessAdapterTypeFromNetworkCost(candidate.network_cost());
+ }
+
+ // TODO(bugs.webrtc.org/9446) : Rewrite if information about remote network
+ // adapter becomes available. The implication of this implementation is that
+ // we will only ever report 1 adapter per type. In practice this is probably
+ // fine, since the endpoint also contains network-id.
+ uint16_t adapter_id = static_cast<int>(adapter_type);
+ return rtc::RouteEndpoint(adapter_type, adapter_id, candidate.network_id(),
+ uses_turn);
+}
+
} // unnamed namespace
namespace cricket {
@@ -1687,10 +1724,21 @@
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_->local = CreateRouteEndpointFromCandidate(
+ /* local= */ true, selected_connection_->local_candidate(),
+ /* uses_turn= */ selected_connection_->port()->Type() ==
+ RELAY_PORT_TYPE);
+ network_route_->remote = CreateRouteEndpointFromCandidate(
+ /* local= */ false, selected_connection_->remote_candidate(),
+ /* uses_turn= */ selected_connection_->remote_candidate().type() ==
+ RELAY_PORT_TYPE);
+
+ // Downstream projects depend on the old representation,
+ // populate that until they have been migrated.
+ // TODO(jonaso): remove.
+ network_route_->local_network_id = network_route_->local.network_id();
+ network_route_->remote_network_id = network_route_->remote.network_id();
+
network_route_->last_sent_packet_id = last_sent_packet_id_;
network_route_->packet_overhead =
selected_connection_->local_candidate().address().ipaddr().overhead() +
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index ee7456a..c66a9f7 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -3221,9 +3221,9 @@
return !last_network_route_.has_value();
} else {
return pair->local_candidate().network_id() ==
- last_network_route_->local_network_id &&
+ last_network_route_->local.network_id() &&
pair->remote_candidate().network_id() ==
- last_network_route_->remote_network_id;
+ last_network_route_->remote.network_id();
}
}
diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc
index d5c51ec..c1037f7 100644
--- a/pc/channel_unittest.cc
+++ b/pc/channel_unittest.cc
@@ -843,7 +843,6 @@
rtc::NetworkRoute network_route;
// The transport channel becomes disconnected.
fake_rtp_dtls_transport1_->ice_transport()->SignalNetworkRouteChanged(
-
absl::optional<rtc::NetworkRoute>(network_route));
});
WaitForThreads();
@@ -854,8 +853,10 @@
network_thread_->Invoke<void>(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.local =
+ rtc::RouteEndpoint::CreateWithNetworkId(kLocalNetId);
+ network_route.remote =
+ rtc::RouteEndpoint::CreateWithNetworkId(kRemoteNetId);
network_route.last_sent_packet_id = kLastPacketId;
network_route.packet_overhead = kTransportOverheadPerPacket;
// The transport channel becomes connected.
@@ -867,9 +868,9 @@
EXPECT_EQ(1, media_channel1->num_network_route_changes());
EXPECT_TRUE(media_channel1->last_network_route().connected);
EXPECT_EQ(kLocalNetId,
- media_channel1->last_network_route().local_network_id);
+ media_channel1->last_network_route().local.network_id());
EXPECT_EQ(kRemoteNetId,
- media_channel1->last_network_route().remote_network_id);
+ media_channel1->last_network_route().remote.network_id());
EXPECT_EQ(kLastPacketId,
media_channel1->last_network_route().last_sent_packet_id);
EXPECT_EQ(kTransportOverheadPerPacket + kSrtpOverheadPerPacket,
diff --git a/pc/composite_rtp_transport_test.cc b/pc/composite_rtp_transport_test.cc
index 0248084..fee8c21 100644
--- a/pc/composite_rtp_transport_test.cc
+++ b/pc/composite_rtp_transport_test.cc
@@ -229,17 +229,17 @@
SetupRtpTransports(/*rtcp_mux=*/true);
rtc::NetworkRoute route;
- route.local_network_id = 7;
+ route.local = rtc::RouteEndpoint::CreateWithNetworkId(7);
packet_transport_1_->SetNetworkRoute(route);
EXPECT_EQ(1, network_route_count_);
- EXPECT_EQ(7, last_network_route_->local_network_id);
+ EXPECT_EQ(7, last_network_route_->local.network_id());
- route.local_network_id = 8;
+ route.local = rtc::RouteEndpoint::CreateWithNetworkId(8);
packet_transport_2_->SetNetworkRoute(route);
EXPECT_EQ(2, network_route_count_);
- EXPECT_EQ(8, last_network_route_->local_network_id);
+ EXPECT_EQ(8, last_network_route_->local.network_id());
}
TEST_F(CompositeRtpTransportTest, RemoveTransport) {
@@ -249,7 +249,7 @@
// Check that signals are disconnected.
rtc::NetworkRoute route;
- route.local_network_id = 7;
+ route.local = rtc::RouteEndpoint::CreateWithNetworkId(7);
packet_transport_1_->SetNetworkRoute(route);
EXPECT_EQ(0, network_route_count_);
diff --git a/pc/rtp_transport_unittest.cc b/pc/rtp_transport_unittest.cc
index 03e8820..b3bd1db 100644
--- a/pc/rtp_transport_unittest.cc
+++ b/pc/rtp_transport_unittest.cc
@@ -155,16 +155,16 @@
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.local = rtc::RouteEndpoint::CreateWithNetworkId(kLocalNetId);
+ network_route.remote = rtc::RouteEndpoint::CreateWithNetworkId(kRemoteNetId);
network_route.last_sent_packet_id = kLastPacketId;
network_route.packet_overhead = kTransportOverheadPerPacket;
fake_rtp.SetNetworkRoute(absl::optional<rtc::NetworkRoute>(network_route));
transport.SetRtpPacketTransport(&fake_rtp);
ASSERT_TRUE(observer.network_route());
EXPECT_TRUE(observer.network_route()->connected);
- EXPECT_EQ(kLocalNetId, observer.network_route()->local_network_id);
- EXPECT_EQ(kRemoteNetId, observer.network_route()->remote_network_id);
+ EXPECT_EQ(kLocalNetId, observer.network_route()->local.network_id());
+ EXPECT_EQ(kRemoteNetId, observer.network_route()->remote.network_id());
EXPECT_EQ(kTransportOverheadPerPacket,
observer.network_route()->packet_overhead);
EXPECT_EQ(kLastPacketId, observer.network_route()->last_sent_packet_id);
@@ -184,16 +184,16 @@
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.local = rtc::RouteEndpoint::CreateWithNetworkId(kLocalNetId);
+ network_route.remote = rtc::RouteEndpoint::CreateWithNetworkId(kRemoteNetId);
network_route.last_sent_packet_id = kLastPacketId;
network_route.packet_overhead = kTransportOverheadPerPacket;
fake_rtcp.SetNetworkRoute(absl::optional<rtc::NetworkRoute>(network_route));
transport.SetRtcpPacketTransport(&fake_rtcp);
ASSERT_TRUE(observer.network_route());
EXPECT_TRUE(observer.network_route()->connected);
- EXPECT_EQ(kLocalNetId, observer.network_route()->local_network_id);
- EXPECT_EQ(kRemoteNetId, observer.network_route()->remote_network_id);
+ EXPECT_EQ(kLocalNetId, observer.network_route()->local.network_id());
+ EXPECT_EQ(kRemoteNetId, observer.network_route()->remote.network_id());
EXPECT_EQ(kTransportOverheadPerPacket,
observer.network_route()->packet_overhead);
EXPECT_EQ(kLastPacketId, observer.network_route()->last_sent_packet_id);
diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn
index 2e4138e..2c6dd3c 100644
--- a/rtc_base/BUILD.gn
+++ b/rtc_base/BUILD.gn
@@ -765,6 +765,7 @@
"../system_wrappers:field_trial",
"network:sent_packet",
"system:file_wrapper",
+ "system:inline",
"system:rtc_export",
"task_utils:to_queued_task",
"third_party/base64",
@@ -818,6 +819,7 @@
"net_helpers.h",
"network.cc",
"network.h",
+ "network_constants.cc",
"network_constants.h",
"network_monitor.cc",
"network_monitor.h",
diff --git a/rtc_base/network.cc b/rtc_base/network.cc
index 4906184..07b121b 100644
--- a/rtc_base/network.cc
+++ b/rtc_base/network.cc
@@ -85,28 +85,6 @@
return a->key() < b->key();
}
-std::string AdapterTypeToString(AdapterType type) {
- switch (type) {
- case ADAPTER_TYPE_ANY:
- return "Wildcard";
- case ADAPTER_TYPE_UNKNOWN:
- return "Unknown";
- case ADAPTER_TYPE_ETHERNET:
- return "Ethernet";
- case ADAPTER_TYPE_WIFI:
- return "Wifi";
- case ADAPTER_TYPE_CELLULAR:
- return "Cellular";
- case ADAPTER_TYPE_VPN:
- return "VPN";
- case ADAPTER_TYPE_LOOPBACK:
- return "Loopback";
- default:
- RTC_NOTREACHED() << "Invalid type " << type;
- return std::string();
- }
-}
-
uint16_t ComputeNetworkCostByType(int type) {
switch (type) {
case rtc::ADAPTER_TYPE_ETHERNET:
diff --git a/rtc_base/network_constants.cc b/rtc_base/network_constants.cc
new file mode 100644
index 0000000..2cb5233
--- /dev/null
+++ b/rtc_base/network_constants.cc
@@ -0,0 +1,39 @@
+/*
+ * Copyright 2020 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/network_constants.h"
+
+#include "rtc_base/checks.h"
+
+namespace rtc {
+
+std::string AdapterTypeToString(AdapterType type) {
+ switch (type) {
+ case ADAPTER_TYPE_ANY:
+ return "Wildcard";
+ case ADAPTER_TYPE_UNKNOWN:
+ return "Unknown";
+ case ADAPTER_TYPE_ETHERNET:
+ return "Ethernet";
+ case ADAPTER_TYPE_WIFI:
+ return "Wifi";
+ case ADAPTER_TYPE_CELLULAR:
+ return "Cellular";
+ case ADAPTER_TYPE_VPN:
+ return "VPN";
+ case ADAPTER_TYPE_LOOPBACK:
+ return "Loopback";
+ default:
+ RTC_NOTREACHED() << "Invalid type " << type;
+ return std::string();
+ }
+}
+
+} // namespace rtc
diff --git a/rtc_base/network_constants.h b/rtc_base/network_constants.h
index efb2c83..1b43243 100644
--- a/rtc_base/network_constants.h
+++ b/rtc_base/network_constants.h
@@ -13,6 +13,8 @@
#include <stdint.h>
+#include <string>
+
namespace rtc {
static const uint16_t kNetworkCostMax = 999;
@@ -37,6 +39,8 @@
ADAPTER_TYPE_ANY = 1 << 5,
};
+std::string AdapterTypeToString(AdapterType type);
+
} // namespace rtc
#endif // RTC_BASE_NETWORK_CONSTANTS_H_
diff --git a/rtc_base/network_route.h b/rtc_base/network_route.h
index 6a8f183..c2b492c 100644
--- a/rtc_base/network_route.h
+++ b/rtc_base/network_route.h
@@ -13,21 +13,82 @@
#include <stdint.h>
+#include <string>
+
+#include "rtc_base/network_constants.h"
+#include "rtc_base/strings/string_builder.h"
+#include "rtc_base/system/inline.h"
+
// TODO(honghaiz): Make a directory that describes the interfaces and structs
// the media code can rely on and the network code can implement, and both can
// depend on that, but not depend on each other. Then, move this file to that
// directory.
namespace rtc {
+class RouteEndpoint {
+ public:
+ RouteEndpoint() {} // Used by tests.
+ RouteEndpoint(AdapterType adapter_type,
+ uint16_t adapter_id,
+ uint16_t network_id,
+ bool uses_turn)
+ : adapter_type_(adapter_type),
+ adapter_id_(adapter_id),
+ network_id_(network_id),
+ uses_turn_(uses_turn) {}
+
+ RouteEndpoint(const RouteEndpoint&) = default;
+ RouteEndpoint& operator=(const RouteEndpoint&) = default;
+
+ // Used by tests.
+ static RouteEndpoint CreateWithNetworkId(uint16_t network_id) {
+ return RouteEndpoint(ADAPTER_TYPE_UNKNOWN,
+ /* adapter_id = */ 0, network_id,
+ /* uses_turn = */ false);
+ }
+
+ AdapterType adapter_type() const { return adapter_type_; }
+ uint16_t adapter_id() const { return adapter_id_; }
+ uint16_t network_id() const { return network_id_; }
+ bool uses_turn() const { return uses_turn_; }
+
+ private:
+ AdapterType adapter_type_ = ADAPTER_TYPE_UNKNOWN;
+ uint16_t adapter_id_ = 0;
+ uint16_t network_id_ = 0;
+ bool uses_turn_ = false;
+};
+
struct NetworkRoute {
bool connected = false;
- uint16_t local_network_id = 0;
- uint16_t remote_network_id = 0;
+ RouteEndpoint local;
+ RouteEndpoint remote;
// Last packet id sent on the PREVIOUS route.
int last_sent_packet_id = -1;
// The overhead in bytes from IP layer and above.
+ // This is the maximum of any part of the route.
int packet_overhead = 0;
+
+ // Downstream projects depend on the old representation,
+ // populate that until they have been migrated.
+ // TODO(jonaso): remove.
+ uint16_t local_network_id = 0;
+ uint16_t remote_network_id = 0;
+
+ RTC_NO_INLINE inline std::string DebugString() const {
+ rtc::StringBuilder oss;
+ oss << "[ connected: " << connected << " local: [ " << local.adapter_id()
+ << "/" << local.network_id() << " "
+ << AdapterTypeToString(local.adapter_type())
+ << " turn: " << local.uses_turn() << " ] remote: [ "
+ << remote.adapter_id() << "/" << remote.network_id() << " "
+ << AdapterTypeToString(remote.adapter_type())
+ << " turn: " << remote.uses_turn()
+ << " ] packet_overhead_bytes: " << packet_overhead << " ]";
+ return oss.Release();
+ }
};
+
} // namespace rtc
#endif // RTC_BASE_NETWORK_ROUTE_H_
diff --git a/test/scenario/network_node.cc b/test/scenario/network_node.cc
index c874add..aa576dc 100644
--- a/test/scenario/network_node.cc
+++ b/test/scenario/network_node.cc
@@ -111,10 +111,10 @@
rtc::NetworkRoute route;
route.connected = true;
// We assume that the address will be unique in the lower bytes.
- route.local_network_id = static_cast<uint16_t>(
- receiver_address.ipaddr().v4AddressAsHostOrderInteger());
- route.remote_network_id = static_cast<uint16_t>(
- receiver_address.ipaddr().v4AddressAsHostOrderInteger());
+ route.local = rtc::RouteEndpoint::CreateWithNetworkId(static_cast<uint16_t>(
+ receiver_address.ipaddr().v4AddressAsHostOrderInteger()));
+ route.remote = rtc::RouteEndpoint::CreateWithNetworkId(static_cast<uint16_t>(
+ receiver_address.ipaddr().v4AddressAsHostOrderInteger()));
route.packet_overhead = packet_overhead.bytes() +
receiver_address.ipaddr().overhead() +
cricket::kUdpHeaderSize;
diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc
index 27bf0f0..cbc12a9 100644
--- a/video/video_send_stream_tests.cc
+++ b/video/video_send_stream_tests.cc
@@ -1772,8 +1772,8 @@
void PerformTest() override {
rtc::NetworkRoute new_route;
new_route.connected = true;
- new_route.local_network_id = 10;
- new_route.remote_network_id = 20;
+ new_route.local = rtc::RouteEndpoint::CreateWithNetworkId(10);
+ new_route.remote = rtc::RouteEndpoint::CreateWithNetworkId(20);
BitrateConstraints bitrate_config;
SendTask(RTC_FROM_HERE, task_queue_,
@@ -1799,7 +1799,8 @@
// TODO(holmer): We should set the last sent packet id here and
// verify that we correctly ignore any packet loss reported prior to
// that id.
- ++new_route.local_network_id;
+ new_route.local = rtc::RouteEndpoint::CreateWithNetworkId(
+ new_route.local.network_id() + 1);
call_->GetTransportControllerSend()->OnNetworkRouteChanged(
"transport", new_route);
EXPECT_GE(call_->GetStats().send_bandwidth_bps, kStartBitrateBps);
