From 8777a4f8c6912d0e6f953afe05a7c14ebdf00a3a Mon Sep 17 00:00:00 2001 From: Nayana Bidari Date: Wed, 1 Dec 2021 13:57:43 -0800 Subject: [PATCH] Increment spurious recovery metric only for RTO. PiperOrigin-RevId: 413504208 --- pkg/sentry/socket/netstack/netstack.go | 1 + pkg/tcpip/tcpip.go | 3 +++ pkg/tcpip/transport/tcp/snd.go | 7 +++++++ pkg/tcpip/transport/tcp/tcp_sack_test.go | 9 +++++---- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/pkg/sentry/socket/netstack/netstack.go b/pkg/sentry/socket/netstack/netstack.go index 87db719f9..30c81c27a 100644 --- a/pkg/sentry/socket/netstack/netstack.go +++ b/pkg/sentry/socket/netstack/netstack.go @@ -273,6 +273,7 @@ var Metrics = tcpip.Stats{ FailedPortReservations: mustCreateMetric("/netstack/tcp/failed_port_reservations", "Number of time TCP failed to reserve a port."), SegmentsAckedWithDSACK: mustCreateMetric("/netstack/tcp/segments_acked_with_dsack", "Number of segments for which DSACK was received."), SpuriousRecovery: mustCreateMetric("/netstack/tcp/spurious_recovery", "Number of times the connection entered loss recovery spuriously."), + SpuriousRTORecovery: mustCreateMetric("/netstack/tcp/spurious_rto_recovery", "Number of times the connection entered RTO spuriously."), }, UDP: tcpip.UDPStats{ PacketsReceived: mustCreateMetric("/netstack/udp/packets_received", "Number of UDP datagrams received via HandlePacket."), diff --git a/pkg/tcpip/tcpip.go b/pkg/tcpip/tcpip.go index d20dd495c..b7616db93 100644 --- a/pkg/tcpip/tcpip.go +++ b/pkg/tcpip/tcpip.go @@ -1870,6 +1870,9 @@ type TCPStats struct { // SpuriousRecovery is the number of times the connection entered loss // recovery spuriously. SpuriousRecovery *StatCounter + + // SpuriousRTORecovery is the number of spurious RTOs. + SpuriousRTORecovery *StatCounter } // UDPStats collects UDP-specific stats. diff --git a/pkg/tcpip/transport/tcp/snd.go b/pkg/tcpip/transport/tcp/snd.go index 4377f07a0..533c3ccf9 100644 --- a/pkg/tcpip/transport/tcp/snd.go +++ b/pkg/tcpip/transport/tcp/snd.go @@ -1344,6 +1344,13 @@ func (s *sender) detectSpuriousRecovery(hasDSACK bool, tsEchoReply uint32) { // between fast, SACK or RTO recovery. s.spuriousRecovery = true s.ep.stack.Stats().TCP.SpuriousRecovery.Increment() + + // RFC 3522 will detect all kinds of spurious recoveries (fast, SACK and + // timeout). Increment the metric for RTO only as we want to track the + // number of timeout recoveries. + if s.state == tcpip.RTORecovery { + s.ep.stack.Stats().TCP.SpuriousRTORecovery.Increment() + } } // Check if the sender is in RTORecovery, FastRecovery or SACKRecovery state. diff --git a/pkg/tcpip/transport/tcp/tcp_sack_test.go b/pkg/tcpip/transport/tcp/tcp_sack_test.go index 896249d2d..a3eb8b6ca 100644 --- a/pkg/tcpip/transport/tcp/tcp_sack_test.go +++ b/pkg/tcpip/transport/tcp/tcp_sack_test.go @@ -704,7 +704,7 @@ func TestRecoveryEntry(t *testing.T) { } } -func verifySpuriousRecoveryMetric(t *testing.T, c *context.Context, numSpuriousRecovery uint64) { +func verifySpuriousRecoveryMetric(t *testing.T, c *context.Context, numSpuriousRecovery, numSpuriousRTO uint64) { t.Helper() metricPollFn := func() error { @@ -715,6 +715,7 @@ func verifySpuriousRecoveryMetric(t *testing.T, c *context.Context, numSpuriousR want uint64 }{ {tcpStats.SpuriousRecovery, "stats.TCP.SpuriousRecovery", numSpuriousRecovery}, + {tcpStats.SpuriousRTORecovery, "stats.TCP.SpuriousRTORecovery", numSpuriousRTO}, } for _, s := range stats { if got, want := s.stat.Value(), s.want; got != want { @@ -829,7 +830,7 @@ func TestDetectSpuriousRecoveryWithRTO(t *testing.T) { // ACK before the test completes. <-probeDone - verifySpuriousRecoveryMetric(t, c, 1 /* numSpuriousRecovery */) + verifySpuriousRecoveryMetric(t, c, 1 /* numSpuriousRecovery */, 1 /* numSpuriousRTO */) } func TestSACKDetectSpuriousRecoveryWithDupACK(t *testing.T) { @@ -923,7 +924,7 @@ func TestSACKDetectSpuriousRecoveryWithDupACK(t *testing.T) { // ACK before the test completes. <-probeDone - verifySpuriousRecoveryMetric(t, c, 1 /* numSpuriousRecovery */) + verifySpuriousRecoveryMetric(t, c, 1 /* numSpuriousRecovery */, 0 /* numSpuriousRTO */) } func TestNoSpuriousRecoveryWithDSACK(t *testing.T) { @@ -955,5 +956,5 @@ func TestNoSpuriousRecoveryWithDSACK(t *testing.T) { seq = seqnum.Value(context.TestInitialSequenceNumber).Add(1) c.SendAckWithSACK(seq, 6*maxPayload, []header.SACKBlock{{start, end}}) - verifySpuriousRecoveryMetric(t, c, 0 /* numSpuriousRecovery */) + verifySpuriousRecoveryMetric(t, c, 0 /* numSpuriousRecovery */, 0 /* numSpuriousRTO */) }