sonic-net · kishanps · Feb 24, 2026 · Feb 15, 2025
diff --git a/p4_infra/p4_pdpi/ir.cc b/p4_infra/p4_pdpi/ir.cc
@@ -822,7 +822,7 @@ StatusOr<O> PiPacketIoToIr(const IrP4Info &info, const std::string &kind,
         **status_or_metadata_definition_ptr;
 
     // Metadata with @padding annotation must be all zeros and must not be
-    // included in IR representation (go/pdpi-padding).
+    // included in IR representation.
     if (metadata_definition.is_padding()) {
       if (!IsAllZeros(metadata.value())) {
         invalid_reasons.push_back(absl::StrCat(
@@ -1208,7 +1208,7 @@ absl::Status IrActionSetToPi(
 }
 
 // Creates a piece of padding metadata. Metadata with the @padding annotation
-// must contain a zero value bytestring (go/pdpi-padding).
+// must contain a zero value bytestring.
 p4::v1::PacketMetadata CreatePaddingMetadata(uint32_t metadata_id) {
   p4::v1::PacketMetadata metadata;
   metadata.set_metadata_id(metadata_id);
@@ -1259,7 +1259,7 @@ StatusOr<I> IrPacketIoToPi(const IrP4Info &info, const std::string &kind,
         **status_or_metadata_definition_ptr;
 
     // Metadata with @padding annotation must not be present in IR
-    // representation (go/pdpi-padding).
+    // representation.
     if (metadata_definition.is_padding()) {
       invalid_reasons.push_back(absl::StrCat(
           kNewBullet, "Metadata '", metadata_definition.metadata().name(),
@@ -1294,7 +1294,7 @@ StatusOr<I> IrPacketIoToPi(const IrP4Info &info, const std::string &kind,
   }
   // Check for missing metadata
   for (const auto &[name, metadata] : metadata_by_name) {
-    // Insert padding metadata into PI representation (go/pdpi-padding).
+    // Insert padding metadata into PI representation.
     if (metadata.is_padding()) {
       *result.add_metadata() = CreatePaddingMetadata(metadata.metadata().id());
     } else if (!used_metadata_names.contains(name)) {

diff --git a/p4_infra/p4_pdpi/ir.proto b/p4_infra/p4_pdpi/ir.proto
@@ -207,7 +207,6 @@ message IrMatchFieldDefinition {
   repeated IrMatchFieldReference references = 3 [deprecated = true];
   // True if match field has @unsupported annotation, otherwise false. Indicates
   // that the match field is not (yet) supported by the target.
-  // See go/unblocking-sai-p4 for more details.
   bool is_unsupported = 14;
 }
 
@@ -260,7 +259,6 @@ message IrTableDefinition {
   string role = 12;
   // True if table has @unsupported annotation, otherwise false. Indicates that
   // the table is not (yet) supported by the target.
-  // See go/unblocking-sai-p4 for more details.
   bool is_unsupported = 14;
   // `IrTableReference` where this table is the `source_table`.
   repeated IrTableReference outgoing_references = 15;
@@ -301,7 +299,6 @@ message IrActionDefinition {
   map<string, IrActionParamDefinition> params_by_name = 3;
   // True if action has @unsupported annotation, otherwise false. Indicates that
   // the action is not (yet) supported by the target.
-  // See go/unblocking-sai-p4 for more details.
   bool is_unsupported = 4;
 }
 
@@ -318,9 +315,9 @@ message IrPacketIoMetadataDefinition {
   // Required, the format of this parameter as deduced from the parameter type
   // and annotations.
   Format format = 2;
-  // True if metadata field has @padding (go/pdpi-padding) annotation present,
-  // otherwise false. Indicates that metadata is excluded from IR and PD proto,
-  // and zero-valued in PI proto.
+  // True if metadata field has @padding annotation present, otherwise false.
+  // Indicates that metadata is excluded from IR and PD proto, and zero-valued
+  // in PI proto.
   bool is_padding = 3;
 }
 

diff --git a/p4_infra/p4_pdpi/p4_runtime_session.cc b/p4_infra/p4_pdpi/p4_runtime_session.cc
@@ -141,7 +141,7 @@ absl::StatusOr<std::unique_ptr<P4RuntimeSession>> P4RuntimeSession::Create(
     return gutil::InternalErrorBuilder() << "Received device id doesn't match: "
                                          << response.ShortDebugString();
   }
-  //TODO Enable this check once p4rt app supports role.
+  // TODO: Enable this check once p4rt app supports role.
   // if (response.arbitration().role().name() != session->role_) {
   //   return gutil::InternalErrorBuilder() << "Received role doesn't match: "
   //                                        << response.ShortDebugString();
@@ -163,7 +163,6 @@ absl::StatusOr<std::unique_ptr<P4RuntimeSession>> P4RuntimeSession::Create(
   // won't implicitly wrap the return expressions in std::move(). Then, the case
   // here will trigger the copy of the unique_ptr, which is invalid. Thus, we
   // need to explicitly std::move the returned object here.
-  // See:go/totw/labs/should-i-return-std-move.
   return std::move(session);
 }
 
@@ -645,8 +644,8 @@ absl::Status ClearEntities(
   absl::c_reverse(entities);
 
   // Get current switch version to determine if we need to mask old errors.
-  // TODO: Remove version check when the P4Info version in release is
-  // equal or higher than SAI_P4_PKGINFO_VERSION_USES_FAIL_ON_FIRST. Almost
+  // TODO: Remove version check when the P4Info version in release
+  // is equal or higher than SAI_P4_PKGINFO_VERSION_USES_FAIL_ON_FIRST. Almost
   // certainly safe to remove by April 2024.
   ASSIGN_OR_RETURN(
       gutil::Version current_version,
@@ -661,7 +660,6 @@ absl::Status ClearEntities(
         << absl::StrJoin(entities, "\n");
   } else {
     // Ideally, whether to use batches or not should be determined by a P4Info
-    // option that will be introduced in b/259194587.
     RETURN_IF_ERROR(SplitSortedUpdatesIntoBatchesAndSend(
         session, info, CreatePiUpdates(entities, Update::DELETE)))
         << "when attempting to delete the following entities: "

diff --git a/p4_infra/p4_pdpi/packetlib/BUILD.bazel b/p4_infra/p4_pdpi/packetlib/BUILD.bazel
@@ -98,7 +98,6 @@ cc_test(
     ],
 )
 
-# go/golden-test-with-coverage
 cc_test(
     name = "packetlib_test_runner",
     srcs = ["packetlib_test_runner.cc"],

diff --git a/p4_infra/p4_pdpi/packetlib/packetlib.cc b/p4_infra/p4_pdpi/packetlib/packetlib.cc
@@ -1790,13 +1790,15 @@ void PspHeaderInvalidReasons(const PspHeader& header,
                                   "reserved0: Must be 0x0, but was ",
                                   header.reserved0(), " instead."));
   }
-  bool reserved1_invalid = HexStringInvalidReasons<kPspReserved1Bitwidth>(
-      header.reserved1(), absl::StrCat(field_prefix, "reserved1"), output);
-  if (!reserved1_invalid && header.reserved1() != "0x1") {
-    output.push_back(absl::StrCat(field_prefix,
-                                  "reserved1: Must be 0x1, but was ",
-                                  header.reserved1(), " instead."));
-  }
+
+  // TODO: Enable once DVaaS handles invalid packets correctly.
+  // bool reserved1_invalid = HexStringInvalidReasons<kPspReserved1Bitwidth>(
+  //     header.reserved1(), absl::StrCat(field_prefix, "reserved1"), output);
+  // if (!reserved1_invalid && header.reserved1() != "0x1") {
+  //   output.push_back(absl::StrCat(field_prefix,
+  //                                 "reserved1: Must be 0x1, but was ",
+  //                                 header.reserved1(), " instead."));
+  // }
 }
 
 } //namespace

diff --git a/p4_infra/p4_pdpi/packetlib/packetlib_test_runner.expected b/p4_infra/p4_pdpi/packetlib/packetlib_test_runner.expected
@@ -5667,12 +5667,10 @@ payload: "ABCDABCDABCDABCDABCD"
 -- OUTPUT ----------------------------------------------------------------------
 ValidatePacket(packet) = INVALID_ARGUMENT: Packet invalid for the following reasons:
 - in PspHeader headers[3]: reserved0: Must be 0x0, but was 0x1 instead.
-- in PspHeader headers[3]: reserved1: Must be 0x1, but was 0x0 instead.
 
 PadPacketToMinimumSize(packet) = false
 
 UpdateMissingComputedFields(packet) = false
 
 Serialize(Packet) = INVALID_ARGUMENT: Packet invalid for the following reasons:
 - in PspHeader headers[3]: reserved0: Must be 0x0, but was 0x1 instead.
-- in PspHeader headers[3]: reserved1: Must be 0x1, but was 0x0 instead.
diff --git a/p4_infra/p4_pdpi/pd.cc b/p4_infra/p4_pdpi/pd.cc
@@ -1352,7 +1352,6 @@ absl::Status IrPacketIoToPd(const IrP4Info &info, const std::string &kind,
     const pdpi::IrPacketIoMetadataDefinition &metadata_definition =
         **status_or_metadata_definition;
 
-    // See go/pdpi-padding.
     if (metadata_definition.is_padding()) {
       invalid_reasons.push_back(absl::StrCat(
           kNewBullet, "Metadata with name '", name,
@@ -2646,7 +2645,7 @@ absl::StatusOr<T> PdPacketIoToIr(const IrP4Info &info, const std::string &kind,
   }
   std::vector<std::string> invalid_reasons;
   for (const auto &[name, metadata] : Ordered(metadata_by_name)) {
-    // Skip metadata with @padding annotation (go/pdpi-padding).
+    // Skip metadata with @padding annotation.
     if (metadata.is_padding()) continue;
 
     const absl::StatusOr<std::string> &pd_entry =

diff --git a/p4_infra/p4_pdpi/pdgen.cc b/p4_infra/p4_pdpi/pdgen.cc
@@ -16,6 +16,7 @@
 
 #include <cstdint>
 #include <iostream>
+#include <optional>
 #include <string>
 
 #include "absl/flags/flag.h"
@@ -42,7 +43,7 @@ ABSL_FLAG(
     "Optional field number used for multicast_group_table_entry in TableEntry "
     "oneof. If set to nullopt, then multicast_group_table_entry is omitted "
     "from PD proto. Defaults to 2047 to avoid conflicts with other tables and "
-    "remain small go/fast/11#small-field-number");
+    "remain small");
 
 constexpr char kUsage[] =
     "--p4info=<file> --package=<package> --roles=<comma-separated-role-list>";

diff --git a/p4_infra/p4_pdpi/sequencing.cc b/p4_infra/p4_pdpi/sequencing.cc
@@ -502,7 +502,7 @@ absl::StatusOr<std::vector<p4::v1::Entity>> GetEntitiesUnreachableFromRoots(
   std::vector<p4::v1::Entity> unreachable_entities;
   unreachable_entities.reserve(unreachable_indices.size());
 
-  // TODO: verios - remove sort once tests are updated to handle
+  // TODO: Remove sort once tests are updated to handle
   // non-determinism.
   std::vector<int> sorted_unreachable_indices(unreachable_indices.begin(),
                                               unreachable_indices.end());

diff --git a/p4_infra/p4_pdpi/sequencing.h b/p4_infra/p4_pdpi/sequencing.h
@@ -30,9 +30,8 @@
 
 namespace pdpi {
 
-// Sequencing functionality, see go/p4-sequencing for details. All APIs are
-// stable, i.e. for entries without constraint that could appear in any order,
-// the input order is preserved.
+// Sequencing functionality. All APIs are stable, i.e. for entries without
+// constraint that could appear in any order, the input order is preserved.
 
 // Returns a list of write requests, such that updates are sequenced correctly
 // when the write requests are sent in order. Order within a write request is

diff --git a/p4_infra/p4_pdpi/string_encodings/BUILD.bazel b/p4_infra/p4_pdpi/string_encodings/BUILD.bazel
@@ -100,7 +100,6 @@ cc_library(
     ],
 )
 
-# go/golden-test-with-coverage
 cc_test(
     name = "hex_string_test_runner",
     srcs = ["hex_string_test_runner.cc"],
@@ -132,7 +131,6 @@ cc_library(
     ],
 )
 
-# go/golden-test-with-coverage
 cc_test(
     name = "decimal_string_test_runner",
     srcs = ["decimal_string_test_runner.cc"],

diff --git a/p4_infra/p4_pdpi/testing/BUILD.bazel b/p4_infra/p4_pdpi/testing/BUILD.bazel
@@ -82,12 +82,10 @@ cc_library(
         "@com_github_p4lang_p4runtime//:p4info_cc_proto",
         "@com_google_absl//absl/log:check",
         "@com_google_absl//absl/status:statusor",
-        "@com_google_absl//absl/strings",
         "@com_google_protobuf//:protobuf",
     ],
 )
 
-# Auxiliary target, see go/totw/128.
 cc_embed_data(
     name = "test_p4info_embed",
     srcs = ["main-p4info.pb.txt"],
@@ -148,7 +146,6 @@ cc_library(
     ],
 )
 
-# go/golden-test-with-coverage
 cc_test(
     name = "sequencing_test_runner",
     srcs = ["sequencing_test_runner.cc"],
@@ -242,7 +239,6 @@ cmd_diff_test(
     tools = [":references_test_runner"],
 )
 
-# go/golden-test-with-coverage
 cc_test(
     name = "info_test_runner",
     srcs = ["info_test_runner.cc"],
@@ -275,7 +271,6 @@ cmd_diff_test(
     ],
 )
 
-# go/golden-test-with-coverage
 cc_test(
     name = "rpc_test_runner",
     srcs = ["rpc_test_runner.cc"],
@@ -315,7 +310,6 @@ cmd_diff_test(
     ],
 )
 
-# go/golden-test-with-coverage
 cc_test(
     name = "packet_io_test_runner",
     srcs = ["packet_io_test_runner.cc"],
@@ -349,7 +343,6 @@ cmd_diff_test(
     ],
 )
 
-# go/golden-test-with-coverage
 cc_test(
     name = "table_entry_test_runner",
     srcs = ["table_entry_test_runner.cc"],

diff --git a/p4_infra/p4_pdpi/testing/test_p4info.cc b/p4_infra/p4_pdpi/testing/test_p4info.cc
@@ -13,9 +13,11 @@
 // limitations under the License.
 #include "p4_infra/p4_pdpi/testing/test_p4info.h"
 
+#include <string>
+#include <utility>
+
 #include "absl/log/check.h"
 #include "absl/status/statusor.h"
-#include "absl/strings/string_view.h"
 #include "google/protobuf/text_format.h"
 #include "p4/config/v1/p4info.pb.h"
 #include "p4_infra/p4_pdpi/ir.h"
@@ -26,12 +28,10 @@ namespace pdpi {
 
 using ::google::protobuf::TextFormat;
 using ::gutil::FileToc;
-using p4::config::v1::P4Info;
-using pdpi::IrP4Info;
-
-// Adapted from go/totw/128.
+using ::p4::config::v1::P4Info;
+using ::pdpi::IrP4Info;
 const P4Info& GetTestP4Info() {
-  // Safe static object initialization following go/totw/110.
+  // Safe static object initialization.
   static const P4Info* info = [] {
     const FileToc* const toc = test_p4info_embed_create();
     std::string data(toc[0].data, toc[0].size);
@@ -45,7 +45,7 @@ const P4Info& GetTestP4Info() {
 }
 
 const IrP4Info& GetTestIrP4Info() {
-  // Safe static object initialization following go/totw/110.
+  // Safe static object initialization.
   static const IrP4Info* info = [] {
     absl::StatusOr<IrP4Info> ir_p4info = pdpi::CreateIrP4Info(GetTestP4Info());
     CHECK(ir_p4info.status().ok())  // Crash ok: TAP rules out failures.

diff --git a/p4_infra/p4_pdpi/testing/testdata/main.p4 b/p4_infra/p4_pdpi/testing/testdata/main.p4
@@ -278,7 +278,7 @@ control ingress(inout headers hdr, inout metadata meta, inout standard_metadata_
   }
 
   // Action that refers to both fields of two_match_fields_table.
-  // TODO Add double reference.
+  // TODO: Add double reference.
   action referring_to_two_match_fields_action(@id(1)
   @refers_to(two_match_fields_table, id_1)
                          string_id_t referring_id_1,

diff --git a/p4_infra/p4_pdpi/utils/ir.cc b/p4_infra/p4_pdpi/utils/ir.cc
@@ -67,6 +67,7 @@ using ::pdpi::IrValue;
 
 absl::StatusOr<std::string> ArbitraryToNormalizedByteString(
     const std::string &bytes, int expected_bitwidth) {
+  // If the bytestring length is zero, the server always rejects the string.
   if (bytes.empty()) {
     return gutil::OutOfRangeErrorBuilder()
            << "Bytestrings must have non-zero length.";
@@ -187,6 +188,7 @@ absl::StatusOr<Format> GetFormat(const std::vector<std::string> &annotations,
 absl::StatusOr<IrValue> ArbitraryByteStringToIrValue(Format format,
                                                      const int bitwidth,
                                                      const std::string &bytes) {
+  // If the bytestring length is zero, the server always rejects the string.
   if (bytes.empty()) {
     return gutil::OutOfRangeErrorBuilder()
            << "Bytestrings must have non-zero length.";
@@ -326,6 +328,7 @@ absl::StatusOr<std::string> IrValueToNormalizedByteString(
       return ipv6.ToPaddedByteString();
     }
     case IrValue::kStr: {
+      // If the bytestring length is zero, the server always rejects the string.
       if (ir_value.str().empty()) {
         return gutil::OutOfRangeErrorBuilder()
                << "Bytestrings must have non-zero length.";

diff --git a/p4rt_app/scripts/BUILD.bazel b/p4rt_app/scripts/BUILD.bazel
@@ -25,7 +25,7 @@ cc_library(
     hdrs = ["p4rt_tool_helpers.h"],
     deps = [
         "//p4_infra/p4_pdpi:p4_runtime_session",
-	"@com_github_grpc_grpc//:grpc++",
+        "@com_github_grpc_grpc//:grpc++",
         "@com_google_absl//absl/flags:flag",
         "@com_google_absl//absl/status:statusor",
         "@com_google_absl//absl/strings",