Skip to content

Commit

Permalink
Fix "new type" handling in P4Info serialization code
Browse files Browse the repository at this point in the history 
The previous code was making assumptions that did not make sense to
me. I rewrote it and added tests (Gtest unit tests + the P4 test program
attached by @jafingerhut when he submitted the issue).

Fixes #2234
  • Loading branch information
@antoninbas
antoninbas committed Mar 12, 2020
1 parent 6ea9e17 commit a1f21cb
Show file tree
Hide file tree
Showing 12 changed files with 1,302 additions and 108 deletions.
41 changes: 10 additions & 31 deletions control-plane/p4RuntimeSerializer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,12 @@ limitations under the License.
#include "lib/ordered_set.h"
#include "midend/removeParameters.h"

#include "bytestrings.h"
#include "flattenHeader.h"
#include "p4RuntimeSerializer.h"
#include "p4RuntimeArchHandler.h"
#include "p4RuntimeArchStandard.h"
#include "flattenHeader.h"
#include "bytestrings.h"
#include "typeSpecConverter.h"

namespace p4v1 = ::p4::v1;
namespace p4configv1 = ::p4::config::v1;
Expand Down Expand Up @@ -636,29 +637,15 @@ getMatchType(cstring matchTypeName) {
}
}

// P4Runtime defines sdnB as a 32-bit integer.
// The APIs in this file for width use an int
// Thus function returns a signed int.
// getTypeWidth returns the width in bits for the @type, except if it is a user-defined type with a
// @p4runtime_translation annotation, in which case it returns the SDN bitwidth specified by the
// user.
static int
getTypeWidth(const IR::Type* type, TypeMap* typeMap) {
auto ann = type->getAnnotation("p4runtime_translation");
if (ann != nullptr) {
auto sdnB = ann->expr[1]->to<IR::Constant>();
if (!sdnB) {
::error("P4runtime annotation in serializer does not have sdn: %1%",
type);
return -1;
}
auto value = sdnB->value;
auto bitsRequired = floor_log2(value) + 1;
if (bitsRequired > 31) {
::error("Cannot represent %1% on 31 bits, require %2%", value,
bitsRequired);
return -2;
}
return static_cast<int>(value);
}
return typeMap->minWidthBits(type, type->getNode());
std::string uri;
int sdnB;
auto isTranslatedType = hasTranslationAnnotation(type, &uri, &sdnB);
return isTranslatedType ? sdnB : typeMap->minWidthBits(type, type->getNode());
}

