Modify GetBestIP to prefer global IPv6 addresses to the link local IPv6 addresses.

The change is behind the flag WebRTC-PreferGlobalIPv6ToLinkLocal.

Bug: webrtc:14306
Change-Id: Id2cae2e510fcfd007016e79ae4bd46947101d0fc
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/269762
Commit-Queue: Diep Bui <diepbp@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37658}

p2p/base/basic_ice_controller.cc

@@ -796,7 +796,7 @@ std::vector<const Connection*> BasicIceController::PruneConnections() {
   auto best_connection_by_network = GetBestConnectionByNetwork();
   for (const Connection* conn : connections_) {
     const Connection* best_conn = selected_connection_;
-    if (!rtc::IPIsAny(conn->network()->ip())) {
+    if (!rtc::IPIsAny(conn->network()->GetBestIP())) {
       // If the connection is bound to a specific network interface (not an
       // "any address" network), compare it against the best connection for
       // that network interface rather than the best connection overall. This

p2p/base/port_unittest.cc

@@ -88,6 +88,7 @@ constexpr int kShortTimeout = 1000;
 constexpr int kMaxExpectedSimulatedRtt = 200;
 const SocketAddress kLocalAddr1("192.168.1.2", 0);
 const SocketAddress kLocalAddr2("192.168.1.3", 0);
+const SocketAddress kLinkLocalIPv6Addr("fe80::aabb:ccff:fedd:eeff", 0);
 const SocketAddress kNatAddr1("77.77.77.77", rtc::NAT_SERVER_UDP_PORT);
 const SocketAddress kNatAddr2("88.88.88.88", rtc::NAT_SERVER_UDP_PORT);
 const SocketAddress kStunAddr("99.99.99.1", STUN_SERVER_PORT);
@@ -515,6 +516,18 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> {
     return &networks_.back();
   }
 
+  rtc::Network* MakeNetworkMultipleAddrs(
+      const SocketAddress& global_addr,
+      const SocketAddress& link_local_addr,
+      const webrtc::FieldTrialsView* field_trials) {
+    networks_.emplace_back("unittest", "unittest", global_addr.ipaddr(), 32,
+                           rtc::ADAPTER_TYPE_UNKNOWN, field_trials);
+    networks_.back().AddIP(link_local_addr.ipaddr());
+    networks_.back().AddIP(global_addr.ipaddr());
+    networks_.back().AddIP(link_local_addr.ipaddr());
+    return &networks_.back();
+  }
+
   // helpers for above functions
   std::unique_ptr<UDPPort> CreateUdpPort(const SocketAddress& addr) {
     return CreateUdpPort(addr, &socket_factory_);
@@ -525,6 +538,16 @@ class PortTest : public ::testing::Test, public sigslot::has_slots<> {
                            username_, password_, true, absl::nullopt,
                            &field_trials_);
   }
+  std::unique_ptr<UDPPort> CreateUdpPortMultipleAddrs(
+      const SocketAddress& global_addr,
+      const SocketAddress& link_local_addr,
+      PacketSocketFactory* socket_factory,
+      const webrtc::test::ScopedKeyValueConfig& field_trials) {
+    return UDPPort::Create(
+        &main_, socket_factory,
+        MakeNetworkMultipleAddrs(global_addr, link_local_addr, &field_trials),
+        0, 0, username_, password_, true, absl::nullopt, &field_trials);
+  }
   std::unique_ptr<TCPPort> CreateTcpPort(const SocketAddress& addr) {
     return CreateTcpPort(addr, &socket_factory_);
   }
@@ -1704,7 +1727,7 @@ void PortTest::ExpectPortsCanConnect(bool can_connect, Port* p1, Port* p2) {
   }
 }
 
-TEST_F(PortTest, TestUdpV6CrossTypePorts) {
+TEST_F(PortTest, TestUdpSingleAddressV6CrossTypePorts) {
   FakePacketSocketFactory factory;
   std::unique_ptr<Port> ports[4];
   SocketAddress addresses[4] = {
@@ -1734,6 +1757,42 @@ TEST_F(PortTest, TestUdpV6CrossTypePorts) {
   ExpectPortsCanConnect(true, standard, localhost);
 }
 
+TEST_F(PortTest, TestUdpMultipleAddressesV6CrossTypePorts) {
+  webrtc::test::ScopedKeyValueConfig field_trials(
+      "WebRTC-PreferGlobalIPv6ToLinkLocal/Enabled/");
+  FakePacketSocketFactory factory;
+  std::unique_ptr<Port> ports[5];
+  SocketAddress addresses[5] = {
+      SocketAddress("2001:db8::1", 0), SocketAddress("2001:db8::2", 0),
+      SocketAddress("fe80::1", 0), SocketAddress("fe80::2", 0),
+      SocketAddress("::1", 0)};
+  for (int i = 0; i < 5; i++) {
+    FakeAsyncPacketSocket* socket = new FakeAsyncPacketSocket();
+    factory.set_next_udp_socket(socket);
+    ports[i] = CreateUdpPortMultipleAddrs(addresses[i], kLinkLocalIPv6Addr,
+                                          &factory, field_trials);
+    socket->set_state(AsyncPacketSocket::STATE_BINDING);
+    socket->SignalAddressReady(socket, addresses[i]);
+    ports[i]->PrepareAddress();
+  }
+
+  Port* standard1 = ports[0].get();
+  Port* standard2 = ports[1].get();
+  Port* link_local1 = ports[2].get();
+  Port* link_local2 = ports[3].get();
+  Port* localhost = ports[4].get();
+
+  ExpectPortsCanConnect(false, link_local1, standard1);
+  ExpectPortsCanConnect(false, standard1, link_local1);
+  ExpectPortsCanConnect(false, link_local1, localhost);
+  ExpectPortsCanConnect(false, localhost, link_local1);
+
+  ExpectPortsCanConnect(true, link_local1, link_local2);
+  ExpectPortsCanConnect(true, localhost, standard1);
+  ExpectPortsCanConnect(true, standard1, localhost);
+  ExpectPortsCanConnect(true, standard2, standard1);
+}
+
 // This test verifies DSCP value set through SetOption interface can be
 // get through DefaultDscpValue.
 TEST_F(PortTest, TestDefaultDscpValue) {

rtc_base/fake_network.h

@@ -127,7 +127,7 @@ class FakeNetworkManager : public NetworkManagerBase,
       IPAddress prefix = TruncateIP(it->socket_address.ipaddr(), prefix_length);
       auto net = std::make_unique<Network>(
           it->socket_address.hostname(), it->socket_address.hostname(), prefix,
-          prefix_length, it->adapter_type);
+          prefix_length, it->adapter_type, /*field_trials=*/nullptr);
       if (it->underlying_vpn_adapter_type.has_value()) {
         net->set_underlying_type_for_vpn(*it->underlying_vpn_adapter_type);
       }

rtc_base/network.cc

@@ -287,7 +287,8 @@ webrtc::MdnsResponderInterface* NetworkManager::GetMdnsResponder() const {
 
 NetworkManagerBase::NetworkManagerBase(
     const webrtc::FieldTrialsView* field_trials)
-    : enumeration_permission_(NetworkManager::ENUMERATION_ALLOWED),
+    : field_trials_(field_trials),
+      enumeration_permission_(NetworkManager::ENUMERATION_ALLOWED),
       signal_network_preference_change_(
           field_trials
               ? field_trials->IsEnabled("WebRTC-SignalNetworkPreferenceChange")
@@ -303,7 +304,7 @@ std::vector<const Network*> NetworkManagerBase::GetAnyAddressNetworks() {
   if (!ipv4_any_address_network_) {
     const rtc::IPAddress ipv4_any_address(INADDR_ANY);
     ipv4_any_address_network_ = std::make_unique<Network>(
-        "any", "any", ipv4_any_address, 0, ADAPTER_TYPE_ANY);
+        "any", "any", ipv4_any_address, 0, ADAPTER_TYPE_ANY, field_trials_);
     ipv4_any_address_network_->set_default_local_address_provider(this);
     ipv4_any_address_network_->set_mdns_responder_provider(this);
     ipv4_any_address_network_->AddIP(ipv4_any_address);
@@ -313,7 +314,7 @@ std::vector<const Network*> NetworkManagerBase::GetAnyAddressNetworks() {
   if (!ipv6_any_address_network_) {
     const rtc::IPAddress ipv6_any_address(in6addr_any);
     ipv6_any_address_network_ = std::make_unique<Network>(
-        "any", "any", ipv6_any_address, 0, ADAPTER_TYPE_ANY);
+        "any", "any", ipv6_any_address, 0, ADAPTER_TYPE_ANY, field_trials_);
     ipv6_any_address_network_->set_default_local_address_provider(this);
     ipv6_any_address_network_->set_mdns_responder_provider(this);
     ipv6_any_address_network_->AddIP(ipv6_any_address);
@@ -650,9 +651,9 @@ void BasicNetworkManager::ConvertIfAddrs(
       if_info.adapter_type = ADAPTER_TYPE_VPN;
     }
 
-    auto network =
+    auto network = std::make_unique<Network>(
-        std::make_unique<Network>(cursor->ifa_name, cursor->ifa_name, prefix,
+        cursor->ifa_name, cursor->ifa_name, prefix, prefix_length,
-                                  prefix_length, if_info.adapter_type);
+        if_info.adapter_type, field_trials_.get());
     network->set_default_local_address_provider(this);
     network->set_scope_id(scope_id);
     network->AddIP(ip);
@@ -1063,8 +1064,10 @@ Network::Network(absl::string_view name,
                  absl::string_view desc,
                  const IPAddress& prefix,
                  int prefix_length,
-                 AdapterType type)
+                 AdapterType type,
-    : name_(name),
+                 const webrtc::FieldTrialsView* field_trials)
+    : field_trials_(field_trials),
+      name_(name),
       description_(desc),
       prefix_(prefix),
       prefix_length_(prefix_length),
@@ -1107,13 +1110,21 @@ IPAddress Network::GetBestIP() const {
     return static_cast<IPAddress>(ips_.at(0));
   }
 
-  InterfaceAddress selected_ip, ula_ip;
+  InterfaceAddress selected_ip, link_local_ip, ula_ip;
-
+  const bool prefer_global_ipv6_to_link_local =
+      field_trials_
+          ? field_trials_->IsEnabled("WebRTC-PreferGlobalIPv6ToLinkLocal")
+          : false;
   for (const InterfaceAddress& ip : ips_) {
     // Ignore any address which has been deprecated already.
     if (ip.ipv6_flags() & IPV6_ADDRESS_FLAG_DEPRECATED)
       continue;
 
+    if (prefer_global_ipv6_to_link_local && IPIsLinkLocal(ip)) {
+      link_local_ip = ip;
+      continue;
+    }
+
     // ULA address should only be returned when we have no other
     // global IP.
     if (IPIsULA(static_cast<const IPAddress&>(ip))) {
@@ -1127,9 +1138,14 @@ IPAddress Network::GetBestIP() const {
       break;
   }
 
-  // No proper global IPv6 address found, use ULA instead.
+  if (IPIsUnspec(selected_ip)) {
-  if (IPIsUnspec(selected_ip) && !IPIsUnspec(ula_ip)) {
+    if (prefer_global_ipv6_to_link_local && !IPIsUnspec(link_local_ip)) {
-    selected_ip = ula_ip;
+      // No proper global IPv6 address found, use link local address instead.
+      selected_ip = link_local_ip;
+    } else if (!IPIsUnspec(ula_ip)) {
+      // No proper global and link local address found, use ULA instead.
+      selected_ip = ula_ip;
+    }
   }
 
   return static_cast<IPAddress>(selected_ip);

rtc_base/network.h

@@ -233,7 +233,7 @@ class RTC_EXPORT NetworkManagerBase : public NetworkManager {
 
  private:
   friend class NetworkTest;
-
+  const webrtc::FieldTrialsView* field_trials_ = nullptr;
   EnumerationPermission enumeration_permission_;
 
   std::vector<Network*> networks_;
@@ -370,18 +370,21 @@ class RTC_EXPORT Network {
   Network(absl::string_view name,
           absl::string_view description,
           const IPAddress& prefix,
-          int prefix_length)
+          int prefix_length,
+          const webrtc::FieldTrialsView* field_trials = nullptr)
       : Network(name,
                 description,
                 prefix,
                 prefix_length,
-                rtc::ADAPTER_TYPE_UNKNOWN) {}
+                rtc::ADAPTER_TYPE_UNKNOWN,
+                field_trials) {}
 
   Network(absl::string_view name,
           absl::string_view description,
           const IPAddress& prefix,
           int prefix_length,
-          AdapterType type);
+          AdapterType type,
+          const webrtc::FieldTrialsView* field_trials = nullptr);
 
   Network(const Network&);
   ~Network();
@@ -427,9 +430,11 @@ class RTC_EXPORT Network {
   // Here is the rule on how we mark the IPv6 address as ignorable for WebRTC.
   // 1) return all global temporary dynamic and non-deprecated ones.
   // 2) if #1 not available, return global ones.
-  // 3) if #2 not available, use ULA ipv6 as last resort. (ULA stands
+  // 3) if #2 not available and WebRTC-PreferGlobalIPv6ToLinkLocal enabled,
-  // for unique local address, which is not route-able in open
+  // return local link ones.
-  // internet but might be useful for a close WebRTC deployment.
+  // 4) if #3 not available, use ULA ipv6 as last resort. (ULA stands for
+  // unique local address, which is not route-able in open internet but might
+  // be useful for a close WebRTC deployment.
 
   // TODO(guoweis): rule #3 actually won't happen at current
   // implementation. The reason being that ULA address starting with
@@ -442,10 +447,6 @@ class RTC_EXPORT Network {
   // IPv6 address
   IPAddress GetBestIP() const;
 
-  // Keep the original function here for now.
-  // TODO(guoweis): Remove this when all callers are migrated to GetBestIP().
-  IPAddress ip() const { return GetBestIP(); }
-
   // Adds an active IP address to this network. Does not check for duplicates.
   void AddIP(const InterfaceAddress& ip) { ips_.push_back(ip); }
   void AddIP(const IPAddress& ip) { ips_.push_back(rtc::InterfaceAddress(ip)); }
@@ -564,6 +565,7 @@ class RTC_EXPORT Network {
   std::string ToString() const;
 
  private:
+  const webrtc::FieldTrialsView* field_trials_ = nullptr;
   const DefaultLocalAddressProvider* default_local_address_provider_ = nullptr;
   const MdnsResponderProvider* mdns_responder_provider_ = nullptr;
   std::string name_;

rtc_base/network_unittest.cc

@@ -339,9 +339,11 @@ TEST_F(NetworkTest, TestNetworkConstruct) {
 
 TEST_F(NetworkTest, TestIsIgnoredNetworkIgnoresIPsStartingWith0) {
   Network ipv4_network1("test_eth0", "Test Network Adapter 1",
-                        IPAddress(0x12345600U), 24, ADAPTER_TYPE_ETHERNET);
+                        IPAddress(0x12345600U), 24, ADAPTER_TYPE_ETHERNET,
+                        &field_trials_);
   Network ipv4_network2("test_eth1", "Test Network Adapter 2",
-                        IPAddress(0x010000U), 24, ADAPTER_TYPE_ETHERNET);
+                        IPAddress(0x010000U), 24, ADAPTER_TYPE_ETHERNET,
+                        &field_trials_);
   PhysicalSocketServer socket_server;
   BasicNetworkManager network_manager(&socket_server);
   network_manager.StartUpdating();
@@ -821,19 +823,19 @@ TEST_F(NetworkTest, NetworksSortedByInterfaceName) {
 
 TEST_F(NetworkTest, TestNetworkAdapterTypes) {
   Network wifi("wlan0", "Wireless Adapter", IPAddress(0x12345600U), 24,
-               ADAPTER_TYPE_WIFI);
+               ADAPTER_TYPE_WIFI, &field_trials_);
   EXPECT_EQ(ADAPTER_TYPE_WIFI, wifi.type());
   Network ethernet("eth0", "Ethernet", IPAddress(0x12345600U), 24,
-                   ADAPTER_TYPE_ETHERNET);
+                   ADAPTER_TYPE_ETHERNET, &field_trials_);
   EXPECT_EQ(ADAPTER_TYPE_ETHERNET, ethernet.type());
   Network cellular("test_cell", "Cellular Adapter", IPAddress(0x12345600U), 24,
-                   ADAPTER_TYPE_CELLULAR);
+                   ADAPTER_TYPE_CELLULAR, &field_trials_);
   EXPECT_EQ(ADAPTER_TYPE_CELLULAR, cellular.type());
   Network vpn("bridge_test", "VPN Adapter", IPAddress(0x12345600U), 24,
-              ADAPTER_TYPE_VPN);
+              ADAPTER_TYPE_VPN, &field_trials_);
   EXPECT_EQ(ADAPTER_TYPE_VPN, vpn.type());
   Network unknown("test", "Test Adapter", IPAddress(0x12345600U), 24,
-                  ADAPTER_TYPE_UNKNOWN);
+                  ADAPTER_TYPE_UNKNOWN, &field_trials_);
   EXPECT_EQ(ADAPTER_TYPE_UNKNOWN, unknown.type());
 }
 
@@ -1153,6 +1155,69 @@ TEST_F(NetworkTest, TestIPv6Selection) {
   EXPECT_EQ(ipv6_network.GetBestIP(), static_cast<IPAddress>(ip));
 }
 
+// Test that the filtering logic follows the defined ruleset in network.h.
+TEST_F(NetworkTest, TestGetBestIPWithPreferGlobalIPv6ToLinkLocalEnabled) {
+  webrtc::test::ScopedKeyValueConfig field_trials(
+      "WebRTC-PreferGlobalIPv6ToLinkLocal/Enabled/");
+  InterfaceAddress ip, link_local;
+  std::string ipstr;
+
+  ipstr = "2401:fa00:4:1000:be30:5bff:fee5:c3";
+  ASSERT_TRUE(IPFromString(ipstr, IPV6_ADDRESS_FLAG_DEPRECATED, &ip));
+
+  // Create a network with this prefix.
+  Network ipv6_network("test_eth0", "Test NetworkAdapter", TruncateIP(ip, 64),
+                       64, ADAPTER_TYPE_UNKNOWN, &field_trials);
+
+  // When there is no address added, it should return an unspecified
+  // address.
+  EXPECT_EQ(ipv6_network.GetBestIP(), IPAddress());
+  EXPECT_TRUE(IPIsUnspec(ipv6_network.GetBestIP()));
+
+  // Deprecated one should not be returned.
+  ipv6_network.AddIP(ip);
+  EXPECT_EQ(ipv6_network.GetBestIP(), IPAddress());
+
+  // Add ULA one. ULA is unique local address which is starting either
+  // with 0xfc or 0xfd.
+  ipstr = "fd00:fa00:4:1000:be30:5bff:fee5:c4";
+  ASSERT_TRUE(IPFromString(ipstr, IPV6_ADDRESS_FLAG_NONE, &ip));
+  ipv6_network.AddIP(ip);
+  EXPECT_EQ(ipv6_network.GetBestIP(), static_cast<IPAddress>(ip));
+
+  // Add link local one.
+  ipstr = "fe80::aabb:ccff:fedd:eeff";
+  ASSERT_TRUE(IPFromString(ipstr, IPV6_ADDRESS_FLAG_NONE, &link_local));
+  ipv6_network.AddIP(link_local);
+  EXPECT_EQ(ipv6_network.GetBestIP(), static_cast<IPAddress>(link_local));
+
+  // Add global one.
+  ipstr = "2401:fa00:4:1000:be30:5bff:fee5:c5";
+  ASSERT_TRUE(IPFromString(ipstr, IPV6_ADDRESS_FLAG_NONE, &ip));
+  ipv6_network.AddIP(ip);
+  EXPECT_EQ(ipv6_network.GetBestIP(), static_cast<IPAddress>(ip));
+
+  // Add another link local address, then the compatible address is still global
+  // one.
+  ipstr = "fe80::aabb:ccff:fedd:eedd";
+  ASSERT_TRUE(IPFromString(ipstr, IPV6_ADDRESS_FLAG_NONE, &link_local));
+  ipv6_network.AddIP(link_local);
+  EXPECT_EQ(ipv6_network.GetBestIP(), static_cast<IPAddress>(ip));
+
+  // Add global dynamic temporary one.
+  ipstr = "2401:fa00:4:1000:be30:5bff:fee5:c6";
+  ASSERT_TRUE(IPFromString(ipstr, IPV6_ADDRESS_FLAG_TEMPORARY, &ip));
+  ipv6_network.AddIP(ip);
+  EXPECT_EQ(ipv6_network.GetBestIP(), static_cast<IPAddress>(ip));
+
+  // Add another link local address, then the compatible address is still global
+  // dynamic one.
+  ipstr = "fe80::aabb:ccff:fedd:eedd";
+  ASSERT_TRUE(IPFromString(ipstr, IPV6_ADDRESS_FLAG_NONE, &link_local));
+  ipv6_network.AddIP(link_local);
+  EXPECT_EQ(ipv6_network.GetBestIP(), static_cast<IPAddress>(ip));
+}
+
 TEST_F(NetworkTest, TestNetworkMonitoring) {
   FakeNetworkMonitorFactory factory;
   PhysicalSocketServer socket_server;