zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Move SendBindingResponse to Connection

Browse Source 
This patch moves the SendBindingResponse from Port
to Connection. This is a behavioural NOP, and I don't
understand why it was in Port in the firs place!

Found when working on GOOG_PING.

BUG=webrtc:11100

Change-Id: I0466c5381f08ec4926ca3380e6914f0bc0dfcf63
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/161081
Commit-Queue: Jonas Oreland <jonaso@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29963}
This commit is contained in:
Jonas Oreland
2019-11-29 15:58:13 +01:00
committed by Commit Bot
parent 89313451d8
commit d003662b15
8 changed files with 90 additions and 109 deletions
Download Patch File Download Diff File Expand all files Collapse all files

68
p2p/base/connection.cc
View File

@ -536,7 +536,7 @@ void Connection::HandleBindingRequest(IceMessage* msg) {
msg->reduced_transaction_id());
// This is a validated stun request from remote peer.
port_->SendBindingResponse(msg, remote_addr);
SendBindingResponse(msg);
// If it timed out on writing check, start up again
if (!pruned_ && write_state_ == STATE_WRITE_TIMEOUT) {
@ -587,6 +587,72 @@ void Connection::HandleBindingRequest(IceMessage* msg) {
}
}
void Connection::SendBindingResponse(const StunMessage* request) {
RTC_DCHECK(request->type() == STUN_BINDING_REQUEST);
// Where I send the response.
const rtc::SocketAddress& addr = remote_candidate_.address();
// Retrieve the username from the request.
const StunByteStringAttribute* username_attr =
request->GetByteString(STUN_ATTR_USERNAME);
RTC_DCHECK(username_attr != NULL);
if (username_attr == NULL) {
// No valid username, skip the response.
return;
}
// Fill in the response message.
StunMessage response;
response.SetType(STUN_BINDING_RESPONSE);
response.SetTransactionID(request->transaction_id());
const StunUInt32Attribute* retransmit_attr =
request->GetUInt32(STUN_ATTR_RETRANSMIT_COUNT);
if (retransmit_attr) {
// Inherit the incoming retransmit value in the response so the other side
// can see our view of lost pings.
response.AddAttribute(std::make_unique<StunUInt32Attribute>(
STUN_ATTR_RETRANSMIT_COUNT, retransmit_attr->value()));
if (retransmit_attr->value() > CONNECTION_WRITE_CONNECT_FAILURES) {
RTC_LOG(LS_INFO)
<< ToString()
<< ": Received a remote ping with high retransmit count: "
<< retransmit_attr->value();
}
}
response.AddAttribute(std::make_unique<StunXorAddressAttribute>(
STUN_ATTR_XOR_MAPPED_ADDRESS, addr));
response.AddMessageIntegrity(local_candidate().password());
response.AddFingerprint();
// Send the response message.
rtc::ByteBufferWriter buf;
response.Write(&buf);
rtc::PacketOptions options(port_->StunDscpValue());
options.info_signaled_after_sent.packet_type =
rtc::PacketType::kIceConnectivityCheckResponse;
auto err = port_->SendTo(buf.Data(), buf.Length(), addr, options, false);
if (err < 0) {
RTC_LOG(LS_ERROR) << ToString()
<< ": Failed to send STUN ping response, to="
<< addr.ToSensitiveString() << ", err=" << err
<< ", id=" << rtc::hex_encode(response.transaction_id());
} else {
// Log at LS_INFO if we send a stun ping response on an unwritable
// connection.
rtc::LoggingSeverity sev = (!writable()) ? rtc::LS_INFO : rtc::LS_VERBOSE;
RTC_LOG_V(sev) << ToString() << ": Sent STUN ping response, to="
<< addr.ToSensitiveString()
<< ", id=" << rtc::hex_encode(response.transaction_id());
stats_.sent_ping_responses++;
LogCandidatePairEvent(webrtc::IceCandidatePairEventType::kCheckResponseSent,
request->reduced_transaction_id());
}
}
void Connection::OnReadyToSend() {
SignalReadyToSend(this);
}

23
p2p/base/connection.h
View File

