Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(99)

Unified Diff: webrtc/p2p/base/port.cc

Issue 2163403002: Prepare for ICE-renomination (Closed) Base URL: https://chromium.googlesource.com/external/webrtc@master
Patch Set: . Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: webrtc/p2p/base/port.cc
diff --git a/webrtc/p2p/base/port.cc b/webrtc/p2p/base/port.cc
index b6173ebf45768be331c589631f81f9aee8ccbc80..f8aaee0e524570d39140560e09f6d8f93f738404 100644
--- a/webrtc/p2p/base/port.cc
+++ b/webrtc/p2p/base/port.cc
@@ -754,15 +754,20 @@ class ConnectionRequest : public StunRequest {
if (connection_->port()->GetIceRole() == ICEROLE_CONTROLLING) {
request->AddAttribute(new StunUInt64Attribute(
STUN_ATTR_ICE_CONTROLLING, connection_->port()->IceTiebreaker()));
- // Since we are trying aggressive nomination, sending USE-CANDIDATE
- // attribute in every ping.
- // If we are dealing with a ice-lite end point, nomination flag
- // in Connection will be set to false by default. Once the connection
- // becomes "best connection", nomination flag will be turned on.
+ // We should really have either USE_CANDIDATE attribute or
+ // ICE_NOMINATION_VALUE attribute but not both. That was enforced in
+ // p2ptransportchannel.
+ // Note: This will keep on sending USE_CANDIDATE_ATTR on the selected
+ // connection if peer does not support re-nomination.
if (connection_->use_candidate_attr()) {
request->AddAttribute(new StunByteStringAttribute(
STUN_ATTR_USE_CANDIDATE));
}
+ if (connection_->nominating_value() > 0 &&
+ !connection_->nomination_acknowledged()) {
+ request->AddAttribute(new StunUInt32Attribute(
+ STUN_ATTR_NOMINATION_VALUE, connection_->nominating_value()));
+ }
} else if (connection_->port()->GetIceRole() == ICEROLE_CONTROLLED) {
request->AddAttribute(new StunUInt64Attribute(
STUN_ATTR_ICE_CONTROLLED, connection_->port()->IceTiebreaker()));
@@ -830,7 +835,6 @@ Connection::Connection(Port* port,
connected_(true),
pruned_(false),
use_candidate_attr_(false),
- nominated_(false),
remote_ice_mode_(ICEMODE_FULL),
requests_(port->thread()),
rtt_(DEFAULT_RTT),
@@ -1041,10 +1045,23 @@ void Connection::HandleBindingRequest(IceMessage* msg) {
}
if (port_->GetIceRole() == ICEROLE_CONTROLLED) {
- const StunByteStringAttribute* use_candidate_attr =
- msg->GetByteString(STUN_ATTR_USE_CANDIDATE);
- if (use_candidate_attr) {
- set_nominated(true);
+ const StunUInt32Attribute* nomination_value_attr =
+ msg->GetUInt32(STUN_ATTR_NOMINATION_VALUE);
+ int nomination_value = 0;
+ if (nomination_value_attr) {
+ nomination_value = static_cast<int>(nomination_value_attr->value());
+ if (nomination_value < 0) {
+ LOG(LS_ERROR) << "Negative nomination value: " << nomination_value;
+ }
+ } else {
+ const StunByteStringAttribute* use_candidate_attr =
+ msg->GetByteString(STUN_ATTR_USE_CANDIDATE);
+ if (use_candidate_attr) {
+ nomination_value = 1;
+ }
+ }
+ if (nomination_value > 0) {
+ set_nominated_value(nomination_value);
SignalNominated(this);
}
}
@@ -1178,9 +1195,11 @@ void Connection::UpdateState(int64_t now) {
void Connection::Ping(int64_t now) {
last_ping_sent_ = now;
ConnectionRequest *req = new ConnectionRequest(this);
- pings_since_last_response_.push_back(SentPing(req->id(), now));
+ pings_since_last_response_.push_back(
+ SentPing(req->id(), now, nominating_value_));
LOG_J(LS_VERBOSE, this) << "Sending STUN ping "
- << ", id=" << rtc::hex_encode(req->id());
+ << ", id=" << rtc::hex_encode(req->id())
+ << ", nominating_value=" << nominating_value_;
requests_.Send(req);
state_ = STATE_INPROGRESS;
num_pings_sent_++;
@@ -1191,17 +1210,26 @@ void Connection::ReceivedPing() {
UpdateReceiving(last_ping_received_);
}
-void Connection::ReceivedPingResponse(int rtt) {
+void Connection::ReceivedPingResponse(int rtt, const std::string& request_id) {
// We've already validated that this is a STUN binding response with
// the correct local and remote username for this connection.
// So if we're not already, become writable. We may be bringing a pruned
// connection back to life, but if we don't really want it, we can always
// prune it again.
+ if (!nomination_acknowledged_) {
+ auto iter = std::find_if(
+ pings_since_last_response_.begin(), pings_since_last_response_.end(),
+ [request_id](const SentPing& ping) { return ping.id == request_id; });
+ if (iter != pings_since_last_response_.end() &&
+ iter->nomination_value == nominating_value_) {
+ nomination_acknowledged_ = true;
+ }
+ }
+ pings_since_last_response_.clear();
last_ping_response_received_ = rtc::TimeMillis();
UpdateReceiving(last_ping_response_received_);
set_write_state(STATE_WRITABLE);
set_state(STATE_SUCCEEDED);
- pings_since_last_response_.clear();
rtt_samples_++;
rtt_ = (RTT_RATIO * rtt_ + rtt) / (RTT_RATIO + 1);
}
@@ -1274,21 +1302,16 @@ std::string Connection::ToString() const {
const Candidate& local = local_candidate();
const Candidate& remote = remote_candidate();
std::stringstream ss;
- ss << "Conn[" << ToDebugId()
- << ":" << port_->content_name()
- << ":" << local.id() << ":" << local.component()
- << ":" << local.generation()
- << ":" << local.type() << ":" << local.protocol()
- << ":" << local.address().ToSensitiveString()
- << "->" << remote.id() << ":" << remote.component()
- << ":" << remote.priority()
- << ":" << remote.type() << ":"
- << remote.protocol() << ":" << remote.address().ToSensitiveString() << "|"
- << CONNECT_STATE_ABBREV[connected()]
- << RECEIVE_STATE_ABBREV[receiving()]
- << WRITE_STATE_ABBREV[write_state()]
- << ICESTATE[state()] << "|"
- << priority() << "|";
+ ss << "Conn[" << ToDebugId() << ":" << port_->content_name() << ":"
+ << local.id() << ":" << local.component() << ":" << local.generation()
+ << ":" << local.type() << ":" << local.protocol() << ":"
+ << local.address().ToSensitiveString() << "->" << remote.id() << ":"
+ << remote.component() << ":" << remote.priority() << ":" << remote.type()
+ << ":" << remote.protocol() << ":" << remote.address().ToSensitiveString()
+ << "|" << CONNECT_STATE_ABBREV[connected()]
+ << RECEIVE_STATE_ABBREV[receiving()] << WRITE_STATE_ABBREV[write_state()]
+ << ICESTATE[state()] << "|" << nominated_value() << "|" << priority()
Taylor Brandstetter 2016/07/22 17:36:01 It seems like nominating_value would also be usefu
honghaiz3 2016/07/22 22:50:11 nominating_value_ could change dynamically. Basic
Taylor Brandstetter 2016/07/22 23:48:28 I don't see where nominating_value_ is set to 0. A
honghaiz3 2016/07/25 22:24:40 Hope this is clearer now with the updated CL.
Taylor Brandstetter 2016/07/27 22:20:55 I see that in the updated CL, the nominating value
+ << "|";
if (rtt_ < DEFAULT_RTT) {
ss << rtt_ << "]";
} else {
@@ -1309,20 +1332,16 @@ void Connection::OnConnectionRequestResponse(ConnectionRequest* request,
int rtt = request->Elapsed();
- ReceivedPingResponse(rtt);
-
if (LOG_CHECK_LEVEL_V(sev)) {
- bool use_candidate = (
- response->GetByteString(STUN_ATTR_USE_CANDIDATE) != nullptr);
std::string pings;
PrintPingsSinceLastResponse(&pings, 5);
LOG_JV(sev, this) << "Received STUN ping response"
<< ", id=" << rtc::hex_encode(request->id())
<< ", code=0" // Makes logging easier to parse.
<< ", rtt=" << rtt
- << ", use_candidate=" << use_candidate
Taylor Brandstetter 2016/07/22 17:36:01 Can we log both use_candidate and nominating_value
honghaiz3 2016/07/22 22:50:11 This is in the PingResponse. We don't send use_ca
Taylor Brandstetter 2016/07/22 23:48:28 Acknowledged.
honghaiz3 2016/07/25 22:24:40 Acknowledged.
<< ", pings_since_last_response=" << pings;
}
+ ReceivedPingResponse(rtt, request->id());
stats_.recv_ping_responses++;
@@ -1369,10 +1388,10 @@ void Connection::OnConnectionRequestTimeout(ConnectionRequest* request) {
void Connection::OnConnectionRequestSent(ConnectionRequest* request) {
// Log at LS_INFO if we send a ping on an unwritable connection.
rtc::LoggingSeverity sev = !writable() ? rtc::LS_INFO : rtc::LS_VERBOSE;
- bool use_candidate = use_candidate_attr();
LOG_JV(sev, this) << "Sent STUN ping"
<< ", id=" << rtc::hex_encode(request->id())
- << ", use_candidate=" << use_candidate;
+ << ", use_candidate=" << use_candidate_attr()
+ << ", nominating value=" << nominating_value();
stats_.sent_ping_requests_total++;
if (stats_.recv_ping_responses == 0) {
stats_.sent_ping_requests_before_first_response++;
« webrtc/p2p/base/port.h ('K') | « webrtc/p2p/base/port.h ('k') | webrtc/p2p/base/stun.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698