/*
Expand Down Expand Up @@ -700,8 +687,6 @@ getMatchFields(const IR::P4Table* table, ReferenceMap* refMap, TypeMap* typeMap)
"Couldn't determine type for key element %1%", keyElement);
type_name = getTypeName(matchFieldType, typeMap);
int width = getTypeWidth(matchFieldType, typeMap);
if (width < 0)
return matchFields;
matchFields.push_back(MatchField{*matchFieldName, *matchType,
matchTypeName, uint32_t(width),
keyElement->to<IR::IAnnotated>(), type_name});
Expand Down Expand Up @@ -903,8 +888,6 @@ class P4RuntimeAnalyzer {
continue;
}
int w = getTypeWidth(paramType, typeMap);
if (w < 0)
return;
param->set_bitwidth(w);
cstring type_name = getTypeName(paramType, typeMap);
if (type_name) {
Expand Down Expand Up @@ -954,8 +937,6 @@ class P4RuntimeAnalyzer {
"int<>, type, or serializable enum",
headerField);
auto w = getTypeWidth(fieldType, typeMap);
if (w < 0)
return;
metadata->set_bitwidth(w);
}
}
Expand Down Expand Up @@ -1424,8 +1405,6 @@ class P4RuntimeEntriesConverter {
protoParam->set_param_id(parameterId++);
auto parameter = actionDecl->parameters->parameters.at(parameterIndex++);
int width = getTypeWidth(parameter->type, typeMap);
if (width < 0)
return;
if (arg->expression->is<IR::Constant>()) {
auto value = stringRepr(arg->expression->to<IR::Constant>(), width);
protoParam->set_value(*value);
Expand Down
94 changes: 56 additions & 38 deletions control-plane/typeSpecConverter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

#include <limits>
#include <map>
#include <string>

Expand All @@ -40,6 +41,35 @@ namespace P4 {

namespace ControlPlaneAPI {

bool hasTranslationAnnotation(const IR::Type* type, std::string* uri, int* sdnB) {
auto ann = type->getAnnotation("p4runtime_translation");
if (!ann) return false;
auto uriL = ann->expr[0]->to<IR::StringLiteral>();
if (!uriL) {
::error(ErrorType::ERR_INVALID,
"%1%: the first argument to @p4runtime_translation must be a string literal",
type);
return false;
}
*uri = static_cast<std::string>(uriL->value);
auto sdnBL = ann->expr[1]->to<IR::Constant>();
if (!sdnBL) {
::error(ErrorType::ERR_INVALID,
"%1%: the second argument to @p4runtime_translation must be a positive constant",
type);
return false;
}
if (sdnBL->value <= 0 || sdnBL->value > std::numeric_limits<int32_t>::max()) {
::error(ErrorType::ERR_INVALID,
"%1%: the second argument to @p4runtime_translation must be positive integer that "
"fits in 32 bits",
type);
return false;
}
*sdnB = static_cast<int32_t>(sdnBL->value);
return true;
}

TypeSpecConverter::TypeSpecConverter(
const P4::ReferenceMap* refMap, const P4::TypeMap* typeMap, P4TypeInfo* p4RtTypeInfo)
: refMap(refMap), typeMap(typeMap), p4RtTypeInfo(p4RtTypeInfo) {
Expand Down Expand Up @@ -117,52 +147,40 @@ bool TypeSpecConverter::preorder(const IR::Type_Name* type) {
return false;
}

// This function is invoked if an architecure's model .p4 file has a Newtype.
// The function is not invoked for user-defined NewType.
// TODO(team): investigate and fix.
bool TypeSpecConverter::preorder(const IR::Type_Newtype* type) {
if (p4RtTypeInfo) {
bool orig_type = true;
const IR::StringLiteral* uri = nullptr;
const IR::Constant* sdnB;
auto ann = type->getAnnotation("p4runtime_translation");
if (ann != nullptr) {
orig_type = false;
uri = ann->expr[0]->to<IR::StringLiteral>();
if (!uri) {
::error("P4runtime annotation does not have uri: %1%",
type);
return false;
}
sdnB = ann->expr[1]->to<IR::Constant>();
if (!sdnB) {
::error("P4runtime annotation does not have sdn: %1%",
type);
return false;
}
auto value = sdnB->value;
auto bitsRequired = floor_log2(value) + 1;
BUG_CHECK(bitsRequired <= 31,
"Cannot represent %1% on 31 bits, require %2%",
value, bitsRequired);
}
std::string uri;
int sdnB;
auto isTranslatedType = hasTranslationAnnotation(type, &uri, &sdnB);

auto name = std::string(type->controlPlaneName());
auto types = p4RtTypeInfo->mutable_new_types();
if (types->find(name) == types->end()) {
auto newTypeSpec = new p4configv1::P4NewTypeSpec();
auto newType = type->type;
auto n = newType->to<IR::Type_Name>();
visit(n);
auto typeSpec = map.at(n);
if (orig_type) {
auto dataType = newTypeSpec->mutable_original_type();
if (typeSpec->has_bitstring())
dataType->mutable_bitstring()->CopyFrom(typeSpec->bitstring());

// walk the chain of new types
const IR::Type* underlyingType = type;
while (underlyingType->is<IR::Type_Newtype>()) {
underlyingType = typeMap->getTypeType(
underlyingType->to<IR::Type_Newtype>()->type, true);
}
if (isTranslatedType && !underlyingType->is<IR::Type_Bits>()) {
::error(ErrorType::ERR_INVALID,
"%1%: P4Runtime requires the underlying type for a user-defined type with "
"the @p4runtime_translation annotation to be bit<W>; it cannot be '%2%'",
type, underlyingType);
// no need to return early here
}
visit(underlyingType);
auto typeSpec = map.at(underlyingType);
CHECK_NULL(typeSpec);

if (isTranslatedType) {
auto translatedType = newTypeSpec->mutable_translated_type();
translatedType->set_uri(uri);
translatedType->set_sdn_bitwidth(sdnB);
} else {
auto dataType = newTypeSpec->mutable_translated_type();
dataType->set_uri(std::string(uri->value));
dataType->set_sdn_bitwidth((uint32_t) sdnB->value);
newTypeSpec->mutable_original_type()->CopyFrom(*typeSpec);
}
(*types)[name] = *newTypeSpec;
}
Expand Down
6 changes: 6 additions & 0 deletions control-plane/typeSpecConverter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ limitations under the License.
#define CONTROL_PLANE_TYPESPECCONVERTER_H_

#include <map>
#include <string>

#include "p4/config/v1/p4types.pb.h"

Expand Down Expand Up @@ -85,6 +86,11 @@ class TypeSpecConverter : public Inspector {
const IR::Type* type, ::p4::config::v1::P4TypeInfo* typeInfo);
};

// hasTranslationAnnotation returns true iff the type is annotated by a *valid*
// p4runtime_translation annotation, in which case it also sets @uri and @sdnB based on the
// annotation's two arguments.
bool hasTranslationAnnotation(const IR::Type* type, std::string* uri, int* sdnB);

} // namespace ControlPlaneAPI

} // namespace P4
Expand Down
122 changes: 83 additions & 39 deletions test/gtest/p4runtime.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1336,44 +1336,6 @@ TEST_F(P4RuntimePkgInfo, ValueNotAString) {
EXPECT_EQ(pkgInfo.name(), "");
}

TEST_F(P4Runtime, P4_16_MatchFieldsSize) {
auto test = createP4RuntimeTestCase(P4_SOURCE(P4Headers::V1MODEL, R"(
@p4runtime_translation("mycompany.com/My_Byte2", 0xffffffff)
type bit<8> CustomT_t;
header Header { CustomT_t headerField; }
struct Headers { Header h; }
struct Metadata { bit<33> metadataField; }
parser parse(packet_in p, out Headers h, inout Metadata m,
inout standard_metadata_t sm) {
state start { transition accept; } }
control verifyChecksum(inout Headers h, inout Metadata m) { apply { } }
control egress(inout Headers h, inout Metadata m,
inout standard_metadata_t sm) { apply { } }
control computeChecksum(inout Headers h, inout Metadata m) { apply { } }
control deparse(packet_out p, in Headers h) { apply { } }
control ingress(inout Headers h, inout Metadata m,
inout standard_metadata_t sm) {
action noop() { }
table igTable {
key = {
h.h.headerField : exact;
}
actions = { noop; }
}
apply {
igTable.apply();
}
}
V1Switch(parse(), verifyChecksum(), ingress(), egress(),
computeChecksum(), deparse()) main;
)"));

ASSERT_TRUE(test);
EXPECT_EQ(1u, ::diagnosticCount());
}

TEST_F(P4RuntimePkgInfo, DuplicateKey) {
auto test = createTestCase(R"(@pkginfo(name="aaa", name="bbb"))");
// kv annotations use an IndexedVector, which does not allow duplicate keys.
Expand Down Expand Up @@ -1407,6 +1369,10 @@ class P4RuntimeDataTypeSpec : public P4Runtime {
auto pgm = P4::parseP4String(programStr, CompilerOptions::FrontendVersion::P4_16);
if (pgm == nullptr) return nullptr;
PassManager passes({
new P4::ParseAnnotations("P4RuntimeDataTypeSpecTest", false, {
// @p4runtime_translation has two args
PARSE_PAIR("p4runtime_translation", Expression),
}),
new P4::ResolveReferences(&refMap),
new P4::TypeInference(&refMap, &typeMap, false)
});
Expand Down Expand Up @@ -1730,7 +1696,7 @@ TEST_F(P4RuntimeDataTypeSpec, StructWithTypedef) {
ASSERT_EQ(2, it->second.members_size());
auto check_member = [&](cstring name, int index) {
EXPECT_EQ(name, it->second.members(index).name());
const auto &memberTypeSpec = it->second.members(0).type_spec();
const auto &memberTypeSpec = it->second.members(index).type_spec();
ASSERT_TRUE(memberTypeSpec.has_bitstring());
ASSERT_TRUE(memberTypeSpec.bitstring().has_bit());
EXPECT_EQ(8, memberTypeSpec.bitstring().bit().bitwidth());
Expand All @@ -1739,4 +1705,82 @@ TEST_F(P4RuntimeDataTypeSpec, StructWithTypedef) {
check_member("f2", 1);
}

TEST_F(P4RuntimeDataTypeSpec, NewType) {
std::string program = P4_SOURCE(R"(
type bit<8> my_type_t;
@p4runtime_translation("p4.org/myArch/v1/Type2", 32)
type bit<8> my_type2_t;
struct my_struct { my_type_t f; my_type2_t f2; }
extern my_extern_t<T> { my_extern_t(bit<32> v); }
my_extern_t<my_struct>(32w1024) my_extern;
)");
auto pgm = getProgram(program);
ASSERT_TRUE(pgm != nullptr && ::errorCount() == 0);

auto type = findExternTypeParameterName<IR::Type_Name>(pgm, "my_extern_t");
ASSERT_TRUE(type != nullptr);
auto typeSpec = P4::ControlPlaneAPI::TypeSpecConverter::convert(
&refMap, &typeMap, type, &typeInfo);
ASSERT_TRUE(typeSpec->has_struct_());
EXPECT_EQ("my_struct", typeSpec->struct_().name());

auto it = typeInfo.structs().find("my_struct");
ASSERT_TRUE(it != typeInfo.structs().end());
ASSERT_EQ(2, it->second.members_size());

auto check_member = [&](cstring memberName, int index, cstring newTypeName) {
EXPECT_EQ(memberName, it->second.members(index).name());
const auto &memberTypeSpec = it->second.members(index).type_spec();
ASSERT_TRUE(memberTypeSpec.has_new_type());
EXPECT_EQ(newTypeName, memberTypeSpec.new_type().name());
};
check_member("f", 0, "my_type_t");
check_member("f2", 1, "my_type2_t");

// non-translated
{
auto it = typeInfo.new_types().find("my_type_t");
ASSERT_TRUE(it != typeInfo.new_types().end());
ASSERT_TRUE(it->second.has_original_type());
const auto &typeSpec = it->second.original_type();
ASSERT_TRUE(typeSpec.has_bitstring());
EXPECT_EQ(8, typeSpec.bitstring().bit().bitwidth());
}

// translated
{
auto it = typeInfo.new_types().find("my_type2_t");
ASSERT_TRUE(it != typeInfo.new_types().end());
ASSERT_TRUE(it->second.has_translated_type());
const auto &translatedType = it->second.translated_type();
EXPECT_EQ("p4.org/myArch/v1/Type2", translatedType.uri());
EXPECT_EQ(32, translatedType.sdn_bitwidth());
}
}

TEST_F(P4RuntimeDataTypeSpec, NewTypeInvalidTranslation) {
std::string program = P4_SOURCE(R"(
@p4runtime_translation("p4.org/myArch/v1/Type", 0xffffffff)
type bit<8> my_type_t;
@p4runtime_translation("p4.org/myArch/v1/Type2", -1)
type bit<8> my_type2_t;
@p4runtime_translation(1, 32)
type bit<8> my_type3_t;
@p4runtime_translation("p4.org/myArch/v1/Type4", 32)
type bool my_type4_t;
struct my_struct { my_type_t f; my_type2_t f2; my_type3_t f3; my_type4_t f4; }
extern my_extern_t<T> { my_extern_t(bit<32> v); }
my_extern_t<my_struct>(32w1024) my_extern;
)");
auto pgm = getProgram(program);
ASSERT_TRUE(pgm != nullptr && ::errorCount() == 0);

auto type = findExternTypeParameterName<IR::Type_Name>(pgm, "my_extern_t");
ASSERT_TRUE(type != nullptr);
P4::ControlPlaneAPI::TypeSpecConverter::convert(
&refMap, &typeMap, type, &typeInfo);
// expect 4 errors, one for each invalid @p4runtime_translation annotation above.
EXPECT_EQ(4u, ::errorCount());
}

} // namespace Test
Loading

0 comments on commit a1f21cb

Please sign in to comment.