[P4RT] Update P4RT app schema to support new multicast tables, Correc…

…t the description of P4RuntimeComponentTestFixture::p4rt_service_, Use expected `HEX_STRING` format for `multicast_replica_instance` & Add packet dump for dropped PacketIn's. (sonic-net#476) Co-authored-by: kishanps <kishanps@google.com>
divyagayathri-hcl · Aug 20, 2024 · 49ea02c · 49ea02c
1 parent 50991f9
commit 49ea02c
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 23 deletions.
diff --git a/p4rt_app/p4runtime/p4info_verification_schema.pb.txt b/p4rt_app/p4runtime/p4info_verification_schema.pb.txt
@@ -7,14 +7,6 @@ tables {
     name: "set_metadata_and_drop"
     parameters { name: "route_metadata" bitwidth: 6 }
   }
-  actions {
-    name: "set_multicast_group_id"
-    parameters {
-      name: "multicast_group_id"
-      format: HEX_STRING
-      bitwidth: 16
-    }
-  }
   actions {
     name: "set_nexthop_id"
     parameters { name: "nexthop_id" format: STRING }
@@ -44,14 +36,6 @@ tables {
     name: "set_metadata_and_drop"
     parameters { name: "route_metadata" bitwidth: 6 }
   }
-  actions {
-    name: "set_multicast_group_id"
-    parameters {
-      name: "multicast_group_id"
-      format: HEX_STRING
-      bitwidth: 16
-    }
-  }
   actions {
     name: "set_nexthop_id"
     parameters { name: "nexthop_id" format: STRING }
@@ -103,7 +87,12 @@ tables {
 tables {
   name: "multicast_router_interface_table"
   match_fields { name: "multicast_replica_port" format: STRING type: EXACT }
-  match_fields { name: "multicast_replica_instance" format: STRING type: EXACT }
+  match_fields {
+    name: "multicast_replica_instance"
+    format: HEX_STRING
+    bitwidth: 16
+    type: EXACT
+  }
   actions {
     name: "set_multicast_src_mac"
     parameters { name: "src_mac" format: MAC }
@@ -197,3 +186,29 @@ tables {
   }
   actions { name: "disable_vlan_checks" }
 }
+tables {
+  name: "ipv4_multicast_table"
+  match_fields { name: "ipv4_dst" format: IPV4 type: EXACT }
+  match_fields { name: "vrf_id" format: STRING type: EXACT }
+  actions {
+    name: "set_multicast_group_id"
+    parameters {
+      name: "multicast_group_id"
+      format: HEX_STRING
+      bitwidth: 16
+    }
+  }
+}
+tables {
+  name: "ipv6_multicast_table"
+  match_fields { name: "ipv6_dst" format: IPV6 type: EXACT }
+  match_fields { name: "vrf_id" format: STRING type: EXACT }
+  actions {
+    name: "set_multicast_group_id"
+    parameters {
+      name: "multicast_group_id"
+      format: HEX_STRING
+      bitwidth: 16
+    }
+  }
+}
diff --git a/p4rt_app/p4runtime/p4runtime_impl.cc b/p4rt_app/p4runtime/p4runtime_impl.cc
@@ -13,6 +13,7 @@
 // limitations under the License.
 #include "p4rt_app/p4runtime/p4runtime_impl.h"
 
+#include <algorithm>
 #include <cstdint>
 #include <memory>
 #include <optional>
@@ -28,6 +29,7 @@
 #include "absl/memory/memory.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
+#include "absl/strings/escaping.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_format.h"
 #include "absl/strings/str_join.h"
@@ -1513,15 +1515,15 @@ absl::StatusOr<std::thread> P4RuntimeImpl::StartReceive(
         return gutil::StatusBuilder(port_id_or.status())
                << "Could not send PacketIn request because of bad source port "
                   "name."
-               << port_id_or.status().message();
+               << port_id_or.status().message() << "Packet(hex): "
+               << absl::BytesToHexString(payload).substr(
+                      0, std::min<int>(payload.size(), 100));
       }
       source_port_id = *port_id_or;
     } else {
       source_port_id = sonic_source_port_name;
     }
 
-    // TODO: Until string port names are supported, re-assign empty
-    // target egress port names to match the ingress port.
     std::string target_port_id = source_port_id;
     if (!sonic_target_port_name.empty()) {
       if (translate_port_ids_) {
@@ -1533,7 +1535,9 @@ absl::StatusOr<std::thread> P4RuntimeImpl::StartReceive(
           return gutil::StatusBuilder(port_id_or.status())
                  << "Could not send PacketIn request because of bad target "
                     "port name."
-                 << port_id_or.status().message();
+                 << port_id_or.status().message() << "Packet(hex): "
+                 << absl::BytesToHexString(payload).substr(
+                        0, std::min<int>(payload.size(), 100));
         }
         target_port_id = *port_id_or;
       } else {

diff --git a/p4rt_app/sonic/receive_genetlink.cc b/p4rt_app/sonic/receive_genetlink.cc
@@ -204,7 +204,8 @@ absl::Status ProcessReceiveMessage(struct nl_msg* msg, void* arg) {
 int ProcessReceiveMessageCallback(struct nl_msg* msg, void* arg) {
   auto status = ProcessReceiveMessage(msg, arg);
   if (!status.ok()) {
-    LOG(WARNING) << "Failed to process receive packet: " << status.ToString();
+    LOG_EVERY_T(WARNING, 5)
+        << "Failed to process receive packet: " << status.ToString();
   }
   // Always return OK to move on to the next packet.
   return NL_OK;

diff --git a/p4rt_app/tests/lib/p4runtime_component_test_fixture.h b/p4rt_app/tests/lib/p4runtime_component_test_fixture.h
@@ -47,7 +47,7 @@ class P4RuntimeComponentTestFixture : public testing::Test {
   const p4::config::v1::P4Info p4_info_;
   const pdpi::IrP4Info ir_p4_info_;
 
-  // Bring up a fake P4Runtime gRPC server to run tests against.
+  // A testable P4Runtime gRPC server with fake DB connections
   P4RuntimeGrpcService p4rt_service_;
 
   // The P4RT gRPC client session tests will use to connect to the fake