Merge branch 'main' into Pins_WF_1

tests/forwarding/l3_admit_test.cc

@@ -526,15 +526,7 @@ TEST_P(L3AdmitTestFixture, DISABLED_L3AdmitCanUseInPortToRestrictMacAddresses) {
   }
   LOG(INFO) << "Done collecting packets.";
 
-  if (GetMirrorTestbed().Environment().MaskKnownFailures()) {
-    // TODO: Reduce expected count by tolerance level.
-    const int kDropTolerance = 1;
-    int adjusted_good_packets = kNumberOfTestPacket - kDropTolerance;
-    EXPECT_GE(good_packet_count, adjusted_good_packets);
-    EXPECT_LE(good_packet_count, kNumberOfTestPacket);
-  } else {
-    EXPECT_EQ(good_packet_count, kNumberOfTestPacket);
-  }
+  EXPECT_EQ(good_packet_count, kNumberOfTestPacket);
   EXPECT_EQ(bad_packet_count, 0);
 }
 

tests/gnmi/BUILD.bazel

@@ -51,23 +51,20 @@ cc_library(
     srcs = ["ethcounter_ixia_test.cc"],
     hdrs = ["ethcounter_ixia_test.h"],
     deps = [
-	"//gutil:collections",
-        "//gutil:status",
+        "//gutil:collections",
         "//gutil:status_matchers",
         "//gutil:testing",
         "//lib:ixia_helper",
         "//lib/gnmi:gnmi_helper",
-        "//lib/p4rt:packet_listener",
-        "//lib/validator:validator_lib",
         "//p4_pdpi:ir",
         "//p4_pdpi:p4_runtime_session",
         "//p4_pdpi:pd",
-	"//p4_pdpi/netaddr:mac_address",
-        "//p4_pdpi/packetlib",
+        "//p4_pdpi/netaddr:mac_address",
         "//p4_pdpi/packetlib:packetlib_cc_proto",
+        "//sai_p4/instantiations/google:sai_pd_cc_proto",
+        "//sai_p4/instantiations/google:sai_p4info_cc",
 	"//tests/forwarding:util",
         "//tests/lib:switch_test_setup_helpers",
-        "//sai_p4/instantiations/google:sai_p4info_cc",
         "//thinkit:generic_testbed",
         "//thinkit:generic_testbed_fixture",
         "//thinkit:switch",
@@ -77,9 +74,11 @@ cc_library(
 	"@com_google_absl//absl/cleanup",
         "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/status",
+        "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",
         "@com_google_absl//absl/time",
-	"@com_google_googletest//:gtest",
+        "@com_google_absl//absl/types:optional",
+        "@com_google_googletest//:gtest",
     ],
     alwayslink = True,
 )

tests/gnmi/ethcounter_ixia_test.cc

@@ -19,45 +19,51 @@
 // (this file)
 //
 // TODO: Resolve whole save/restore thing wrt speed?
-//
+
 #include "tests/gnmi/ethcounter_ixia_test.h"
 
+#include <cstdint>
+#include <cwchar>
+#include <memory>
 #include <string>
+#include <utility>
+#include <vector>
 
 #include "absl/cleanup/cleanup.h"
 #include "absl/container/flat_hash_map.h"
 #include "absl/status/status.h"
-#include "absl/strings/escaping.h"
+#include "absl/status/statusor.h"
 #include "absl/strings/match.h"
+#include "absl/strings/numbers.h"
 #include "absl/strings/str_cat.h"
+#include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
 #include "absl/strings/substitute.h"
 #include "absl/time/clock.h"
 #include "absl/time/time.h"
+#include "absl/types/optional.h"
 #include "glog/logging.h"
-#include "gmock/gmock.h"
-#include "gtest/gtest.h"
 #include "gutil/collections.h"
 #include "gutil/status.h"
 #include "gutil/status_matchers.h"
 #include "gutil/testing.h"
 #include "include/nlohmann/json.hpp"
 #include "lib/gnmi/gnmi_helper.h"
 #include "lib/ixia_helper.h"
-#include "lib/p4rt/packet_listener.h"
-#include "lib/validator/validator_lib.h"
 #include "p4_pdpi/ir.h"
 #include "p4_pdpi/netaddr/mac_address.h"
 #include "p4_pdpi/p4_runtime_session.h"
-#include "p4_pdpi/packetlib/packetlib.h"
 #include "p4_pdpi/packetlib/packetlib.pb.h"
 #include "p4_pdpi/pd.h"
+#include "sai_p4/instantiations/google/sai_p4info.h"
+#include "sai_p4/instantiations/google/sai_pd.pb.h"
 #include "tests/forwarding/util.h"
 #include "tests/lib/switch_test_setup_helpers.h"
-#include "sai_p4/instantiations/google/sai_p4info.h"
 #include "thinkit/generic_testbed.h"
 #include "thinkit/proto/generic_testbed.pb.h"
 #include "thinkit/switch.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
 
 namespace pins_test {
 
@@ -395,6 +401,8 @@ absl::Status SetPortSpeed(absl::string_view port_speed, absl::string_view iface,
   return absl::OkStatus();
 }
 
+// TODO: Refactor to use gnmi_helper for stats.
+
 absl::StatusOr<uint64_t> GetGnmiStat(std::string stat_name,
                                      absl::string_view iface,
                                      gnmi::gNMI::StubInterface *gnmi_stub) {

tests/gnoi/factory_reset_test.cc

@@ -38,18 +38,26 @@
 namespace factory_reset {
 
 constexpr absl::Duration kFactoryResetWaitForDownTime = absl::Seconds(60);
+constexpr absl::Duration kPingReachabilityInterval = absl::Seconds(2);
+constexpr absl::Duration kPingTimeout = absl::Seconds(1);
+constexpr int kConsecutivePingsRequired = 3;
 constexpr absl::Duration kFactoryResetWaitForUpTime = absl::Minutes(25);
 constexpr absl::Duration kSshSessionTimeout = absl::Seconds(5);
 
 void IssueGnoiFactoryResetAndValidateStatus(
     thinkit::Switch& sut, const gnoi::factory_reset::StartRequest& request,
     gnoi::factory_reset::StartResponse* response,
     grpc::Status expected_status) {
+  LOG(INFO) << "Issuing factory reset with parameters: "
+            << request.DebugString();
   ASSERT_OK_AND_ASSIGN(auto sut_gnoi_factory_reset_stub,
                        sut.CreateGnoiFactoryResetStub());
   grpc::ClientContext context;
   grpc::Status status =
       sut_gnoi_factory_reset_stub->Start(&context, request, response);
+  LOG(INFO) << "Factory reset status: " << status.error_code() << ", "
+            << status.error_message();
+  LOG(INFO) << "Factory reset response: " << response->DebugString();
   if (expected_status.ok()) {
     EXPECT_OK(status);
   } else {
@@ -66,11 +74,15 @@ void ValidateStackState(thinkit::Switch& sut,
 
   // Wait for system to become unreachable via ping - as that's the last thing
   // that goes down.
+  LOG(INFO) << "Starting polling for unreachability, now: " << absl::Now()
+            << " deadline: " << start_time + kFactoryResetWaitForDownTime;
   while (absl::Now() < (start_time + kFactoryResetWaitForDownTime)) {
-    if (!pins_test::Pingable(sut).ok()) {
+    if (!pins_test::Pingable(sut, kPingTimeout).ok()) {
       system_down = true;
       break;
     }
+    LOG(INFO) << "System still reachable at " << absl::Now() << " sleeping";
+    absl::SleepFor(kPingReachabilityInterval);
   }
   // Return failure if system did not go down.
   ASSERT_TRUE(system_down) << "System did not go down in "
@@ -82,6 +94,8 @@ void ValidateStackState(thinkit::Switch& sut,
       system_down = false;
       break;
     }
+    LOG(INFO) << "System still unreachable, sleeping";
+    absl::SleepFor(kPingReachabilityInterval);
   }
   // Return failure if system did not come up.
   ASSERT_FALSE(system_down)
@@ -120,9 +134,23 @@ void TestGnoiFactoryResetGnoiServerUnreachableFail(
   IssueGnoiFactoryResetAndValidateStatus(sut, request, &response);
   EXPECT_TRUE(response.has_reset_success());
 
-  absl::SleepFor(kFactoryResetWaitForDownTime);
-  ASSERT_TRUE(!pins_test::Pingable(sut).ok())
-      << "System did not go down in " << kFactoryResetWaitForDownTime;
+  LOG(INFO) << "Waiting for device to become unreachable";
+  absl::Time start = absl::Now();
+  int consecutive_unreachable_count = 0;
+  while (consecutive_unreachable_count < kConsecutivePingsRequired) {
+    if (pins_test::Pingable(sut, kPingTimeout).ok()) {
+      consecutive_unreachable_count = 0;
+    } else {
+      consecutive_unreachable_count++;
+    }
+    if (absl::Now() - start > kFactoryResetWaitForDownTime) {
+      FAIL() << "System did not go down in " << kFactoryResetWaitForDownTime;
+    }
+    LOG(INFO) << "System unreachable for " << consecutive_unreachable_count
+              << " consecutive pings";
+    absl::SleepFor(kPingReachabilityInterval);
+  }
+  LOG(INFO) << "Device became unreachable after: " << absl::Now() - start;
 
   // Wait until the switch goes down, send another request and expect a failure
   // due to gNOI server unreachable.
@@ -131,6 +159,7 @@ void TestGnoiFactoryResetGnoiServerUnreachableFail(
       grpc::Status(grpc::StatusCode::UNAVAILABLE,
                    "failed to connect to all addresses"));
 
+  LOG(INFO) << "Waiting for device to become reachable";
   absl::SleepFor(kFactoryResetWaitForUpTime);
   // Return failure if system did not come up.
   ASSERT_OK(pins_test::SwitchReady(sut, interfaces))

tests/lib/p4rt_fixed_table_programming_helper.h

@@ -11,8 +11,8 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
-#ifndef GOOGLE_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_
-#define GOOGLE_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_
+#ifndef PINS_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_
+#define PINS_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_
 #include <optional>
 #include <utility>
 #include <vector>
@@ -110,4 +110,4 @@ WcmpGroupTableUpdate(const pdpi::IrP4Info &ir_p4_info,
 
 } // namespace pins
 
-#endif // GOOGLE_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_
+#endif // PINS_TESTS_LIB_P4RT_FIXED_TABLE_PROGRAMMING_HELPER_H_

tests/lib/p4rt_fixed_table_programming_helper_test.cc

@@ -13,6 +13,8 @@
 // limitations under the License.
 #include "tests/lib/p4rt_fixed_table_programming_helper.h"
 
+#include <utility>
+
 #include "absl/status/status.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -27,6 +29,7 @@ namespace {
 
 using ::gutil::StatusIs;
 using ::testing::HasSubstr;
+using ::testing::StrEq;
 
 MATCHER_P(HasExactMatch, value, "") {
   for (const auto& match_field : arg.entity().table_entry().match()) {
@@ -145,7 +148,7 @@ TEST_P(L3RouteProgrammingTest, VrfTableAddFailsWithEmptyId) {
       StatusIs(absl::StatusCode::kInvalidArgument));
 }
 
-TEST_P(L3RouteProgrammingTest, Ipv4TableDoesNotRequireAnAction) {
+TEST_P(L3RouteProgrammingTest, IpTableDoesNotRequireAnAction) {
   // The helper class will assume a default (e.g. drop).
   ASSERT_OK_AND_ASSIGN(
       p4::v1::Update pi_update,
@@ -155,7 +158,7 @@ TEST_P(L3RouteProgrammingTest, Ipv4TableDoesNotRequireAnAction) {
   EXPECT_THAT(pi_update, HasExactMatch("vrf-0"));
 }
 
-TEST_P(L3RouteProgrammingTest, Ipv4TableWithSetNexthopAction) {
+TEST_P(L3RouteProgrammingTest, IpTableWithSetNexthopAction) {
   ASSERT_OK_AND_ASSIGN(
       p4::v1::Update pi_update,
       pins::Ipv4TableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
@@ -171,7 +174,7 @@ TEST_P(L3RouteProgrammingTest, Ipv4TableWithSetNexthopAction) {
   EXPECT_THAT(pi_update, HasActionParam("nexthop-0"));
 }
 
-TEST_P(L3RouteProgrammingTest, Ipv4TableEntryFailsWihInvalidParameters) {
+TEST_P(L3RouteProgrammingTest, IpTableEntryFailsWihInvalidParameters) {
   EXPECT_THAT(
       pins::Ipv4TableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
                       pins::IpTableOptions{
@@ -183,6 +186,26 @@ TEST_P(L3RouteProgrammingTest, Ipv4TableEntryFailsWihInvalidParameters) {
                HasSubstr("Expected 0 parameters")));
 }
 
+TEST_P(L3RouteProgrammingTest, Ipv4TableEntryCannotHaveIPv6Address) {
+  EXPECT_THAT(Ipv4TableUpdate(sai::GetIrP4Info(GetParam()),
+                              p4::v1::Update::INSERT,
+                              IpTableOptions{
+                                  .dst_addr_lpm = std::make_pair("FE80::1", 32),
+                              }),
+              StatusIs(absl::StatusCode::kInvalidArgument,
+                       HasSubstr("Invalid IPv4 address")));
+}
+
+TEST_P(L3RouteProgrammingTest, Ipv6TableEntryCannotHaveIPv4Address) {
+  EXPECT_THAT(
+      Ipv6TableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
+                      IpTableOptions{
+                          .dst_addr_lpm = std::make_pair("10.1.1.1", 32),
+                      }),
+      StatusIs(absl::StatusCode::kInvalidArgument,
+               HasSubstr("invalid IPv6 address")));
+}
+
 TEST_P(L3RouteProgrammingTest, L3AdmitTableWithoutInPort) {
   ASSERT_OK_AND_ASSIGN(
       p4::v1::Update pi_update,
@@ -197,6 +220,13 @@ TEST_P(L3RouteProgrammingTest, L3AdmitTableWithoutInPort) {
                                          "\377\377\377\377\377\377"));
 }
 
+TEST_P(L3RouteProgrammingTest, L3AdmitTableAllPacketsDoesNotSetAMatchKey) {
+  ASSERT_OK_AND_ASSIGN(p4::v1::Update pi_update,
+                       L3AdmitAllTableUpdate(sai::GetIrP4Info(GetParam()),
+                                             p4::v1::Update::INSERT));
+  EXPECT_TRUE(pi_update.entity().table_entry().match().empty());
+}
+
 TEST_P(L3RouteProgrammingTest, L3AdmitTableWithInPort) {
   ASSERT_OK_AND_ASSIGN(
       p4::v1::Update pi_update,
@@ -224,6 +254,57 @@ TEST_P(L3RouteProgrammingTest, L3AdmitTableMustSetPriority) {
                HasSubstr("require a positive non-zero priority")));
 }
 
+TEST_P(L3RouteProgrammingTest, WcmpGroupCanHaveMultipleNextHops) {
+  ASSERT_OK_AND_ASSIGN(
+      p4::v1::Update pi_update,
+      WcmpGroupTableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
+                           /*wcmp_group_id=*/"group-1",
+                           {WcmpAction{.nexthop_id = "nh-2", .weight = 1},
+                            WcmpAction{.nexthop_id = "nh-3", .weight = 2}}));
+
+  EXPECT_THAT(pi_update, HasExactMatch("group-1"));
+  EXPECT_EQ(pi_update.entity()
+                .table_entry()
+                .action()
+                .action_profile_action_set()
+                .action_profile_actions_size(),
+            2);
+}
+
+TEST_P(L3RouteProgrammingTest, WcmpGroupActionCannotHaveWeightZero) {
+  EXPECT_THAT(
+      WcmpGroupTableUpdate(sai::GetIrP4Info(GetParam()), p4::v1::Update::INSERT,
+                           /*wcmp_group_id=*/"group-1",
+                           {WcmpAction{.nexthop_id = "nh-3", .weight = 0}}),
+      StatusIs(absl::StatusCode::kInvalidArgument,
+               HasSubstr("Expected positive action set weight")));
+}
+
+TEST_P(L3RouteProgrammingTest, WcmpGroupActionCanSetWatchPort) {
+  ASSERT_OK_AND_ASSIGN(p4::v1::Update pi_update,
+                       WcmpGroupTableUpdate(sai::GetIrP4Info(GetParam()),
+                                            p4::v1::Update::INSERT,
+                                            /*wcmp_group_id=*/"group-1",
+                                            {WcmpAction{
+                                                .nexthop_id = "nh-3",
+                                                .weight = 2,
+                                                .watch_port = "1",
+                                            }}));
+  ASSERT_EQ(pi_update.entity()
+                .table_entry()
+                .action()
+                .action_profile_action_set()
+                .action_profile_actions_size(),
+            1);
+  EXPECT_THAT(pi_update.entity()
+                  .table_entry()
+                  .action()
+                  .action_profile_action_set()
+                  .action_profile_actions(0)
+                  .watch_port(),
+              StrEq("1"));
+}
+
 INSTANTIATE_TEST_SUITE_P(
     L3RouteProgrammingTestInstance, L3RouteProgrammingTest,
     testing::Values(sai::Instantiation::kMiddleblock,