@ -184,13 +184,6 @@ class Connection : public CandidatePairInterface,
// a nomination value. The controlling agent gets its |acked_nomination_| set
// when receiving a response to a nominating ping.
bool nominated() const { return acked_nomination_ || remote_nomination_; }
// Public for unit tests.
void set_remote_nomination(uint32_t remote_nomination) {
remote_nomination_ = remote_nomination;
}
// Public for unit tests.
uint32_t acked_nomination() const { return acked_nomination_; }
void set_remote_ice_mode(IceMode mode) { remote_ice_mode_ = mode; }
int receiving_timeout() const;
@ -300,13 +293,23 @@ class Connection : public CandidatePairInterface,
// Check if we sent |val| pings without receving a response.
bool TooManyOutstandingPings(const absl::optional<int>& val) const;
void SetIceFieldTrials(const IceFieldTrials* field_trials);
const rtc::EventBasedExponentialMovingAverage& GetRttEstimate() const {
return rtt_estimate_;
}
void SendBindingResponse(const StunMessage* request);
// An accessor for unit tests.
Port* PortForTest() { return port_; }
const Port* PortForTest() const { return port_; }
void SetIceFieldTrials(const IceFieldTrials* field_trials);
const rtc::EventBasedExponentialMovingAverage& GetRttEstimate() const {
return rtt_estimate_;
// Public for unit tests.
uint32_t acked_nomination() const { return acked_nomination_; }
// Public for unit tests.
void set_remote_nomination(uint32_t remote_nomination) {
remote_nomination_ = remote_nomination;
}
protected:

11
p2p/base/fake_port_allocator.h
View File

@ -48,15 +48,6 @@ class TestUDPPort : public UDPPort {
}
return port;
}
void SendBindingResponse(StunMessage* request,
const rtc::SocketAddress& addr) override {
UDPPort::SendBindingResponse(request, addr);
sent_binding_response_ = true;
}
bool sent_binding_response() { return sent_binding_response_; }
void set_sent_binding_response(bool response) {
sent_binding_response_ = response;
}
protected:
TestUDPPort(rtc::Thread* thread,
@ -77,8 +68,6 @@ class TestUDPPort : public UDPPort {
password,
origin,
emit_localhost_for_anyaddress) {}
bool sent_binding_response_ = false;
};
// A FakePortAllocatorSession can be used with either a real or fake socket

12
p2p/base/p2p_transport_channel_unittest.cc
View File

@ -3808,11 +3808,10 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {
&request, kIceUfrag[1], false);
Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
ASSERT_TRUE(conn1 != nullptr);
EXPECT_TRUE(port->sent_binding_response());
EXPECT_EQ(conn1->stats().sent_ping_responses, 1u);
EXPECT_NE(conn1, ch.selected_connection());
conn1->ReceivedPingResponse(LOW_RTT, "id");
EXPECT_EQ_WAIT(conn1, ch.selected_connection(), kDefaultTimeout);
port->set_sent_binding_response(false);
// Another connection is nominated via use_candidate.
ch.AddRemoteCandidate(CreateUdpCandidate(LOCAL_PORT_TYPE, "2.2.2.2", 2, 1));
@ -3833,10 +3832,9 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {
&request, kIceUfrag[1], false);
Connection* conn3 = WaitForConnectionTo(&ch, "3.3.3.3", 3);
ASSERT_TRUE(conn3 != nullptr);
EXPECT_TRUE(port->sent_binding_response());
EXPECT_EQ(conn3->stats().sent_ping_responses, 1u);
conn3->ReceivedPingResponse(LOW_RTT, "id"); // Become writable.
EXPECT_EQ(conn2, ch.selected_connection());
port->set_sent_binding_response(false);
// However if the request contains use_candidate attribute, it will be
// selected as the selected connection.
@ -3846,7 +3844,7 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {
&request, kIceUfrag[1], false);
Connection* conn4 = WaitForConnectionTo(&ch, "4.4.4.4", 4);
ASSERT_TRUE(conn4 != nullptr);
EXPECT_TRUE(port->sent_binding_response());
EXPECT_EQ(conn4->stats().sent_ping_responses, 1u);
// conn4 is not the selected connection yet because it is not writable.
EXPECT_EQ(conn2, ch.selected_connection());
conn4->ReceivedPingResponse(LOW_RTT, "id"); // Become writable.
@ -3854,14 +3852,14 @@ TEST_F(P2PTransportChannelPingTest, TestSelectConnectionFromUnknownAddress) {
// Test that the request from an unknown address contains a ufrag from an old
// generation.
port->set_sent_binding_response(false);
// port->set_sent_binding_response(false);
ch.SetRemoteIceParameters(kIceParams[2]);
ch.SetRemoteIceParameters(kIceParams[3]);
port->SignalUnknownAddress(port, rtc::SocketAddress("5.5.5.5", 5), PROTO_UDP,
&request, kIceUfrag[2], false);
Connection* conn5 = WaitForConnectionTo(&ch, "5.5.5.5", 5);
ASSERT_TRUE(conn5 != nullptr);
EXPECT_TRUE(port->sent_binding_response());
EXPECT_EQ(conn5->stats().sent_ping_responses, 1u);
EXPECT_EQ(kIcePwd[2], conn5->remote_candidate().password());
}

67
p2p/base/port.cc
View File

