Add an explicit optional match key type.

p4lang · fruffy · Mar 14, 2023 · Mar 7, 2023 · Mar 13, 2023 · Mar 13, 2023
commit 896b204c4354f8b69b609df358103c4234d2378e
diff --git a/backends/p4tools/modules/testgen/lib/test_spec.cpp b/backends/p4tools/modules/testgen/lib/test_spec.cpp
@@ -214,6 +214,27 @@ const Exact *Exact::evaluate(const Model &model) const {
 
 cstring Exact::getObjectName() const { return "Exact"; }
 
+Optional::Optional(const IR::KeyElement *key, const IR::Expression *val, bool addMatch)
+    : TableMatch(key), value(val), addMatch(addMatch) {}
+
+const IR::Constant *Optional::getEvaluatedValue() const {
+    const auto *constant = value->to<IR::Constant>();
+    BUG_CHECK(constant,
+              "Variable is not a constant. It has type %1% instead. Has the test object %2% "
+              "been evaluated?",
+              value->type->node_type_name(), getObjectName());
+    return constant;
+}
+
+const Optional *Optional::evaluate(const Model &model) const {
+    const auto *evaluatedValue = model.evaluate(value);
+    return new Optional(getKey(), evaluatedValue, addMatch);
+}
+
+cstring Optional::getObjectName() const { return "Optional"; }
+
+bool Optional::addAsExactMatch() const { return addMatch; }
+
 TableRule::TableRule(std::map<cstring, const FieldMatch> matches, int priority, ActionCall action,
                      int ttl)
     : matches(std::move(matches)), priority(priority), action(std::move(action)), ttl(ttl) {}

diff --git a/backends/p4tools/modules/testgen/lib/test_spec.h b/backends/p4tools/modules/testgen/lib/test_spec.h
@@ -244,7 +244,30 @@ class Exact : public TableMatch {
     const IR::Constant *getEvaluatedValue() const;
 };
 
-using FieldMatch = boost::variant<Exact, Ternary, LPM, Range>;
+class Optional : public TableMatch {
+ private:
+    /// The value the key is matched with.
+    const IR::Expression *value;
+
+    /// Whether to add this optional match as an exact match.
+    bool addMatch;
+
+ public:
+    explicit Optional(const IR::KeyElement *key, const IR::Expression *value, bool addMatch);
+
+    const Optional *evaluate(const Model &model) const override;
+
+    cstring getObjectName() const override;
+
+    /// @returns the match value. It is expected to be a constant at this point.
+    /// A BUG is thrown otherwise.
+    const IR::Constant *getEvaluatedValue() const;
+
+    /// @returns whether to add this optional match as an exact match.
+    bool addAsExactMatch() const;
+};
+
+using FieldMatch = boost::variant<Exact, Ternary, LPM, Range, Optional>;
 
 class TableRule : public TestObject {
  private:

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/backend/protobuf/protobuf.cpp b/backends/p4tools/modules/testgen/targets/bmv2/backend/protobuf/protobuf.cpp
@@ -236,6 +236,13 @@ inja::json Protobuf::getControlPlaneForTable(const std::map<cstring, const Field
                 j["id"] = *p4RuntimeId;
                 rulesJson["lpm_matches"].push_back(j);
             }
+            void operator()(const Optional &elem) const {
+                inja::json j;
+                j["field_name"] = fieldName;
+                j["value"] = formatHexExpr(elem.getEvaluatedValue()).c_str();
+                rulesJson["needs_priority"] = true;
+                rulesJson["optional_matches"].push_back(j);
+            }
         };
         boost::apply_visitor(GetRange(rulesJson, fieldName), fieldMatch);
     }
@@ -324,6 +331,15 @@ entities : [
         }
       }
 ## endfor
+## for r in rule.rules.optional_matches
+    # Match field {{r.field_name}}
+    match {
+      field_id: {{r.id}}
+      optional {
+        value: "{{r.value}}"
+      }
+    }
+## endfor
 ## for r in rule.rules.range_matches
       # Match field {{r.field_name}}
       match {

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/backend/ptf/ptf.cpp b/backends/p4tools/modules/testgen/targets/bmv2/backend/ptf/ptf.cpp
@@ -169,6 +169,18 @@ inja::json PTF::getControlPlaneForTable(const std::map<cstring, const FieldMatch
                 j["prefix_len"] = elem.getEvaluatedPrefixLength()->value.str();
                 rulesJson["lpm_matches"].push_back(j);
             }
+            void operator()(const Optional &elem) const {
+                inja::json j;
+                j["field_name"] = fieldName;
+                j["value"] = formatHexExpr(elem.getEvaluatedValue()).c_str();
+                if (elem.addAsExactMatch()) {
+                    j["use_exact"] = "True";
+                } else {
+                    j["use_exact"] = "False";
+                }
+                rulesJson["needs_priority"] = true;
+                rulesJson["optional_matches"].push_back(j);
+            }
         };
 
         boost::apply_visitor(GetRange(rulesJson, fieldName), fieldMatch);
