diff --git a/webrtc/p2p/stunprober/main.cc b/webrtc/p2p/stunprober/main.cc index 762b6da6d..99f66caa9 100644 --- a/webrtc/p2p/stunprober/main.cc +++ b/webrtc/p2p/stunprober/main.cc @@ -106,16 +106,19 @@ class HostNameResolver : public HostNameResolverInterface, rtc::AsyncResolver* resolver_; }; -std::string HistogramName(bool behind_nat, - bool is_src_port_shared, - int interval_ms, - std::string suffix) { - char output[1000]; - rtc::sprintfn(output, sizeof(output), "NetConnectivity6.%s.%s.%dms.%s", - behind_nat ? "NAT" : "NoNAT", - is_src_port_shared ? "SrcPortShared" : "SrcPortUnique", - interval_ms, suffix.c_str()); - return std::string(output); +const char* PrintNatType(stunprober::NatType type) { + switch (type) { + case stunprober::NATTYPE_NONE: + return "Not behind a NAT"; + case stunprober::NATTYPE_UNKNOWN: + return "Unknown NAT type"; + case stunprober::NATTYPE_SYMMETRIC: + return "Symmetric NAT"; + case stunprober::NATTYPE_NON_SYMMETRIC: + return "Non-Symmetric NAT"; + default: + return "Invalid"; + } } void PrintStats(StunProber* prober) { @@ -130,27 +133,15 @@ void PrintStats(StunProber* prober) { LOG(LS_INFO) << "Responses received: " << stats.num_response_received; LOG(LS_INFO) << "Target interval (ns): " << stats.target_request_interval_ns; LOG(LS_INFO) << "Actual interval (ns): " << stats.actual_request_interval_ns; - LOG(LS_INFO) << "Behind NAT: " << stats.behind_nat; - if (stats.behind_nat) { - LOG(LS_INFO) << "NAT is symmetrical: " << (stats.srflx_addrs.size() > 1); - } + LOG(LS_INFO) << "NAT Type: " << PrintNatType(stats.nat_type); LOG(LS_INFO) << "Host IP: " << stats.host_ip; LOG(LS_INFO) << "Server-reflexive ips: "; for (auto& ip : stats.srflx_addrs) { LOG(LS_INFO) << "\t" << ip; } - std::string histogram_name = HistogramName( - stats.behind_nat, FLAG_shared_socket, FLAG_interval, "SuccessPercent"); - - LOG(LS_INFO) << "Histogram '" << histogram_name.c_str() - << "' = " << stats.success_percent; - - histogram_name = HistogramName(stats.behind_nat, FLAG_shared_socket, - FLAG_interval, "ResponseLatency"); - - LOG(LS_INFO) << "Histogram '" << histogram_name.c_str() - << "' = " << stats.average_rtt_ms << " ms"; + LOG(LS_INFO) << "Success Precent: " << stats.success_percent; + LOG(LS_INFO) << "Response Latency:" << stats.average_rtt_ms; } void StopTrial(rtc::Thread* thread, StunProber* prober, int result) { diff --git a/webrtc/p2p/stunprober/stunprober.cc b/webrtc/p2p/stunprober/stunprober.cc index ffa66fade..8efe069b0 100644 --- a/webrtc/p2p/stunprober/stunprober.cc +++ b/webrtc/p2p/stunprober/stunprober.cc @@ -24,11 +24,15 @@ namespace stunprober { namespace { -void IncrementCounterByAddress(std::map* counter_per_ip, - const rtc::IPAddress& ip) { +template +void IncrementCounterByAddress(std::map* counter_per_ip, const T& ip) { counter_per_ip->insert(std::make_pair(ip, 0)).first->second++; } +bool behind_nat(NatType nat_type) { + return nat_type > stunprober::NATTYPE_NONE; +} + } // namespace // A requester tracks the requests and responses from a single socket to many @@ -418,7 +422,7 @@ void StunProber::MaybeScheduleStunRequests() { rtc::Bind(&StunProber::MaybeScheduleStunRequests, this), 1 /* ms */); } -bool StunProber::GetStats(StunProber::Stats* prob_stats) { +bool StunProber::GetStats(StunProber::Stats* prob_stats) const { // No need to be on the same thread. if (!prob_stats) { return false; @@ -427,25 +431,26 @@ bool StunProber::GetStats(StunProber::Stats* prob_stats) { StunProber::Stats stats; int rtt_sum = 0; - bool behind_nat_set = false; int64 first_sent_time = 0; int64 last_sent_time = 0; + NatType nat_type = NATTYPE_INVALID; // Track of how many srflx IP that we have seen. std::set srflx_ips; // If we're not receiving any response on a given IP, all requests sent to // that IP should be ignored as this could just be an DNS error. - std::map num_response_per_ip; - std::map num_request_per_ip; + std::map num_response_per_server; + std::map num_request_per_server; for (auto* requester : requesters_) { + std::map num_response_per_srflx_addr; for (auto request : requester->requests()) { if (request->sent_time_ms <= 0) { continue; } - IncrementCounterByAddress(&num_request_per_ip, request->server_addr); + IncrementCounterByAddress(&num_request_per_server, request->server_addr); if (!first_sent_time) { first_sent_time = request->sent_time_ms; @@ -456,19 +461,26 @@ bool StunProber::GetStats(StunProber::Stats* prob_stats) { continue; } - IncrementCounterByAddress(&num_response_per_ip, request->server_addr); + IncrementCounterByAddress(&num_response_per_server, request->server_addr); + IncrementCounterByAddress(&num_response_per_srflx_addr, + request->srflx_addr); rtt_sum += request->rtt(); - if (!behind_nat_set) { - stats.behind_nat = request->behind_nat; - behind_nat_set = true; - } else if (stats.behind_nat != request->behind_nat) { + if (nat_type == NATTYPE_INVALID) { + nat_type = request->behind_nat ? NATTYPE_UNKNOWN : NATTYPE_NONE; + } else if (behind_nat(nat_type) != request->behind_nat) { // Detect the inconsistency in NAT presence. return false; } stats.srflx_addrs.insert(request->srflx_addr.ToString()); srflx_ips.insert(request->srflx_addr.ipaddr()); } + + // If we're using shared mode and seeing >1 srflx addresses for a single + // requester, it's symmetric NAT. + if (shared_socket_mode_ && num_response_per_srflx_addr.size() > 1) { + nat_type = NATTYPE_SYMMETRIC; + } } // We're probably not behind a regular NAT. We have more than 1 distinct @@ -481,11 +493,11 @@ bool StunProber::GetStats(StunProber::Stats* prob_stats) { int num_received = 0; int num_server_ip_with_response = 0; - for (const auto& kv : num_response_per_ip) { + for (const auto& kv : num_response_per_server) { DCHECK_GT(kv.second, 0); num_server_ip_with_response++; num_received += kv.second; - num_sent += num_request_per_ip[kv.first]; + num_sent += num_request_per_server[kv.first]; } // Not receiving any response, the trial is inconclusive. @@ -493,17 +505,21 @@ bool StunProber::GetStats(StunProber::Stats* prob_stats) { return false; } + stats.nat_type = nat_type; + // Shared mode is only true if we use the shared socket and there are more // than 1 responding servers. stats.shared_socket_mode = shared_socket_mode_ && (num_server_ip_with_response > 1); + if (stats.shared_socket_mode && nat_type == NATTYPE_UNKNOWN) { + stats.nat_type = NATTYPE_NON_SYMMETRIC; + } + stats.host_ip = local_addr_.ToString(); stats.num_request_sent = num_sent; stats.num_response_received = num_received; stats.target_request_interval_ns = interval_ms_ * 1000; - stats.symmetric_nat = - stats.srflx_addrs.size() > static_cast(GetTotalServerSockets()); if (num_sent) { stats.success_percent = static_cast(100 * num_received / num_sent); diff --git a/webrtc/p2p/stunprober/stunprober.h b/webrtc/p2p/stunprober/stunprober.h index 9352e36db..d773d5760 100644 --- a/webrtc/p2p/stunprober/stunprober.h +++ b/webrtc/p2p/stunprober/stunprober.h @@ -30,6 +30,14 @@ static const int kMaxUdpBufferSize = 1200; typedef rtc::Callback1 AsyncCallback; +enum NatType { + NATTYPE_INVALID, + NATTYPE_NONE, // Not behind a NAT. + NATTYPE_UNKNOWN, // Behind a NAT but type can't be determine. + NATTYPE_SYMMETRIC, // Behind a symmetric NAT. + NATTYPE_NON_SYMMETRIC // Behind a non-symmetric NAT. +}; + class HostNameResolverInterface { public: HostNameResolverInterface() {} @@ -139,10 +147,10 @@ class StunProber { struct Stats { Stats() {} + int num_request_sent = 0; int num_response_received = 0; - bool behind_nat = false; - bool symmetric_nat = false; + NatType nat_type = NATTYPE_INVALID; int average_rtt_ms = -1; int success_percent = 0; int target_request_interval_ns = 0; @@ -188,7 +196,7 @@ class StunProber { // Method to retrieve the Stats once |finish_callback| is invoked. Returning // false when the result is inconclusive, for example, whether it's behind a // NAT or not. - bool GetStats(Stats* stats); + bool GetStats(Stats* stats) const; private: // A requester tracks the requests and responses from a single socket to many diff --git a/webrtc/p2p/stunprober/stunprober_unittest.cc b/webrtc/p2p/stunprober/stunprober_unittest.cc index 92544a6c2..ffd5040e5 100644 --- a/webrtc/p2p/stunprober/stunprober_unittest.cc +++ b/webrtc/p2p/stunprober/stunprober_unittest.cc @@ -168,7 +168,7 @@ class StunProberTest : public testing::Test { EXPECT_EQ(ss_->num_socket(), total_sockets); EXPECT_TRUE(prober->GetStats(&stats)); EXPECT_EQ(stats.success_percent, 100); - EXPECT_TRUE(stats.behind_nat); + EXPECT_TRUE(stats.nat_type > stunprober::NATTYPE_NONE); EXPECT_EQ(stats.host_ip, kLocalAddr.ipaddr().ToString()); EXPECT_EQ(stats.srflx_addrs, srflx_addresses); EXPECT_EQ(static_cast(stats.num_request_sent),