@ -661,73 +661,6 @@ bool Port::CanHandleIncomingPacketsFrom(const rtc::SocketAddress&) const {
return false;
}
void Port::SendBindingResponse(StunMessage* request,
const rtc::SocketAddress& addr) {
RTC_DCHECK(request->type() == STUN_BINDING_REQUEST);
// Retrieve the username from the request.
const StunByteStringAttribute* username_attr =
request->GetByteString(STUN_ATTR_USERNAME);
RTC_DCHECK(username_attr != NULL);
if (username_attr == NULL) {
// No valid username, skip the response.
return;
}
// Fill in the response message.
StunMessage response;
response.SetType(STUN_BINDING_RESPONSE);
response.SetTransactionID(request->transaction_id());
const StunUInt32Attribute* retransmit_attr =
request->GetUInt32(STUN_ATTR_RETRANSMIT_COUNT);
if (retransmit_attr) {
// Inherit the incoming retransmit value in the response so the other side
// can see our view of lost pings.
response.AddAttribute(std::make_unique<StunUInt32Attribute>(
STUN_ATTR_RETRANSMIT_COUNT, retransmit_attr->value()));
if (retransmit_attr->value() > CONNECTION_WRITE_CONNECT_FAILURES) {
RTC_LOG(LS_INFO)
<< ToString()
<< ": Received a remote ping with high retransmit count: "
<< retransmit_attr->value();
}
}
response.AddAttribute(std::make_unique<StunXorAddressAttribute>(
STUN_ATTR_XOR_MAPPED_ADDRESS, addr));
response.AddMessageIntegrity(password_);
response.AddFingerprint();
// Send the response message.
rtc::ByteBufferWriter buf;
response.Write(&buf);
rtc::PacketOptions options(StunDscpValue());
options.info_signaled_after_sent.packet_type =
rtc::PacketType::kIceConnectivityCheckResponse;
auto err = SendTo(buf.Data(), buf.Length(), addr, options, false);
if (err < 0) {
RTC_LOG(LS_ERROR) << ToString()
<< ": Failed to send STUN ping response, to="
<< addr.ToSensitiveString() << ", err=" << err
<< ", id=" << rtc::hex_encode(response.transaction_id());
} else {
// Log at LS_INFO if we send a stun ping response on an unwritable
// connection.
Connection* conn = GetConnection(addr);
rtc::LoggingSeverity sev =
(conn && !conn->writable()) ? rtc::LS_INFO : rtc::LS_VERBOSE;
RTC_LOG_V(sev) << ToString() << ": Sent STUN ping response, to="
<< addr.ToSensitiveString()
<< ", id=" << rtc::hex_encode(response.transaction_id());
conn->stats_.sent_ping_responses++;
conn->LogCandidatePairEvent(
webrtc::IceCandidatePairEventType::kCheckResponseSent,
request->reduced_transaction_id());
}
}
void Port::SendBindingErrorResponse(StunMessage* request,
const rtc::SocketAddress& addr,
int error_code,

6
p2p/base/port.h
View File

@ -287,11 +287,7 @@ class Port : public PortInterface,
virtual bool CanHandleIncomingPacketsFrom(
const rtc::SocketAddress& remote_addr) const;
// Sends a response message (normal or error) to the given request. One of
// these methods should be called as a response to SignalUnknownAddress.
// NOTE: You MUST call CreateConnection BEFORE SendBindingResponse.
void SendBindingResponse(StunMessage* request,
const rtc::SocketAddress& addr) override;
// Sends a response error to the given request.
void SendBindingErrorResponse(StunMessage* request,
const rtc::SocketAddress& addr,
int error_code,

3
p2p/base/port_interface.h
View File

@ -105,9 +105,6 @@ class PortInterface {
// Sends a response message (normal or error) to the given request. One of
// these methods should be called as a response to SignalUnknownAddress.
// NOTE: You MUST call CreateConnection BEFORE SendBindingResponse.
virtual void SendBindingResponse(StunMessage* request,
const rtc::SocketAddress& addr) = 0;
virtual void SendBindingErrorResponse(StunMessage* request,
const rtc::SocketAddress& addr,
int error_code,

9
p2p/base/port_unittest.cc
View File

@ -302,7 +302,7 @@ class TestChannel : public sigslot::has_slots<> {
c.set_address(remote_address_);
conn_ = port_->CreateConnection(c, Port::ORIGIN_MESSAGE);
conn_->SignalDestroyed.connect(this, &TestChannel::OnDestroyed);
port_->SendBindingResponse(remote_request_.get(), remote_address_);
conn_->SendBindingResponse(remote_request_.get());
remote_request_.reset();
}
void Ping() { Ping(0); }
@ -2618,11 +2618,10 @@ TEST_F(PortTest, TestIceLiteConnectivity) {
// NOTE: Ideally we should't create connection at this stage from lite
// port, as it should be done only after receiving ping with USE_CANDIDATE.
// But we need a connection to send a response message.
ice_lite_port->CreateConnection(ice_full_port_ptr->Candidates()[0],
cricket::Port::ORIGIN_MESSAGE);
auto* con = ice_lite_port->CreateConnection(
ice_full_port_ptr->Candidates()[0], cricket::Port::ORIGIN_MESSAGE);
std::unique_ptr<IceMessage> request = CopyStunMessage(*msg);
ice_lite_port->SendBindingResponse(
request.get(), ice_full_port_ptr->Candidates()[0].address());
con->SendBindingResponse(request.get());
// Feeding the respone message from litemode to the full mode connection.
ch1.conn()->OnReadPacket(ice_lite_port->last_stun_buf()->data<char>(),