Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix support for verify() and error type to fully support verify() in PSA/eBPF backend #3250

Merged
merged 1 commit into from
Apr 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions backends/ebpf/codeGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ bool CodeGenInspector::comparison(const IR::Operation_Relation* b) {
builder->append(")");
} else {
if (!et->is<IHasWidth>())
BUG("%1%: Comparisons for type %2% not yet implemented", type);
BUG("%1%: Comparisons for type %2% not yet implemented", b->left, type);
unsigned width = et->to<IHasWidth>()->implementationWidthInBits();
builder->append("memcmp(&");
visit(b->left);
Expand Down Expand Up @@ -195,10 +195,16 @@ bool CodeGenInspector::preorder(const IR::Cast* c) {
}

bool CodeGenInspector::preorder(const IR::Member* expression) {
bool isErrorAccess = false;
if (auto tne = expression->expr->to<IR::TypeNameExpression>()) {
if (tne->typeName->path->name.name == IR::Type_Error::error)
isErrorAccess = true;
}

int prec = expressionPrecedence;
expressionPrecedence = expression->getPrecedence();
auto ei = P4::EnumInstance::resolve(expression, typeMap);
if (ei == nullptr) {
if (ei == nullptr && !isErrorAccess) {
visit(expression->expr);
auto pe = expression->expr->to<IR::PathExpression>();
if (pe != nullptr && isPointerVariable(pe->path->name.name)) {
Expand Down
41 changes: 39 additions & 2 deletions backends/ebpf/ebpfParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,39 @@ StateTranslationVisitor::compileAdvance(const P4::ExternMethod* extMethod) {
builder->blockEnd(true);
}

void StateTranslationVisitor::compileVerify(const IR::MethodCallExpression * expression) {
BUG_CHECK(expression->arguments->size() == 2, "Expected 2 arguments: %1%", expression);

auto errorExpr = expression->arguments->at(1)->expression;
auto errorMember = errorExpr->to<IR::Member>();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use the error elimination pass (see below) this code may need to change.

auto type = typeMap->getType(errorExpr, true);
if (!type->is<IR::Type_Error>() || errorMember == nullptr) {
::error(ErrorType::ERR_UNEXPECTED, "%1%: not accessing a member error type", errorExpr);
return;
}

builder->emitIndent();
builder->append("if (!(");
visit(expression->arguments->at(0));
builder->append(")) ");
builder->blockStart();

builder->emitIndent();
builder->appendFormat("%s = %s",
state->parser->program->errorVar.c_str(), errorMember->member.name);
builder->endOfStatement(true);

cstring msg = Util::printf_format("Verify: condition failed, parser_error=%%u (%s)",
errorMember->member.name);
builder->target->emitTraceMessage(builder, msg.c_str(),
1, state->parser->program->errorVar.c_str());

builder->emitIndent();
builder->appendFormat("goto %s", IR::ParserState::reject.c_str());
builder->endOfStatement(true);
builder->blockEnd(true);
}

bool StateTranslationVisitor::preorder(const IR::AssignmentStatement* statement) {
if (auto mce = statement->right->to<IR::MethodCallExpression>()) {
auto mi = P4::MethodInstance::resolve(mce,
Expand Down Expand Up @@ -426,8 +459,12 @@ StateTranslationVisitor::compileExtract(const IR::Expression* destination) {
}

void StateTranslationVisitor::processFunction(const P4::ExternFunction* function) {
::error(ErrorType::ERR_UNEXPECTED,
"Unexpected extern function call in parser %1%", function->expr);
if (function->method->name.name == IR::ParserState::verify) {
compileVerify(function->expr);
} else {
::error(ErrorType::ERR_UNEXPECTED,
"Unexpected extern function call in parser %1%", function->expr);
}
}

void StateTranslationVisitor::processMethod(const P4::ExternMethod* method) {
Expand Down
1 change: 1 addition & 0 deletions backends/ebpf/ebpfParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class StateTranslationVisitor : public CodeGenInspector {
virtual void compileExtract(const IR::Expression* destination);
void compileLookahead(const IR::Expression* destination);
void compileAdvance(const P4::ExternMethod *ext);
void compileVerify(const IR::MethodCallExpression * expression);

virtual void processFunction(const P4::ExternFunction* function);
virtual void processMethod(const P4::ExternMethod* method);
Expand Down
3 changes: 3 additions & 0 deletions backends/ebpf/ebpfType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ EBPFType* EBPFTypeFactory::create(const IR::Type* type) {
if (et == nullptr)
return nullptr;
result = new EBPFStackType(ts, et);
} else if (type->is<IR::Type_Error>()) {
// Implement error type as scalar of width 8 bits
result = new EBPFScalarType(new IR::Type_Bits(8, false));
} else {
::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,
"Type %1% not supported", type);
Expand Down
37 changes: 37 additions & 0 deletions backends/ebpf/psa/ebpfPsaGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,38 @@ limitations under the License.

namespace EBPF {

class PSAErrorCodesGen : public Inspector {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have a midend pass called convertErrors that could perhaps be used to do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use this class only to iterate over error codes and emit them into C code. Similarly is done for filter architecture.

I've prepared a PR in my fork which shows difference with this midend pass, especially this commit. In my opinion there is no profits from using convertErrors pass.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds fine, it's a small amount of code.

CodeBuilder* builder;

public:
explicit PSAErrorCodesGen(CodeBuilder* builder) : builder(builder) {}

bool preorder(const IR::Type_Error* errors) override {
int id = -1;
for (auto decl : errors->members) {
++id;
if (decl->srcInfo.isValid()) {
auto sourceFile = decl->srcInfo.getSourceFile();
// all the error codes are located in core.p4 file, they are defined in psa.h
if (sourceFile.endsWith("p4include/core.p4"))
continue;
}

builder->emitIndent();
builder->appendFormat("static const ParserError_t %s = %d", decl->name.name, id);
builder->endOfStatement(true);

// type ParserError_t is u8, which can have values from 0 to 255
if (id > 255) {
::error(ErrorType::ERR_OVERLIMIT,
"%1%: Reached maximum number of possible errors", decl);
}
}
builder->newline();
return false;
}
};

// =====================PSAEbpfGenerator=============================
void PSAEbpfGenerator::emitPSAIncludes(CodeBuilder *builder) const {
builder->appendLine("#include <stdbool.h>");
Expand Down Expand Up @@ -89,6 +121,9 @@ void PSAEbpfGenerator::emitInternalStructures(CodeBuilder *builder) const {

/* Generate headers and structs in p4 prog */
void PSAEbpfGenerator::emitTypes(CodeBuilder *builder) const {
PSAErrorCodesGen errorGen(builder);
ingress->program->apply(errorGen);

for (auto type : ebpfTypes) {
type->emit(builder);
}
Expand Down Expand Up @@ -447,6 +482,8 @@ const PSAEbpfGenerator * ConvertToEbpfPSA::build(const IR::ToplevelBlock *tlb) {

const IR::Node *ConvertToEbpfPSA::preorder(IR::ToplevelBlock *tlb) {
ebpf_psa_arch = build(tlb);
ebpf_psa_arch->ingress->program = tlb->getProgram();
ebpf_psa_arch->egress->program = tlb->getProgram();
return tlb;
}

Expand Down
54 changes: 0 additions & 54 deletions backends/ebpf/psa/ebpfPsaParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,6 @@ limitations under the License.

namespace EBPF {

bool PsaStateTranslationVisitor::preorder(const IR::Expression* expression) {
// Allow for friendly error name in comment before verify() call, e.g. error.NoMatch
if (expression->is<IR::TypeNameExpression>()) {
auto tne = expression->to<IR::TypeNameExpression>();
builder->append(tne->typeName->path->name.name);
return false;
}

return CodeGenInspector::preorder(expression);
}

void PsaStateTranslationVisitor::processFunction(const P4::ExternFunction* function) {
if (function->method->name.name == "verify") {
compileVerify(function->expr);
return;
}

StateTranslationVisitor::processFunction(function);
}

void PsaStateTranslationVisitor::processMethod(const P4::ExternMethod* ext) {
auto externName = ext->originalExternType->name.name;

Expand All @@ -53,40 +33,6 @@ void PsaStateTranslationVisitor::processMethod(const P4::ExternMethod* ext) {
StateTranslationVisitor::processMethod(ext);
}

void PsaStateTranslationVisitor::compileVerify(const IR::MethodCallExpression * expression) {
BUG_CHECK(expression->arguments->size() == 2, "Expected 2 arguments: %1%", expression);

builder->emitIndent();
builder->append("if (!(");
visit(expression->arguments->at(0));
builder->append(")) ");

builder->blockStart();

builder->emitIndent();
builder->appendFormat("%s = ", parser->program->errorVar.c_str());

auto errorType = expression->arguments->at(1)->expression;
auto ei = P4::EnumInstance::resolve(errorType, typeMap);
if (ei == nullptr) {
::error(ErrorType::ERR_MODEL, "%1%: must be a constant on this target", errorType);
return;
}

builder->append(ei->name);

builder->endOfStatement(true);

cstring msg = Util::printf_format("Verify: condition failed, parser_error=%%u (%s)",
ei->name.name);
builder->target->emitTraceMessage(builder, msg.c_str(), 1, parser->program->errorVar.c_str());

builder->emitIndent();
builder->appendFormat("goto %s", IR::ParserState::reject.c_str());
builder->endOfStatement(true);
builder->blockEnd(true);
}

// =====================EBPFPsaParser=============================
EBPFPsaParser::EBPFPsaParser(const EBPFProgram* program, const IR::ParserBlock* block,
const P4::TypeMap* typeMap) : EBPFParser(program, block, typeMap) {
Expand Down
5 changes: 0 additions & 5 deletions backends/ebpf/psa/ebpfPsaParser.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,7 @@ class PsaStateTranslationVisitor : public StateTranslationVisitor {
EBPFPsaParser * prsr) :
StateTranslationVisitor(refMap, typeMap), parser(prsr) {}

bool preorder(const IR::Expression* expression) override;

void processFunction(const P4::ExternFunction* function) override;
void processMethod(const P4::ExternMethod* ext) override;

void compileVerify(const IR::MethodCallExpression * expression);
};

class EBPFPsaParser : public EBPFParser {
Expand Down
31 changes: 17 additions & 14 deletions backends/ebpf/tests/p4testdata/internet-checksum.p4
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,6 @@ limitations under the License.
#include <psa.p4>
#include "common_headers.p4"

header clone_i2e_metadata_t {
}

struct empty_metadata_t {
}

struct metadata {
bit<16> checksum;
bit<16> state;
Expand All @@ -35,13 +29,17 @@ struct headers {
udp_t udp;
}

error {
BadIPv4HeaderChecksum
}

parser IngressParserImpl(
packet_in buffer,
out headers parsed_hdr,
inout metadata user_meta,
in psa_ingress_parser_input_metadata_t istd,
in empty_metadata_t resubmit_meta,
in empty_metadata_t recirculate_meta)
in empty_t resubmit_meta,
in empty_t recirculate_meta)
{
InternetChecksum() ck;

Expand All @@ -68,6 +66,7 @@ parser IngressParserImpl(
/* 16-bit words 6-7 */ parsed_hdr.ipv4.srcAddr,
/* 16-bit words 8-9 */ parsed_hdr.ipv4.dstAddr
});
verify(parsed_hdr.ipv4.hdrChecksum == ck.get(), error.BadIPv4HeaderChecksum);
parsed_hdr.ipv4.hdrChecksum = ck.get_state();
transition accept;
}
Expand All @@ -80,14 +79,18 @@ control ingress(inout headers hdr,
inout psa_ingress_output_metadata_t ostd)
{
apply {
if (istd.parser_error != error.NoError) {
return;
}

send_to_port(ostd, (PortId_t) 5);
}
}

control IngressDeparserImpl(
packet_out packet,
out clone_i2e_metadata_t clone_i2e_meta,
out empty_metadata_t resubmit_meta,
out empty_t clone_i2e_meta,
out empty_t resubmit_meta,
out metadata normal_meta,
inout headers parsed_hdr,
in metadata meta,
Expand All @@ -105,8 +108,8 @@ parser EgressParserImpl(
inout metadata user_meta,
in psa_egress_parser_input_metadata_t istd,
in metadata normal_meta,
in clone_i2e_metadata_t clone_i2e_meta,
in empty_metadata_t clone_e2e_meta)
in empty_t clone_i2e_meta,
in empty_t clone_e2e_meta)
{
InternetChecksum() ck;

Expand Down Expand Up @@ -158,8 +161,8 @@ control egress(inout headers hdr,

control EgressDeparserImpl(
packet_out packet,
out empty_metadata_t clone_e2e_meta,
out empty_metadata_t recirculate_meta,
out empty_t clone_e2e_meta,
out empty_t recirculate_meta,
inout headers parsed_hdr,
in metadata meta,
in psa_egress_output_metadata_t istd,
Expand Down
Loading