@@ -299,8 +311,10 @@ class Test{{test_id}}(AbstractTest):
 ## for r in rule.rules.single_exact_matches
                 self.Exact('{{r.field_name}}', {{r.value}}),
 ## endfor
+## for r in rule.rules.optional_matches
+                self.Optional('{{r.field_name}}', {{r.value}}, {{r.use_exact}}),
+## endfor
 ## for r in rule.rules.range_matches
-                # TODO: p4Runtime doesn't have Range match - this would fail. Need to fix.
                 self.Range('{{r.field_name}}', {{r.lo}}, {{r.hi}}),
 ## endfor
 ## for r in rule.rules.ternary_matches

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/backend/stf/stf.cpp b/backends/p4tools/modules/testgen/targets/bmv2/backend/stf/stf.cpp
@@ -167,6 +167,13 @@ inja::json STF::getControlPlaneForTable(const std::map<cstring, const FieldMatch
                 // If the rule has a ternary match we need to add the priority.
                 rulesJson["needs_priority"] = true;
             }
+            void operator()(const Optional &elem) const {
+                inja::json j;
+                j["field_name"] = fieldName;
+                j["value"] = formatHexExpr(elem.getEvaluatedValue()).c_str();
+                rulesJson["needs_priority"] = true;
+                rulesJson["matches"].push_back(j);
+            }
         };
         boost::apply_visitor(GetRange(rulesJson, fieldName), fieldMatch);
     }

diff --git a/backends/p4tools/modules/testgen/targets/bmv2/table_stepper.cpp b/backends/p4tools/modules/testgen/targets/bmv2/table_stepper.cpp
@@ -31,11 +31,7 @@
 #include "backends/p4tools/modules/testgen/targets/bmv2/expr_stepper.h"
 #include "backends/p4tools/modules/testgen/targets/bmv2/test_spec.h"
 
