dcsctp: Cleanup Metrics
This CL first restricts Metrics to be retrievable when the socket is
created. This avoids having most fields as optional and makes it
easier to add more metrics.
Secondly, the peer implementation is moved into Metrics.
Bug: webrtc:13052
Change-Id: I6cb53eeef3f84ac34f3efc883853338f903cc758
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262256
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36888}
diff --git a/net/dcsctp/public/dcsctp_socket.h b/net/dcsctp/public/dcsctp_socket.h
index 7001585..e15a5bf 100644
--- a/net/dcsctp/public/dcsctp_socket.h
+++ b/net/dcsctp/public/dcsctp_socket.h
@@ -170,43 +170,6 @@
kError,
};
-// Tracked metrics, which is the return value of GetMetrics. Optional members
-// will be unset when they are not yet known.
-struct Metrics {
- // Transmission stats and metrics.
-
- // Number of packets sent.
- size_t tx_packets_count = 0;
-
- // Number of messages requested to be sent.
- size_t tx_messages_count = 0;
-
- // The current congestion window (cwnd) in bytes, corresponding to spinfo_cwnd
- // defined in RFC6458.
- absl::optional<size_t> cwnd_bytes = absl::nullopt;
-
- // Smoothed round trip time, corresponding to spinfo_srtt defined in RFC6458.
- absl::optional<int> srtt_ms = absl::nullopt;
-
- // Number of data items in the retransmission queue that haven’t been
- // acked/nacked yet and are in-flight. Corresponding to sstat_unackdata
- // defined in RFC6458. This may be an approximation when there are messages in
- // the send queue that haven't been fragmented/packetized yet.
- size_t unack_data_count = 0;
-
- // Receive stats and metrics.
-
- // Number of packets received.
- size_t rx_packets_count = 0;
-
- // Number of messages received.
- size_t rx_messages_count = 0;
-
- // The peer’s last announced receiver window size, corresponding to
- // sstat_rwnd defined in RFC6458.
- absl::optional<uint32_t> peer_rwnd_bytes = absl::nullopt;
-};
-
// Represent known SCTP implementations.
enum class SctpImplementation {
// There is not enough information toto determine any SCTP implementation.
@@ -232,6 +195,48 @@
}
}
+// Tracked metrics, which is the return value of GetMetrics. Optional members
+// will be unset when they are not yet known.
+struct Metrics {
+ // Transmission stats and metrics.
+
+ // Number of packets sent.
+ size_t tx_packets_count = 0;
+
+ // Number of messages requested to be sent.
+ size_t tx_messages_count = 0;
+
+ // The current congestion window (cwnd) in bytes, corresponding to spinfo_cwnd
+ // defined in RFC6458.
+ size_t cwnd_bytes = 0;
+
+ // Smoothed round trip time, corresponding to spinfo_srtt defined in RFC6458.
+ int srtt_ms = 0;
+
+ // Number of data items in the retransmission queue that haven’t been
+ // acked/nacked yet and are in-flight. Corresponding to sstat_unackdata
+ // defined in RFC6458. This may be an approximation when there are messages in
+ // the send queue that haven't been fragmented/packetized yet.
+ size_t unack_data_count = 0;
+
+ // Receive stats and metrics.
+
+ // Number of packets received.
+ size_t rx_packets_count = 0;
+
+ // Number of messages received.
+ size_t rx_messages_count = 0;
+
+ // The peer’s last announced receiver window size, corresponding to
+ // sstat_rwnd defined in RFC6458.
+ uint32_t peer_rwnd_bytes = 0;
+
+ // Returns the detected SCTP implementation of the peer. As this is not
+ // explicitly signalled during the connection establishment, heuristics is
+ // used to analyze e.g. the state cookie in the INIT-ACK chunk.
+ SctpImplementation peer_implementation = SctpImplementation::kUnknown;
+};
+
// Callbacks that the DcSctpSocket will call synchronously to the owning
// client. It is allowed to call back into the library from callbacks that start
// with "On". It has been explicitly documented when it's not allowed to call
@@ -467,8 +472,9 @@
virtual void SetBufferedAmountLowThreshold(StreamID stream_id,
size_t bytes) = 0;
- // Retrieves the latest metrics.
- virtual Metrics GetMetrics() const = 0;
+ // Retrieves the latest metrics. If the socket is not fully connected,
+ // `absl::nullopt` will be returned.
+ virtual absl::optional<Metrics> GetMetrics() const = 0;
// Returns empty bitmask if the socket is in the state in which a snapshot of
// the state can be made by `GetHandoverStateAndClose()`. Return value is
@@ -491,7 +497,10 @@
// If this method is called too early (before
// `DcSctpSocketCallbacks::OnConnected` has triggered), this will likely
// return `SctpImplementation::kUnknown`.
- virtual SctpImplementation peer_implementation() const = 0;
+ ABSL_DEPRECATED("See Metrics::peer_implementation instead")
+ virtual SctpImplementation peer_implementation() const {
+ return SctpImplementation::kUnknown;
+ }
};
} // namespace dcsctp
diff --git a/net/dcsctp/public/mock_dcsctp_socket.h b/net/dcsctp/public/mock_dcsctp_socket.h
index d207899..6560a3f 100644
--- a/net/dcsctp/public/mock_dcsctp_socket.h
+++ b/net/dcsctp/public/mock_dcsctp_socket.h
@@ -63,7 +63,7 @@
(StreamID stream_id, size_t bytes),
(override));
- MOCK_METHOD(Metrics, GetMetrics, (), (const, override));
+ MOCK_METHOD(absl::optional<Metrics>, GetMetrics, (), (const, override));
MOCK_METHOD(HandoverReadinessStatus,
GetHandoverReadiness,
@@ -73,8 +73,6 @@
GetHandoverStateAndClose,
(),
(override));
-
- MOCK_METHOD(SctpImplementation, peer_implementation, (), (const));
};
} // namespace dcsctp
diff --git a/net/dcsctp/socket/dcsctp_socket.cc b/net/dcsctp/socket/dcsctp_socket.cc
index d081bd9..9d6ae0e 100644
--- a/net/dcsctp/socket/dcsctp_socket.cc
+++ b/net/dcsctp/socket/dcsctp_socket.cc
@@ -524,24 +524,24 @@
send_queue_.SetBufferedAmountLowThreshold(stream_id, bytes);
}
-Metrics DcSctpSocket::GetMetrics() const {
+absl::optional<Metrics> DcSctpSocket::GetMetrics() const {
RTC_DCHECK_RUN_ON(&thread_checker_);
- Metrics metrics = metrics_;
- if (tcb_ != nullptr) {
- // Update the metrics with some stats that are extracted from
- // sub-components.
- metrics.cwnd_bytes = tcb_->cwnd();
- metrics.srtt_ms = tcb_->current_srtt().value();
- size_t packet_payload_size =
- options_.mtu - SctpPacket::kHeaderSize - DataChunk::kHeaderSize;
- metrics.unack_data_count =
- tcb_->retransmission_queue().outstanding_items() +
- (send_queue_.total_buffered_amount() + packet_payload_size - 1) /
- packet_payload_size;
- metrics.peer_rwnd_bytes = tcb_->retransmission_queue().rwnd();
+ if (tcb_ == nullptr) {
+ return absl::nullopt;
}
+ Metrics metrics = metrics_;
+ metrics.cwnd_bytes = tcb_->cwnd();
+ metrics.srtt_ms = tcb_->current_srtt().value();
+ size_t packet_payload_size =
+ options_.mtu - SctpPacket::kHeaderSize - DataChunk::kHeaderSize;
+ metrics.unack_data_count =
+ tcb_->retransmission_queue().outstanding_items() +
+ (send_queue_.total_buffered_amount() + packet_payload_size - 1) /
+ packet_payload_size;
+ metrics.peer_rwnd_bytes = tcb_->retransmission_queue().rwnd();
+
return metrics;
}
@@ -1187,7 +1187,7 @@
Capabilities capabilities = GetCapabilities(options_, chunk->parameters());
t1_init_->Stop();
- peer_implementation_ = DeterminePeerImplementation(cookie->data());
+ metrics_.peer_implementation = DeterminePeerImplementation(cookie->data());
tcb_ = std::make_unique<TransmissionControlBlock>(
timer_manager_, log_prefix_, options_, capabilities, callbacks_,
diff --git a/net/dcsctp/socket/dcsctp_socket.h b/net/dcsctp/socket/dcsctp_socket.h
index 0ab54e8..07e760a 100644
--- a/net/dcsctp/socket/dcsctp_socket.h
+++ b/net/dcsctp/socket/dcsctp_socket.h
@@ -99,11 +99,11 @@
size_t buffered_amount(StreamID stream_id) const override;
size_t buffered_amount_low_threshold(StreamID stream_id) const override;
void SetBufferedAmountLowThreshold(StreamID stream_id, size_t bytes) override;
- Metrics GetMetrics() const override;
+ absl::optional<Metrics> GetMetrics() const override;
HandoverReadinessStatus GetHandoverReadiness() const override;
absl::optional<DcSctpSocketHandoverState> GetHandoverStateAndClose() override;
SctpImplementation peer_implementation() const override {
- return peer_implementation_;
+ return metrics_.peer_implementation;
}
// Returns this socket's verification tag, or zero if not yet connected.
VerificationTag verification_tag() const {
@@ -282,8 +282,6 @@
State state_ = State::kClosed;
// If the connection is established, contains a transmission control block.
std::unique_ptr<TransmissionControlBlock> tcb_;
-
- SctpImplementation peer_implementation_ = SctpImplementation::kUnknown;
};
} // namespace dcsctp
diff --git a/net/dcsctp/socket/dcsctp_socket_test.cc b/net/dcsctp/socket/dcsctp_socket_test.cc
index 914bea3..770fd84 100644
--- a/net/dcsctp/socket/dcsctp_socket_test.cc
+++ b/net/dcsctp/socket/dcsctp_socket_test.cc
@@ -1874,18 +1874,10 @@
MaybeHandoverSocketAndSendMessage(a, std::move(z));
}
-TEST(DcSctpSocketTest, InitialMetricsAreZeroed) {
+TEST(DcSctpSocketTest, InitialMetricsAreUnset) {
SocketUnderTest a("A");
- Metrics metrics = a.socket.GetMetrics();
- EXPECT_EQ(metrics.tx_packets_count, 0u);
- EXPECT_EQ(metrics.tx_messages_count, 0u);
- EXPECT_EQ(metrics.cwnd_bytes.has_value(), false);
- EXPECT_EQ(metrics.srtt_ms.has_value(), false);
- EXPECT_EQ(metrics.unack_data_count, 0u);
- EXPECT_EQ(metrics.rx_packets_count, 0u);
- EXPECT_EQ(metrics.rx_messages_count, 0u);
- EXPECT_EQ(metrics.peer_rwnd_bytes.has_value(), false);
+ EXPECT_FALSE(a.socket.GetMetrics().has_value());
}
TEST(DcSctpSocketTest, RxAndTxPacketMetricsIncrease) {
@@ -1897,65 +1889,65 @@
const size_t initial_a_rwnd = a.options.max_receiver_window_buffer_size *
ReassemblyQueue::kHighWatermarkLimit;
- EXPECT_EQ(a.socket.GetMetrics().tx_packets_count, 2u);
- EXPECT_EQ(a.socket.GetMetrics().rx_packets_count, 2u);
- EXPECT_EQ(a.socket.GetMetrics().tx_messages_count, 0u);
- EXPECT_EQ(*a.socket.GetMetrics().cwnd_bytes,
+ EXPECT_EQ(a.socket.GetMetrics()->tx_packets_count, 2u);
+ EXPECT_EQ(a.socket.GetMetrics()->rx_packets_count, 2u);
+ EXPECT_EQ(a.socket.GetMetrics()->tx_messages_count, 0u);
+ EXPECT_EQ(a.socket.GetMetrics()->cwnd_bytes,
a.options.cwnd_mtus_initial * a.options.mtu);
- EXPECT_EQ(a.socket.GetMetrics().unack_data_count, 0u);
+ EXPECT_EQ(a.socket.GetMetrics()->unack_data_count, 0u);
- EXPECT_EQ(z.socket.GetMetrics().rx_packets_count, 2u);
- EXPECT_EQ(z.socket.GetMetrics().rx_messages_count, 0u);
+ EXPECT_EQ(z.socket.GetMetrics()->rx_packets_count, 2u);
+ EXPECT_EQ(z.socket.GetMetrics()->rx_messages_count, 0u);
a.socket.Send(DcSctpMessage(StreamID(1), PPID(53), {1, 2}), kSendOptions);
- EXPECT_EQ(a.socket.GetMetrics().unack_data_count, 1u);
+ EXPECT_EQ(a.socket.GetMetrics()->unack_data_count, 1u);
z.socket.ReceivePacket(a.cb.ConsumeSentPacket()); // DATA
a.socket.ReceivePacket(z.cb.ConsumeSentPacket()); // SACK
- EXPECT_EQ(*a.socket.GetMetrics().peer_rwnd_bytes, initial_a_rwnd);
- EXPECT_EQ(a.socket.GetMetrics().unack_data_count, 0u);
+ EXPECT_EQ(a.socket.GetMetrics()->peer_rwnd_bytes, initial_a_rwnd);
+ EXPECT_EQ(a.socket.GetMetrics()->unack_data_count, 0u);
EXPECT_TRUE(z.cb.ConsumeReceivedMessage().has_value());
- EXPECT_EQ(a.socket.GetMetrics().tx_packets_count, 3u);
- EXPECT_EQ(a.socket.GetMetrics().rx_packets_count, 3u);
- EXPECT_EQ(a.socket.GetMetrics().tx_messages_count, 1u);
+ EXPECT_EQ(a.socket.GetMetrics()->tx_packets_count, 3u);
+ EXPECT_EQ(a.socket.GetMetrics()->rx_packets_count, 3u);
+ EXPECT_EQ(a.socket.GetMetrics()->tx_messages_count, 1u);
- EXPECT_EQ(z.socket.GetMetrics().rx_packets_count, 3u);
- EXPECT_EQ(z.socket.GetMetrics().rx_messages_count, 1u);
+ EXPECT_EQ(z.socket.GetMetrics()->rx_packets_count, 3u);
+ EXPECT_EQ(z.socket.GetMetrics()->rx_messages_count, 1u);
// Send one more (large - fragmented), and receive the delayed SACK.
a.socket.Send(DcSctpMessage(StreamID(1), PPID(53),
std::vector<uint8_t>(a.options.mtu * 2 + 1)),
kSendOptions);
- EXPECT_EQ(a.socket.GetMetrics().unack_data_count, 3u);
+ EXPECT_EQ(a.socket.GetMetrics()->unack_data_count, 3u);
z.socket.ReceivePacket(a.cb.ConsumeSentPacket()); // DATA
z.socket.ReceivePacket(a.cb.ConsumeSentPacket()); // DATA
a.socket.ReceivePacket(z.cb.ConsumeSentPacket()); // SACK
- EXPECT_EQ(a.socket.GetMetrics().unack_data_count, 1u);
- EXPECT_GT(*a.socket.GetMetrics().peer_rwnd_bytes, 0u);
- EXPECT_LT(*a.socket.GetMetrics().peer_rwnd_bytes, initial_a_rwnd);
+ EXPECT_EQ(a.socket.GetMetrics()->unack_data_count, 1u);
+ EXPECT_GT(a.socket.GetMetrics()->peer_rwnd_bytes, 0u);
+ EXPECT_LT(a.socket.GetMetrics()->peer_rwnd_bytes, initial_a_rwnd);
z.socket.ReceivePacket(a.cb.ConsumeSentPacket()); // DATA
EXPECT_TRUE(z.cb.ConsumeReceivedMessage().has_value());
- EXPECT_EQ(a.socket.GetMetrics().tx_packets_count, 6u);
- EXPECT_EQ(a.socket.GetMetrics().rx_packets_count, 4u);
- EXPECT_EQ(a.socket.GetMetrics().tx_messages_count, 2u);
+ EXPECT_EQ(a.socket.GetMetrics()->tx_packets_count, 6u);
+ EXPECT_EQ(a.socket.GetMetrics()->rx_packets_count, 4u);
+ EXPECT_EQ(a.socket.GetMetrics()->tx_messages_count, 2u);
- EXPECT_EQ(z.socket.GetMetrics().rx_packets_count, 6u);
- EXPECT_EQ(z.socket.GetMetrics().rx_messages_count, 2u);
+ EXPECT_EQ(z.socket.GetMetrics()->rx_packets_count, 6u);
+ EXPECT_EQ(z.socket.GetMetrics()->rx_messages_count, 2u);
// Delayed sack
AdvanceTime(a, z, a.options.delayed_ack_max_timeout);
a.socket.ReceivePacket(z.cb.ConsumeSentPacket()); // SACK
- EXPECT_EQ(a.socket.GetMetrics().unack_data_count, 0u);
- EXPECT_EQ(a.socket.GetMetrics().rx_packets_count, 5u);
- EXPECT_EQ(*a.socket.GetMetrics().peer_rwnd_bytes, initial_a_rwnd);
+ EXPECT_EQ(a.socket.GetMetrics()->unack_data_count, 0u);
+ EXPECT_EQ(a.socket.GetMetrics()->rx_packets_count, 5u);
+ EXPECT_EQ(a.socket.GetMetrics()->peer_rwnd_bytes, initial_a_rwnd);
}
TEST_P(DcSctpSocketParametrizedTest, UnackDataAlsoIncludesSendQueue) {
@@ -1980,10 +1972,10 @@
// Due to alignment, padding etc, it's hard to calculate the exact number, but
// it should be in this range.
- EXPECT_GE(a.socket.GetMetrics().unack_data_count,
+ EXPECT_GE(a.socket.GetMetrics()->unack_data_count,
expected_sent_packets + expected_queued_packets);
- EXPECT_LE(a.socket.GetMetrics().unack_data_count,
+ EXPECT_LE(a.socket.GetMetrics()->unack_data_count,
expected_sent_packets + expected_queued_packets + 2);
MaybeHandoverSocketAndSendMessage(a, std::move(z));
