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}

call/rtp_transport_controller_send.cc

@@ -249,8 +249,11 @@ void RtpTransportControllerSend::OnNetworkRouteChanged(
     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 @@ void RtpTransportControllerSend::OnNetworkRouteChanged(
     // 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) {
+  //
+  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 @@ void RtpTransportControllerSend::OnNetworkRouteChanged(
       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));

p2p/base/p2p_transport_channel.cc

@@ -57,6 +57,43 @@ uint32_t GetWeakPingIntervalInFieldTrial() {
   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 @@ void P2PTransportChannel::SwitchSelectedConnection(Connection* conn,
 
     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() +

p2p/base/p2p_transport_channel_unittest.cc

@@ -3221,9 +3221,9 @@ class P2PTransportChannelPingTest : public ::testing::Test,
       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();
     }
   }
 

pc/channel_unittest.cc

@@ -843,7 +843,6 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> {
       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 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> {
     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 @@ class ChannelTest : public ::testing::Test, public sigslot::has_slots<> {
     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,

pc/composite_rtp_transport_test.cc

@@ -229,17 +229,17 @@ TEST_F(CompositeRtpTransportTest, NetworkRouteChange) {
   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 @@ TEST_F(CompositeRtpTransportTest, RemoveTransport) {
 
   // 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_);

pc/rtp_transport_unittest.cc

@@ -155,16 +155,16 @@ TEST(RtpTransportTest, SetRtpTransportWithNetworkRouteChanged) {
   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 @@ TEST(RtpTransportTest, SetRtcpTransportWithNetworkRouteChanged) {
   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);

rtc_base/BUILD.gn

@@ -765,6 +765,7 @@ rtc_library("rtc_base") {
     "../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 @@ rtc_library("rtc_base") {
     "net_helpers.h",
     "network.cc",
     "network.h",
+    "network_constants.cc",
     "network_constants.h",
     "network_monitor.cc",
     "network_monitor.h",

rtc_base/network.cc

@@ -85,28 +85,6 @@ bool SortNetworks(const Network* a, const Network* b) {
   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:

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

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 @@ enum AdapterType {
   ADAPTER_TYPE_ANY = 1 << 5,
 };
 
+std::string AdapterTypeToString(AdapterType type);
+
 }  // namespace rtc
 
 #endif  // RTC_BASE_NETWORK_CONSTANTS_H_

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_

test/scenario/network_node.cc

@@ -111,10 +111,10 @@ void NetworkNodeTransport::Connect(EmulatedEndpoint* endpoint,
   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;

video/video_send_stream_tests.cc

@@ -1772,8 +1772,8 @@ TEST_F(VideoSendStreamTest, ChangingNetworkRoute) {
     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 @@ TEST_F(VideoSendStreamTest, ChangingNetworkRoute) {
             // 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);