Revert "Add transaction id to CandidatePairEvents."
This reverts commit c308bdfa451ee2ceac7096b6777fcbf756f4091a.
Reason for revert: The msan bot has been consistently failing since this commit. See eg https://ci.chromium.org/p/webrtc/builders/luci.webrtc.ci/Linux%20MSan/16989
Original change's description:
> Add transaction id to CandidatePairEvents.
>
> The transaction id is a randomly generated number used to link stun
> requests and responses (https://tools.ietf.org/html/rfc5389#section-6).
> Logging this will help us debug ICE network issues.
>
> Bug: webrtc:9972
> Change-Id: I93167cb119aad99156e8727b6e4eeeff5198f924
> Reviewed-on: https://webrtc-review.googlesource.com/c/109720
> Commit-Queue: Zach Stein <zstein@webrtc.org>
> Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
> Reviewed-by: Elad Alon <eladalon@webrtc.org>
> Reviewed-by: Björn Terelius <terelius@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#25848}
TBR=eladalon@webrtc.org,terelius@webrtc.org,zstein@webrtc.org,qingsi@webrtc.org,jeroendb@webrtc.org
Change-Id: Ib3b0a845f2300f4fcba2061650e17522735f08b3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: webrtc:9972
Reviewed-on: https://webrtc-review.googlesource.com/c/112581
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25852}diff --git a/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.cc b/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.cc
index 225362d..9c31737 100644
--- a/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.cc
+++ b/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.cc
@@ -16,18 +16,14 @@
RtcEventIceCandidatePair::RtcEventIceCandidatePair(
IceCandidatePairEventType type,
- uint32_t candidate_pair_id,
- uint32_t transaction_id)
- : type_(type),
- candidate_pair_id_(candidate_pair_id),
- transaction_id_(transaction_id) {}
+ uint32_t candidate_pair_id)
+ : type_(type), candidate_pair_id_(candidate_pair_id) {}
RtcEventIceCandidatePair::RtcEventIceCandidatePair(
const RtcEventIceCandidatePair& other)
: RtcEvent(other.timestamp_us_),
type_(other.type_),
- candidate_pair_id_(other.candidate_pair_id_),
- transaction_id_(other.transaction_id_) {}
+ candidate_pair_id_(other.candidate_pair_id_) {}
RtcEventIceCandidatePair::~RtcEventIceCandidatePair() = default;
diff --git a/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.h b/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.h
index 973a12a..1dc1b8e 100644
--- a/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.h
+++ b/logging/rtc_event_log/events/rtc_event_ice_candidate_pair.h
@@ -29,8 +29,7 @@
class RtcEventIceCandidatePair final : public RtcEvent {
public:
RtcEventIceCandidatePair(IceCandidatePairEventType type,
- uint32_t candidate_pair_id,
- uint32_t transaction_id);
+ uint32_t candidate_pair_id);
~RtcEventIceCandidatePair() override;
@@ -42,14 +41,12 @@
IceCandidatePairEventType type() const { return type_; }
uint32_t candidate_pair_id() const { return candidate_pair_id_; }
- uint32_t transaction_id() const { return transaction_id_; }
private:
RtcEventIceCandidatePair(const RtcEventIceCandidatePair& other);
const IceCandidatePairEventType type_;
const uint32_t candidate_pair_id_;
- const uint32_t transaction_id_;
};
} // namespace webrtc
diff --git a/logging/rtc_event_log/icelogger.cc b/logging/rtc_event_log/icelogger.cc
index c1dbcd8..5f91c5e 100644
--- a/logging/rtc_event_log/icelogger.cc
+++ b/logging/rtc_event_log/icelogger.cc
@@ -31,13 +31,12 @@
}
void IceEventLog::LogCandidatePairEvent(IceCandidatePairEventType type,
- uint32_t candidate_pair_id,
- uint32_t transaction_id) {
+ uint32_t candidate_pair_id) {
if (event_log_ == nullptr) {
return;
}
- event_log_->Log(absl::make_unique<RtcEventIceCandidatePair>(
- type, candidate_pair_id, transaction_id));
+ event_log_->Log(
+ absl::make_unique<RtcEventIceCandidatePair>(type, candidate_pair_id));
}
void IceEventLog::DumpCandidatePairDescriptionToMemoryAsConfigEvents() const {
diff --git a/logging/rtc_event_log/icelogger.h b/logging/rtc_event_log/icelogger.h
index f14cf0d..b590fd7 100644
--- a/logging/rtc_event_log/icelogger.h
+++ b/logging/rtc_event_log/icelogger.h
@@ -36,8 +36,7 @@
const IceCandidatePairDescription& candidate_pair_desc);
void LogCandidatePairEvent(IceCandidatePairEventType type,
- uint32_t candidate_pair_id,
- uint32_t transaction_id);
+ uint32_t candidate_pair_id);
// This method constructs a config event for each candidate pair with their
// description and logs these config events. It is intended to be called when
diff --git a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc
index 6787cf5..7e20d43 100644
--- a/logging/rtc_event_log/rtc_event_log_unittest_helper.cc
+++ b/logging/rtc_event_log/rtc_event_log_unittest_helper.cc
@@ -214,10 +214,8 @@
static_cast<IceCandidatePairEventType>(prng_.Rand(
static_cast<uint32_t>(IceCandidatePairEventType::kNumValues) - 1));
uint32_t pair_id = prng_.Rand<uint32_t>();
- uint32_t transaction_id = prng_.Rand<uint32_t>();
- return absl::make_unique<RtcEventIceCandidatePair>(type, pair_id,
- transaction_id);
+ return absl::make_unique<RtcEventIceCandidatePair>(type, pair_id);
}
rtcp::ReportBlock EventGenerator::NewReportBlock() {
diff --git a/p2p/base/port.cc b/p2p/base/port.cc
index 4d93b4f..5b8e02d 100644
--- a/p2p/base/port.cc
+++ b/p2p/base/port.cc
@@ -860,8 +860,7 @@
conn->stats_.sent_ping_responses++;
conn->LogCandidatePairEvent(
- webrtc::IceCandidatePairEventType::kCheckResponseSent,
- request->reduced_transaction_id());
+ webrtc::IceCandidatePairEventType::kCheckResponseSent);
}
}
@@ -1341,8 +1340,7 @@
}
stats_.recv_ping_requests++;
- LogCandidatePairEvent(webrtc::IceCandidatePairEventType::kCheckReceived,
- msg->reduced_transaction_id());
+ LogCandidatePairEvent(webrtc::IceCandidatePairEventType::kCheckReceived);
// This is a validated stun request from remote peer.
port_->SendBindingResponse(msg, remote_addr);
@@ -1679,12 +1677,11 @@
ice_event_log_->LogCandidatePairConfig(type, id(), ToLogDescription());
}
-void Connection::LogCandidatePairEvent(webrtc::IceCandidatePairEventType type,
- uint32_t transaction_id) {
+void Connection::LogCandidatePairEvent(webrtc::IceCandidatePairEventType type) {
if (ice_event_log_ == nullptr) {
return;
}
- ice_event_log_->LogCandidatePairEvent(type, id(), transaction_id);
+ ice_event_log_->LogCandidatePairEvent(type, id());
}
void Connection::OnConnectionRequestResponse(ConnectionRequest* request,
@@ -1711,8 +1708,7 @@
stats_.recv_ping_responses++;
LogCandidatePairEvent(
- webrtc::IceCandidatePairEventType::kCheckResponseReceived,
- response->reduced_transaction_id());
+ webrtc::IceCandidatePairEventType::kCheckResponseReceived);
MaybeUpdateLocalCandidate(request, response);
}
@@ -1758,8 +1754,7 @@
<< ", use_candidate=" << use_candidate_attr()
<< ", nomination=" << nomination();
stats_.sent_ping_requests_total++;
- LogCandidatePairEvent(webrtc::IceCandidatePairEventType::kCheckSent,
- request->reduced_transaction_id());
+ LogCandidatePairEvent(webrtc::IceCandidatePairEventType::kCheckSent);
if (stats_.recv_ping_responses == 0) {
stats_.sent_ping_requests_before_first_response++;
}
diff --git a/p2p/base/port.h b/p2p/base/port.h
index ca4fedb..9a8f92a 100644
--- a/p2p/base/port.h
+++ b/p2p/base/port.h
@@ -789,8 +789,7 @@
StunMessage* response);
void LogCandidatePairConfig(webrtc::IceCandidatePairConfigType type);
- void LogCandidatePairEvent(webrtc::IceCandidatePairEventType type,
- uint32_t transaction_id);
+ void LogCandidatePairEvent(webrtc::IceCandidatePairEventType type);
WriteState write_state_;
bool receiving_;
diff --git a/p2p/base/stun.cc b/p2p/base/stun.cc
index 33b4448..fa6b6f8 100644
--- a/p2p/base/stun.cc
+++ b/p2p/base/stun.cc
@@ -26,25 +26,6 @@
using rtc::ByteBufferReader;
using rtc::ByteBufferWriter;
-namespace {
-
-uint32_t ReduceTransactionId(const std::string& transaction_id) {
- RTC_DCHECK(transaction_id.length() == cricket::kStunTransactionIdLength ||
- transaction_id.length() ==
- cricket::kStunLegacyTransactionIdLength);
- uint32_t transaction_id_as_ints[4];
- memcpy(transaction_id_as_ints, transaction_id.c_str(),
- transaction_id.length());
-
- uint32_t result = 0;
- for (size_t i = 0; i < transaction_id.length() / 4; ++i) {
- result ^= transaction_id_as_ints[i];
- }
- return result;
-}
-
-} // namespace
-
namespace cricket {
const char STUN_ERROR_REASON_TRY_ALTERNATE_SERVER[] = "Try Alternate Server";
@@ -87,7 +68,6 @@
return false;
}
transaction_id_ = str;
- reduced_transaction_id_ = ReduceTransactionId(transaction_id_);
return true;
}
@@ -374,7 +354,6 @@
}
RTC_DCHECK(IsValidTransactionId(transaction_id));
transaction_id_ = transaction_id;
- reduced_transaction_id_ = ReduceTransactionId(transaction_id_);
if (length_ != buf->Length())
return false;
diff --git a/p2p/base/stun.h b/p2p/base/stun.h
index ca664b0..6083772 100644
--- a/p2p/base/stun.h
+++ b/p2p/base/stun.h
@@ -141,7 +141,6 @@
int type() const { return type_; }
size_t length() const { return length_; }
const std::string& transaction_id() const { return transaction_id_; }
- uint32_t reduced_transaction_id() const { return reduced_transaction_id_; }
// Returns true if the message confirms to RFC3489 rather than
// RFC5389. The main difference between two version of the STUN
@@ -215,7 +214,6 @@
uint16_t type_;
uint16_t length_;
std::string transaction_id_;
- uint32_t reduced_transaction_id_;
std::vector<std::unique_ptr<StunAttribute>> attrs_;
uint32_t stun_magic_cookie_;
};
diff --git a/p2p/base/stunrequest.h b/p2p/base/stunrequest.h
index 4a9a9b4..3fac2d3 100644
--- a/p2p/base/stunrequest.h
+++ b/p2p/base/stunrequest.h
@@ -100,11 +100,6 @@
// Returns the transaction ID of this request.
const std::string& id() { return msg_->transaction_id(); }
- // Returns the reduced transaction ID of this request.
- uint32_t reduced_transaction_id() const {
- return msg_->reduced_transaction_id();
- }
-
// the origin value
const std::string& origin() const { return origin_; }
void set_origin(const std::string& origin) { origin_ = origin; }
