admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert of Prune connections based on network name. (patchset #3 id:130001 of https://codereview.webrtc.org/2395243005/ )

Browse Source 
Reason for revert:
Breaks upstream code.

Original issue's description:
> Prune connections based on network name.
> Previously we prune connections on the same network pointer.
> So if an IPv6 and an IPv4 network are on the same network interface, IPv4 connection won't be pruned even if an IPv6 connection with higher priority becomes writable.
>
> With this change, as long as one connection becomes writable, all connections  having lower priority with the same network name will be pruned.
>
> Also simplify the implementation.
>
> BUG=webrtc:6512
>
> Committed: https://crrev.com/aae2784c1fab9d1510393dec15d76caa574e2da8
> Cr-Commit-Position: refs/heads/master@{#14593}

TBR=skvlad@webrtc.org,honghaiz@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:6512

Review-Url: https://codereview.webrtc.org/2412433003
Cr-Commit-Position: refs/heads/master@{#14601}
This commit is contained in:
sprang 2016-10-11 06:43:28 -07:00 committed by Commit bot
parent e89141500a
commit 716978d075
4 changed files with 76 additions and 107 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
webrtc/p2p/base/fakeportallocator.h
View File

@ -94,8 +94,7 @@ class FakePortAllocatorSession : public PortAllocatorSession {
const std::string& content_name,
int component,
const std::string& ice_ufrag,
const std::string& ice_pwd,
bool ipv6_enabled)
const std::string& ice_pwd)
: PortAllocatorSession(content_name,
component,
ice_ufrag,
@ -111,10 +110,10 @@ class FakePortAllocatorSession : public PortAllocatorSession {
"unittest",
rtc::IPAddress(in6addr_loopback),
64),
port_(),
port_config_count_(0),
stun_servers_(allocator->stun_servers()),
turn_servers_(allocator->turn_servers()),
ipv6_enabled_(ipv6_enabled) {
turn_servers_(allocator->turn_servers()) {
ipv4_network_.AddIP(rtc::IPAddress(INADDR_LOOPBACK));
ipv6_network_.AddIP(rtc::IPAddress(in6addr_loopback));
}
@ -123,20 +122,18 @@ class FakePortAllocatorSession : public PortAllocatorSession {
candidate_filter_ = filter;
}
Port* CreatePort(rtc::Network* network) {
Port* port = TestUDPPort::Create(network_thread_, factory_, network,
network->GetBestIP(), 0, 0, username(),
password(), std::string(), false);
AddPort(port);
return port;
}
void StartGettingPorts() override {
if (!ipv4_port_) {
ipv4_port_.reset(CreatePort(&ipv4_network_));
}
if (!ipv6_port_ && ipv6_enabled_ && (flags() & PORTALLOCATOR_ENABLE_IPV6)) {
ipv6_port_.reset(CreatePort(&ipv6_network_));
if (!port_) {
rtc::Network& network =
(rtc::HasIPv6Enabled() && (flags() & PORTALLOCATOR_ENABLE_IPV6))
? ipv6_network_
: ipv4_network_;
port_.reset(TestUDPPort::Create(network_thread_, factory_, &network,
network.GetBestIP(), 0, 0, username(),
password(), std::string(), false));
port_->SignalDestroyed.connect(
this, &FakePortAllocatorSession::OnPortDestroyed);
AddPort(port_.get());
}
++port_config_count_;
running_ = true;
@ -152,14 +149,7 @@ class FakePortAllocatorSession : public PortAllocatorSession {
std::vector<Candidate> ReadyCandidates() const override {
return candidates_;
}
void PruneAllPorts() override {
if (ipv4_port_) {
ipv4_port_->Prune();
}
if (ipv6_port_) {
ipv6_port_->Prune();
}
}
void PruneAllPorts() override { port_->Prune(); }
bool CandidatesAllocationDone() const override { return allocation_done_; }
int port_config_count() { return port_config_count_; }
@ -189,8 +179,6 @@ class FakePortAllocatorSession : public PortAllocatorSession {
port->set_generation(generation());
port->SignalPortComplete.connect(this,
&FakePortAllocatorSession::OnPortComplete);
port->SignalDestroyed.connect(this,
&FakePortAllocatorSession::OnPortDestroyed);
port->PrepareAddress();
ready_ports_.push_back(port);
SignalPortReady(this, port);
@ -206,19 +194,14 @@ class FakePortAllocatorSession : public PortAllocatorSession {
}
void OnPortDestroyed(cricket::PortInterface* port) {
// Don't want to double-delete port if it deletes itself.
if (port == ipv4_port_.get()) {
ipv4_port_.release();
} else if (port == ipv6_port_.get()) {
ipv6_port_.release();
}
port_.release();
}
rtc::Thread* network_thread_;
rtc::PacketSocketFactory* factory_;
rtc::Network ipv4_network_;
rtc::Network ipv6_network_;
std::unique_ptr<cricket::Port> ipv4_port_;
std::unique_ptr<cricket::Port> ipv6_port_;
std::unique_ptr<cricket::Port> port_;
int port_config_count_;
std::vector<Candidate> candidates_;
std::vector<PortInterface*> ready_ports_;
@ -227,7 +210,6 @@ class FakePortAllocatorSession : public PortAllocatorSession {
std::vector<RelayServerConfig> turn_servers_;
uint32_t candidate_filter_ = CF_ALL;
int transport_info_update_count_ = 0;
bool ipv6_enabled_;
bool running_ = false;
};
@ -240,7 +222,6 @@ class FakePortAllocator : public cricket::PortAllocator {
owned_factory_.reset(new rtc::BasicPacketSocketFactory(network_thread_));
factory_ = owned_factory_.get();
}
ipv6_enabled_ = rtc::HasIPv6Enabled();
}
void Initialize() override {
@ -251,10 +232,6 @@ class FakePortAllocator : public cricket::PortAllocator {
void SetNetworkIgnoreMask(int network_ignore_mask) override {}
// Sometimes we can ignore the value returned by rtc::HasIpv6Enabled because
// we are using the virtual socket server.
void set_ipv6_enabled(bool ipv6_enabled) { ipv6_enabled_ = ipv6_enabled; }
cricket::PortAllocatorSession* CreateSessionInternal(
const std::string& content_name,
int component,
@ -262,7 +239,7 @@ class FakePortAllocator : public cricket::PortAllocator {
const std::string& ice_pwd) override {
return new FakePortAllocatorSession(this, network_thread_, factory_,
content_name, component, ice_ufrag,
ice_pwd, ipv6_enabled_);
ice_pwd);
}
bool initialized() const { return initialized_; }
@ -272,7 +249,6 @@ class FakePortAllocator : public cricket::PortAllocator {
rtc::PacketSocketFactory* factory_;
std::unique_ptr<rtc::BasicPacketSocketFactory> owned_factory_;
bool initialized_ = false;
bool ipv6_enabled_ = false;
};
} // namespace cricket

73
webrtc/p2p/base/p2ptransportchannel.cc
View File

@ -1306,35 +1306,34 @@ void P2PTransportChannel::SortConnectionsAndUpdateState() {
}
void P2PTransportChannel::PruneConnections() {
// We can prune any connection for which there is a connected, writable,
// and receiving connection with the same network name with better or equal
// priority. We leave those with better priority just in case they become
// writable later (at which point, we would prune out the current selected
// connection). We leave connections on other networks because they may not
// be using the same resources and they may represent very distinct paths
// over which we can switch. If the |premier| connection is not connected,
// we may be reconnecting a TCP connection and temporarily do not prune
// connections in this network. See the big comment in
// CompareConnectionStates.
// We can prune any connection for which there is a connected, writable
// connection on the same network with better or equal priority. We leave
// those with better priority just in case they become writable later (at
// which point, we would prune out the current selected connection). We leave
// connections on other networks because they may not be using the same
// resources and they may represent very distinct paths over which we can
// switch. If the |premier| connection is not connected, we may be
// reconnecting a TCP connection and temporarily do not prune connections in
// this network. See the big comment in CompareConnectionStates.
std::map<std::string, Connection*> premier_connection_by_network_name;
if (selected_connection_) {
// |selected_connection_| is always a premier connection.
const std::string& network_name =
selected_connection_->port()->Network()->name();
premier_connection_by_network_name[network_name] = selected_connection_;
// Get a list of the networks that we are using.
std::set<rtc::Network*> networks;
for (const Connection* conn : connections_) {
networks.insert(conn->port()->Network());
}
for (Connection* conn : connections_) {
const std::string& network_name = conn->port()->Network()->name();
Connection* premier = premier_connection_by_network_name[network_name];
// Since the connections are sorted, the first one with a given network name
// is the premier connection for the network name.
// |premier| might be equal to |conn| if this is the selected connection.
if (premier == nullptr) {
premier_connection_by_network_name[network_name] = conn;
} else if (premier != conn && !premier->weak() &&
CompareConnectionCandidates(premier, conn) >= 0) {
conn->Prune();
for (rtc::Network* network : networks) {
Connection* premier = GetBestConnectionOnNetwork(network);
// Do not prune connections if the current selected connection is weak on
// this network. Otherwise, it may delete connections prematurely.
if (!premier || premier->weak()) {
continue;
}
for (Connection* conn : connections_) {
if ((conn != premier) && (conn->port()->Network() == network) &&
(CompareConnectionCandidates(premier, conn) >= 0)) {
conn->Prune();
}
}
}
}
@ -1472,6 +1471,26 @@ bool P2PTransportChannel::ReadyToSend(Connection* connection) const {
PresumedWritable(connection));
}
// If we have a selected connection, return it, otherwise return top one in the
// list (later we will mark it best).
Connection* P2PTransportChannel::GetBestConnectionOnNetwork(
rtc::Network* network) const {
// If the selected connection is on this network, then it wins.
if (selected_connection_ &&
(selected_connection_->port()->Network() == network)) {
return selected_connection_;
}
// Otherwise, we return the top-most in sorted order.
for (size_t i = 0; i < connections_.size(); ++i) {
if (connections_[i]->port()->Network() == network) {
return connections_[i];
}
}
return NULL;
}
// Handle any queued up requests
void P2PTransportChannel::OnMessage(rtc::Message *pmsg) {
switch (pmsg->message_id) {

1
webrtc/p2p/base/p2ptransportchannel.h
View File

@ -248,6 +248,7 @@ class P2PTransportChannel : public TransportChannelImpl,
void MaybeStopPortAllocatorSessions();
TransportChannelState ComputeState() const;
Connection* GetBestConnectionOnNetwork(rtc::Network* network) const;
bool CreateConnections(const Candidate& remote_candidate,
PortInterface* origin_port);
bool CreateConnection(PortInterface* port,

49
webrtc/p2p/base/p2ptransportchannel_unittest.cc
View File

@ -2813,7 +2813,7 @@ class P2PTransportChannelPingTest : public testing::Test,
return GetConnectionTo(ch, ip, port_num);
}
Port* GetFirstPort(P2PTransportChannel* ch) {
Port* GetPort(P2PTransportChannel* ch) {
if (ch->ports().empty()) {
return nullptr;
}
@ -2830,13 +2830,11 @@ class P2PTransportChannelPingTest : public testing::Test,
Connection* GetConnectionTo(P2PTransportChannel* ch,
const std::string& ip,
int port_num) {
for (PortInterface* port : ch->ports()) {
Connection* conn = port->GetConnection(rtc::SocketAddress(ip, port_num));
if (conn != nullptr) {
return conn;
}
Port* port = GetPort(ch);
if (!port) {
return nullptr;
}
return nullptr;
return port->GetConnection(rtc::SocketAddress(ip, port_num));
}
Connection* FindNextPingableConnectionAndPingIt(P2PTransportChannel* ch) {
@ -3093,7 +3091,7 @@ TEST_F(P2PTransportChannelPingTest, PingingStartedAsSoonAsPossible) {
uint32_t prflx_priority = ICE_TYPE_PREFERENCE_PRFLX << 24;
request.AddAttribute(
new StunUInt32Attribute(STUN_ATTR_PRIORITY, prflx_priority));
Port* port = GetFirstPort(&ch);
Port* port = GetPort(&ch);
ASSERT_NE(nullptr, port);
port->SignalUnknownAddress(port, rtc::SocketAddress("1.1.1.1", 1), PROTO_UDP,
&request, kIceUfrag[1], false);
@ -3262,7 +3260,7 @@ TEST_F(P2PTransportChannelPingTest, ConnectionResurrection) {
new StunUInt32Attribute(STUN_ATTR_PRIORITY, prflx_priority));
EXPECT_NE(prflx_priority, remote_priority);
Port* port = GetFirstPort(&ch);
Port* port = GetPort(&ch);
// conn1 should be resurrected with original priority.
port->SignalUnknownAddress(port, rtc::SocketAddress("1.1.1.1", 1), PROTO_UDP,
&request, kIceUfrag[1], false);
@ -3401,7 +3399,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {
uint32_t prflx_priority = ICE_TYPE_PREFERENCE_PRFLX << 24;
request.AddAttribute(
new StunUInt32Attribute(STUN_ATTR_PRIORITY, prflx_priority));
TestUDPPort* port = static_cast<TestUDPPort*>(GetFirstPort(&ch));
TestUDPPort* port = static_cast<TestUDPPort*>(GetPort(&ch));
port->SignalUnknownAddress(port, rtc::SocketAddress("1.1.1.1", 1), PROTO_UDP,
&request, kIceUfrag[1], false);
Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
@ -3499,7 +3497,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionBasedOnMediaReceived) {
request.AddAttribute(
new StunUInt32Attribute(STUN_ATTR_PRIORITY, prflx_priority));
request.AddAttribute(new StunByteStringAttribute(STUN_ATTR_USE_CANDIDATE));
Port* port = GetFirstPort(&ch);
Port* port = GetPort(&ch);
port->SignalUnknownAddress(port, rtc::SocketAddress("3.3.3.3", 3), PROTO_UDP,
&request, kIceUfrag[1], false);
Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
@ -3874,31 +3872,6 @@ TEST_F(P2PTransportChannelPingTest, TestConnectionPrunedAgain) {
EXPECT_EQ(TransportChannelState::STATE_COMPLETED, ch.GetState());
}
TEST_F(P2PTransportChannelPingTest, TestPruneConnectionsByNetworkName) {
std::string ipv4_addr("1.1.1.1");
std::string ipv6_addr("2400:1:2:3:4:5:6:7");
FakePortAllocator pa(rtc::Thread::Current(), nullptr);
pa.set_ipv6_enabled(true);
pa.set_flags(PORTALLOCATOR_ENABLE_IPV6);
P2PTransportChannel ch("test channel", 1, &pa);
PrepareChannel(&ch);
ch.MaybeStartGathering();
ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, ipv4_addr, 1, 100));
ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, ipv6_addr, 1, 100));
Connection* conn1 = WaitForConnectionTo(&ch, ipv4_addr, 1);
ASSERT_TRUE(conn1 != nullptr);
Connection* conn2 = WaitForConnectionTo(&ch, ipv6_addr, 1);
ASSERT_TRUE(conn2 != nullptr);
conn1->ReceivedPingResponse(LOW_RTT, "id");
EXPECT_EQ_WAIT(conn1, ch.selected_connection(), kDefaultTimeout);
conn2->ReceivedPingResponse(LOW_RTT, "id");
// IPv6 connection has higher priority.
EXPECT_EQ_WAIT(conn2, ch.selected_connection(), kDefaultTimeout);
// Since conn1 and conn2 are on networks with the same network name,
// conn1 will be pruned when conn2 becomes writable and receiving.
EXPECT_FALSE(conn1->writable());
}
// Test that if all connections in a channel has timed out on writing, they
// will all be deleted. We use Prune to simulate write_time_out.
TEST_F(P2PTransportChannelPingTest, TestDeleteConnectionsIfAllWriteTimedout) {
@ -4033,12 +4006,12 @@ TEST_F(P2PTransportChannelPingTest, TestPortDestroyedAfterTimeoutAndPruned) {
}
EXPECT_EQ(nullptr, GetConnectionTo(&ch, "1.1.1.1", 1));
// Port will not be removed because it is not pruned yet.
PortInterface* port = GetFirstPort(&ch);
PortInterface* port = GetPort(&ch);
ASSERT_NE(nullptr, port);
// If the session prunes all ports, the port will be destroyed.
ch.allocator_session()->PruneAllPorts();
EXPECT_EQ_SIMULATED_WAIT(nullptr, GetFirstPort(&ch), 1, fake_clock);
EXPECT_EQ_SIMULATED_WAIT(nullptr, GetPort(&ch), 1, fake_clock);
EXPECT_EQ_SIMULATED_WAIT(nullptr, GetPrunedPort(&ch), 1, fake_clock);
}