-namespace P4Tools {
-
-namespace P4Testgen {
-
-namespace Bmv2 {
+namespace P4Tools::P4Testgen::Bmv2 {
 
 const IR::Expression *BMv2_V1ModelTableStepper::computeTargetMatchType(
     ExecutionState *nextState, const KeyProperties &keyProperties,
@@ -44,6 +40,17 @@ const IR::Expression *BMv2_V1ModelTableStepper::computeTargetMatchType(
 
     // TODO: We consider optional match types to be a no-op, but we could make them exact matches.
     if (keyProperties.matchType == BMv2Constants::MATCH_KIND_OPT) {
+        // We can recover from taint by simply not adding the optional match.
+        // Create a new zombie constant that corresponds to the key expression.
+        cstring keyName = properties.tableName + "_key_" + keyProperties.name;
+        const auto ctrlPlaneKey = nextState->createZombieConst(keyExpr->type, keyName);
+        if (keyProperties.isTainted) {
+            matches->emplace(keyProperties.name, Optional(keyProperties.key, ctrlPlaneKey, false));
+        } else {
+            const IR::Expression *keyExpr = keyProperties.key->expression;
+            matches->emplace(keyProperties.name, Optional(keyProperties.key, ctrlPlaneKey, true));
+            hitCondition = new IR::LAnd(hitCondition, new IR::Equ(keyExpr, ctrlPlaneKey));
+        }
         return hitCondition;
     }
     // Action selector entries are not part of the match.
@@ -441,8 +448,4 @@ BMv2_V1ModelTableStepper::BMv2_V1ModelTableStepper(BMv2_V1ModelExprStepper *step
                                                    const IR::P4Table *table)
     : TableStepper(stepper, table) {}
 
-}  // namespace Bmv2
-
-}  // namespace P4Testgen
-
-}  // namespace P4Tools
+}  // namespace P4Tools::P4Testgen::Bmv2
diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test/p4-programs/bmv2_table_opt.p4 b/backends/p4tools/modules/testgen/targets/bmv2/test/p4-programs/bmv2_table_opt.p4
@@ -0,0 +1,94 @@
+#include <v1model.p4>
+
+header ethernet_t {
+    bit<48> dst_addr;
+    bit<48> src_addr;
+    bit<16>         ether_type;
+}
+
+header ipv4_t {
+    bit<4>      version;
+    bit<4>      ihl;
+    bit<6>      dscp;
+    bit<2>      ecn;
+    bit<16>     total_len;
+    bit<16>     identification;
+    bit<1>      reserved;
+    bit<1>      do_not_fragment;
+    bit<1>      more_fragments;
+    bit<13>     frag_offset;
+    bit<8>      ttl;
+    bit<8>      protocol;
+    bit<16>     header_checksum;
+    bit<32> src_addr;
+    bit<32> dst_addr;
+}
+
+struct local_metadata_t {
+}
+
+struct Headers {
+    ethernet_t ethernet;
+    ipv4_t     ipv4;
+}
+
+
+parser p(packet_in pkt, out Headers h, inout local_metadata_t local_metadata, inout standard_metadata_t stdmeta) {
+    state start {
+        pkt.extract(h.ethernet);
+        pkt.extract(h.ipv4);
+        transition accept;
+    }
+
+}
+
+control vrfy(inout Headers h, inout local_metadata_t local_metadata) {
+    apply { }
+}
+
+
+control ingress(inout Headers h, inout local_metadata_t local_metadata, inout standard_metadata_t s) {
+    action acl_drop(inout standard_metadata_t standard_metadata) {
+        mark_to_drop(standard_metadata);
+    }
+
+    table acl_ingress_table {
+        key = {
+            h.ethernet.dst_addr                       : exact @name("dst_mac");
+            h.ipv4.src_addr                           : exact @name("src_ip");
+            h.ipv4.dst_addr                           : exact @name("dst_ip");
+            h.ipv4.ttl                                : optional @name("ttl");
+            h.ipv4.dscp                               : optional @name("dscp");
+            h.ipv4.ecn                                : optional @name("ecn");
+            h.ipv4.protocol                           : optional @name("ip_protocol");
+        }
+        actions = {
+            acl_drop(s);
+            @defaultonly NoAction;
+        }
+        const default_action = NoAction;
+    }
+    apply {
+        if (h.ipv4.isValid()) {
+            acl_ingress_table.apply();
+        }
+    }
+
+}
+
+control egress(inout Headers h, inout local_metadata_t local_metadata, inout standard_metadata_t s) {
+    apply { }
+}
+
+control update(inout Headers h, inout local_metadata_t local_metadata) {
+    apply { }
+}
+
+control deparser(packet_out pkt, in Headers h) {
+    apply {
+        pkt.emit(h);
+    }
+}
+
+
+V1Switch(p(), vrfy(), ingress(), egress(), update(), deparser()) main;
diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test_spec.cpp b/backends/p4tools/modules/testgen/targets/bmv2/test_spec.cpp
@@ -5,11 +5,7 @@
 
 #include "backends/p4tools/modules/testgen/lib/test_spec.h"
 
-namespace P4Tools {
-
-namespace P4Testgen {
-
-namespace Bmv2 {
+namespace P4Tools::P4Testgen::Bmv2 {
 
 /* =========================================================================================
  *  Bmv2Register
@@ -173,8 +169,4 @@ const Bmv2_CloneInfo *Bmv2_CloneInfo::evaluate(const Model &model) const {
 
 bool Bmv2_CloneInfo::isClonedPacket() const { return isClone; }
 
-}  // namespace Bmv2
-
-}  // namespace P4Testgen
-
-}  // namespace P4Tools
+}  // namespace P4Tools::P4Testgen::Bmv2
diff --git a/backends/p4tools/modules/testgen/targets/bmv2/test_spec.h b/backends/p4tools/modules/testgen/targets/bmv2/test_spec.h
@@ -12,11 +12,7 @@
 
 #include "backends/p4tools/modules/testgen/lib/test_spec.h"
 
-namespace P4Tools {
-
-namespace P4Testgen {
-
-namespace Bmv2 {
+namespace P4Tools::P4Testgen::Bmv2 {
 
 /* =========================================================================================
  *  Bmv2Register
@@ -181,10 +177,6 @@ class Bmv2_CloneInfo : public TestObject {
     const IR::Constant *getEvaluatedSessionId() const;
 };
 
-}  // namespace Bmv2
-
-}  // namespace P4Testgen
-
-}  // namespace P4Tools
+}  // namespace P4Tools::P4Testgen::Bmv2
 
 #endif /* BACKENDS_P4TOOLS_MODULES_TESTGEN_TARGETS_BMV2_TEST_SPEC_H_ */
diff --git a/backends/p4tools/modules/testgen/targets/ebpf/backend/stf/stf.cpp b/backends/p4tools/modules/testgen/targets/ebpf/backend/stf/stf.cpp
@@ -164,6 +164,13 @@ inja::json STF::getControlPlaneForTable(const std::map<cstring, const FieldMatch
                 // If the rule has a ternary match we need to add the priority.
                 rulesJson["needs_priority"] = true;
             }
+            void operator()(const Optional &elem) const {
+                inja::json j;
+                j["field_name"] = fieldName;
+                j["value"] = formatHexExpr(elem.getEvaluatedValue()).c_str();
+                rulesJson["needs_priority"] = true;
+                rulesJson["optional_matches"].push_back(j);
+            }
         };
         boost::apply_visitor(GetRange(rulesJson, fieldName), fieldMatch);
     }