Remove VALID match type from P4Runtime (#368)

* Remove VALID match type from P4Runtime

We remove the VALID match type from p4runtime.proto and
p4info.proto. The valid match type does not exist in P4_16, which means
that the p4c compiler never produces a VALID match type in P4Info even
for P4_14 programs, since all P4_14 programs are converted to P4_16
before P4Info is generated.

While the change is not backward-compatible since we are deleting
Protobuf fields, it should not impact existing Protobuf binaries that do
not use VALID, as it does not affect other Protobuf fields (we did not
change any of the Protobuf integer field values).

The VALID match type is preserved in the PI implementation for now, to
ensure backward-compatibility with the bmv2 JSON produced by the legacy
compiler (p4c-bm).

Fixes #367

* Added more comments to the code as requested by reviewer

CLI/table_common.c

@@ -81,9 +81,34 @@ static int match_key_add_valid_field(pi_p4_id_t t_id, pi_p4_id_t f_id,
   return 0;
 }
 
+// P4_16 and p4c do not have a VALID match type (h.isValid() : exact), but we
+// want to be able to support 'true' and 'false' in the CLI when matching on a
+// header validity (h.isValid()), for exact and ternary matches.
+static int match_key_try_add_valid_field(pi_p4_id_t t_id, pi_p4_id_t f_id,
+                                         size_t f_bitwidth, char *mf,
+                                         pi_match_key_t *mk) {
+  if (f_bitwidth > 8) return 1;
+  int v;
+  if (!strncasecmp("true", mf, sizeof("true"))) {
+    v = 1;
+  } else if (!strncasecmp("false", mf, sizeof("false"))) {
+    v = 0;
+  } else {
+    return 1;
+  }
+  pi_netv_t f_netv;
+  pi_status_t rc;
+  rc = pi_getnetv_u8(p4info_curr, t_id, f_id, (uint8_t)v, &f_netv);
+  assert(rc == PI_STATUS_SUCCESS);
+  rc = pi_match_key_exact_set(mk, &f_netv);
+  assert(rc == PI_STATUS_SUCCESS);
+  return 0;
+}
+
 static int match_key_add_exact_field(pi_p4_id_t t_id, pi_p4_id_t f_id,
                                      size_t f_bitwidth, char *mf,
                                      pi_match_key_t *mk) {
+  if (!match_key_try_add_valid_field(t_id, f_id, f_bitwidth, mf, mk)) return 0;
   char bytes[BYTES_TEMP_SIZE];
   if (param_to_bytes(mf, bytes, f_bitwidth)) return 1;
   pi_netv_t f_netv;
@@ -114,6 +139,7 @@ static int match_key_add_LPM_field(pi_p4_id_t t_id, pi_p4_id_t f_id,
 static int match_key_add_ternary_field(pi_p4_id_t t_id, pi_p4_id_t f_id,
                                        size_t f_bitwidth, char *mf, char *mask,
                                        pi_match_key_t *mk) {
+  if (!match_key_try_add_valid_field(t_id, f_id, f_bitwidth, mf, mk)) return 0;
   char bytes[BYTES_TEMP_SIZE];
   char mask_bytes[BYTES_TEMP_SIZE];
   pi_status_t rc;

include/PI/p4info/tables.h

@@ -31,6 +31,9 @@ extern "C" {
 #endif
 
 typedef enum {
+  /* The VALID match type has been deprecated from P4Runtime. However, for
+     backward-compatibility with the bmv2 JSON produced by the legacy compiler
+     (p4c-bm), we keep the VALID match type around in PI for now */
   PI_P4INFO_MATCH_TYPE_VALID = 0,
   PI_P4INFO_MATCH_TYPE_EXACT,
   PI_P4INFO_MATCH_TYPE_LPM,

proto/frontend/src/device_mgr.cpp

@@ -637,12 +637,19 @@ class DeviceMgrImp {
       auto mf = entry->add_match();
       mf->set_field_id(finfo->mf_id);
       switch (finfo->match_type) {
+        // For backward-compatibility with the old workflow (P4_14 program ---
+        // p4c-bm compiler ---> bmv2 JSON --- converter ---> P4Info), we still
+        // support PI_P4INFO_MATCH_TYPE_VALID. The P4_14 valid match type will
+        // show up as exact in the P4Info, which is why we set the exact field
+        // in the P4Runtime message (to '\x01' for valid and '\x00' for
+        // invalid).
         case PI_P4INFO_MATCH_TYPE_VALID:
           {
-            auto valid = mf->mutable_valid();
+            auto exact = mf->mutable_exact();
             bool value;
             mk_reader.get_valid(finfo->mf_id, &value);
-            valid->set_value(value);
+            exact->set_value(
+                value ? std::string("\x01", 1) : std::string("\x00", 1));
           }
           break;
         case PI_P4INFO_MATCH_TYPE_EXACT:
@@ -1569,10 +1576,11 @@ class DeviceMgrImp {
       num_mf_matched++;
       auto bitwidth = mf_info->bitwidth;
       switch (mf_info->match_type) {
+        // For backward-compatibility with old workflow. A P4_14 valid match
+        // type is replaced by an exact match in the P4Info, which is why we
+        // check that the P4Runtime message includes an exact field in that
+        // case.
         case PI_P4INFO_MATCH_TYPE_VALID:
-          if (!mf->has_valid())
-            RETURN_ERROR_STATUS(Code::INVALID_ARGUMENT, "Invalid match type");
-          break;
         case PI_P4INFO_MATCH_TYPE_EXACT:
           if (!mf->has_exact())
             RETURN_ERROR_STATUS(Code::INVALID_ARGUMENT, "Invalid match type");
@@ -1638,8 +1646,13 @@ class DeviceMgrImp {
       auto mf = find_mf(entry, mf_id);
       if (mf != nullptr) {
         switch (mf_info->match_type) {
+          // For backward-compatibility with old workflow. A P4_14 valid match
+          // type is replaced by an exact match in the P4Info, which is why we
+          // read the value from the exact field in the P4Runtime message
+          // ('\x00' means invalid and every other value means valid).
           case PI_P4INFO_MATCH_TYPE_VALID:
-            match_key->set_valid(mf_id, mf->valid().value());
+            match_key->set_valid(mf_id,
+                                 mf->exact().value() != std::string("\x00", 1));
             break;
           case PI_P4INFO_MATCH_TYPE_EXACT:
             match_key->set_exact(mf_id, mf->exact().value().data(),

proto/p4/config/v1/p4info.proto

@@ -156,7 +156,6 @@ message MatchField {
   int32 bitwidth = 4;
   enum MatchType {
     UNSPECIFIED = 0;
-    VALID = 1;
     EXACT = 2;
     LPM = 3;
     TERNARY = 4;

proto/p4/v1/p4runtime.proto

@@ -182,7 +182,7 @@ message TableEntry {
   IdleTimeout time_since_last_hit = 10;
 }
 
-// field_match_type ::= exact | ternary | lpm | range | valid
+// field_match_type ::= exact | ternary | lpm | range
 message FieldMatch {
   uint32 field_id = 1;
 
@@ -205,16 +205,12 @@ message FieldMatch {
     bytes low = 1;
     bytes high = 2;
   }
-  message Valid {
-    bool value = 1;
-  }
 
   oneof field_match_type {
     Exact exact = 2;
     Ternary ternary = 3;
     LPM lpm = 4;
     Range range = 6;
-    Valid valid = 7;
   }
 }
 

proto/p4info/p4info_to_and_from_proto.cpp

@@ -96,8 +96,6 @@ void read_tables(const p4configv1::P4Info &p4info_proto, pi_p4info_t *p4info) {
     for (const auto &mf : table.match_fields()) {
       auto match_type_convert = [&mf]() {
         switch (mf.match_type()) {
-          case p4configv1::MatchField_MatchType_VALID:
-            return PI_P4INFO_MATCH_TYPE_VALID;
           case p4configv1::MatchField_MatchType_EXACT:
             return PI_P4INFO_MATCH_TYPE_EXACT;
           case p4configv1::MatchField_MatchType_LPM:
@@ -316,8 +314,12 @@ void p4info_serialize_tables(const pi_p4info_t *p4info,
       mf->set_id(mf_id);
       auto match_type_convert = [info]() {
         switch (info->match_type) {
+          // A P4_14 valid match type is replaced by an exact match in the
+          // P4Info, since P4Runtime no longer supports the valid matches, which
+          // no longer exist in P4_16. The new p4c compiler always translates
+          // P4_14 programs to P4_16 before generating P4Info, which replaces
+          // all valid matches with exact matches on the validity bit.
           case PI_P4INFO_MATCH_TYPE_VALID:
-            return p4configv1::MatchField_MatchType_VALID;
           case PI_P4INFO_MATCH_TYPE_EXACT:
             return p4configv1::MatchField_MatchType_EXACT;
           case PI_P4INFO_MATCH_TYPE_LPM: