[op] Deflake SNMP Metric proc_net tests.

Earlier the tests were checking for equality of system-wide metrics before and
after some network related operations. That is inherently racy for native tests
because depending on the testing infrastructure, multiple tests might run
parallely hence trampling over each other's metrics.

Tests should only compare metrics that are increasing in nature. The comparison
should not be a hard comparison, instead a less-than/greater-than relation test.

I have changed the checks and also removed tests for tcpCurrEstab metric which
has "SYNTAX  Gauge" and hence can not be tested reliably.

PiperOrigin-RevId: 391460081

1 files changed, 41 insertions, 20 deletions

diff --git a/test/syscalls/linux/proc_net.cc b/test/syscalls/linux/proc_net.cc
index 162c0b665..672f2dd42 100644
--- a/test/syscalls/linux/proc_net.cc
+++ b/test/syscalls/linux/proc_net.cc
@@ -168,6 +168,21 @@ int GetMibsAllocationSysctl() {
   return val;
 }
 
+// GetSNMPMetricFromProc retrieves the metric named `item` of `type` from the
+// `snmp` string which represents the file contents of /proc/net/snmp.
+//
+// Note to test writers: If you are writing tests to check the change in value
+// of SNMP metrics, note that if GetMibsAllocationSysctl() == 0
+// (net.core.mibs_allocation isn't set), MIB is from the init network namespace
+// and hence these metrics can have system-wide noise.
+//
+// A feasible way of testing is the following:
+// * Consult RFC 1213 to learn the "SYNTAX" of the metric.
+// * If net.core.mibs_allocation is set:
+//   - Can test any metric while checking for hard equality.
+// * If net.core.mibs_allocation is NOT set (system-wide noise is present):
+//   - Only test "SYNTAX  Counter" metrics.
+//   - Counter metrics are increasing in nature, so use LE/GE equality checks.
 PosixErrorOr<uint64_t> GetSNMPMetricFromProc(const std::string snmp,
                                              const std::string& type,
                                              const std::string& item) {
@@ -243,18 +258,22 @@ TEST(ProcNetSnmp, TcpReset) {
       GetSNMPMetricFromProc(snmp, "Tcp", "AttemptFails"));
 
   if (GetMibsAllocationSysctl()) {
-    EXPECT_EQ(oldActiveOpens, newActiveOpens - 1);
-    EXPECT_EQ(oldOutRsts, newOutRsts - 1);
-    EXPECT_EQ(oldAttemptFails, newAttemptFails - 1);
+    EXPECT_EQ(oldActiveOpens + 1, newActiveOpens);
+    EXPECT_EQ(oldOutRsts + 1, newOutRsts);
+    EXPECT_EQ(oldAttemptFails + 1, newAttemptFails);
   } else {
-    // System-wide statistics can have some noise.
-    EXPECT_LE(oldOutRsts, newOutRsts - 1);
-    EXPECT_LE(oldAttemptFails, newAttemptFails - 1);
+    // System-wide statistics can have some noise. These metrics should have
+    // increased by at least 1.
+    EXPECT_LE(oldActiveOpens + 1, newActiveOpens);
+    EXPECT_LE(oldOutRsts + 1, newOutRsts);
+    EXPECT_LE(oldAttemptFails + 1, newAttemptFails);
   }
 }
 
 TEST(ProcNetSnmp, TcpEstab) {
-  // System-wide statistics can have some noise.
+  // This test aims to test the tcpCurrEstab metric which has "SYNTAX  Gauge" as
+  // per RFC 1213. Hence, it becomes infeasible to test this when system-wide
+  // statistics have noise.
   SKIP_IF(GetMibsAllocationSysctl() == 0);
 
   // TODO(gvisor.dev/issue/866): epsocket metrics are not savable.
@@ -311,9 +330,9 @@ TEST(ProcNetSnmp, TcpEstab) {
   newCurrEstab = ASSERT_NO_ERRNO_AND_VALUE(
       GetSNMPMetricFromProc(snmp, "Tcp", "CurrEstab"));
 
-  EXPECT_EQ(oldActiveOpens, newActiveOpens - 1);
-  EXPECT_EQ(oldPassiveOpens, newPassiveOpens - 1);
-  EXPECT_EQ(oldCurrEstab, newCurrEstab - 2);
+  EXPECT_EQ(oldActiveOpens + 1, newActiveOpens);
+  EXPECT_EQ(oldPassiveOpens + 1, newPassiveOpens);
+  EXPECT_EQ(oldCurrEstab + 2, newCurrEstab);
 
   // Send 1 byte from client to server.
   ASSERT_THAT(send(s_connect.get(), "a", 1, 0), SyscallSucceedsWithValue(1));
@@ -381,12 +400,13 @@ TEST(ProcNetSnmp, UdpNoPorts) {
       ASSERT_NO_ERRNO_AND_VALUE(GetSNMPMetricFromProc(snmp, "Udp", "NoPorts"));
 
   if (GetMibsAllocationSysctl()) {
-    EXPECT_EQ(oldOutDatagrams, newOutDatagrams - 1);
-    EXPECT_EQ(oldNoPorts, newNoPorts - 1);
+    EXPECT_EQ(oldOutDatagrams + 1, newOutDatagrams);
+    EXPECT_EQ(oldNoPorts + 1, newNoPorts);
   } else {
-    // System-wide statistics can have some noise.
-    EXPECT_LE(oldOutDatagrams, newOutDatagrams - 1);
-    EXPECT_LE(oldNoPorts, newNoPorts - 1);
+    // System-wide statistics can have some noise. These metrics should have
+    // increased by at least 1.
+    EXPECT_LE(oldOutDatagrams + 1, newOutDatagrams);
+    EXPECT_LE(oldNoPorts + 1, newNoPorts);
   }
 }
 
@@ -437,12 +457,13 @@ TEST(ProcNetSnmp, UdpIn) {
       GetSNMPMetricFromProc(snmp, "Udp", "InDatagrams"));
 
   if (GetMibsAllocationSysctl()) {
-    EXPECT_EQ(oldOutDatagrams, newOutDatagrams - 1);
-    EXPECT_EQ(oldInDatagrams, newInDatagrams - 1);
+    EXPECT_EQ(oldOutDatagrams + 1, newOutDatagrams);
+    EXPECT_EQ(oldInDatagrams + 1, newInDatagrams);
   } else {
-    // System-wide statistics can have some noise.
-    EXPECT_LE(oldOutDatagrams, newOutDatagrams - 1);
-    EXPECT_LE(oldInDatagrams, newInDatagrams - 1);
+    // System-wide statistics can have some noise. These metrics should have
+    // increased by at least 1.
+    EXPECT_LE(oldOutDatagrams + 1, newOutDatagrams);
+    EXPECT_LE(oldInDatagrams + 1, newInDatagrams);
   }
 }