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.

Sign up for GitHub

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

ebpf/PSA: Support for wide fields in Register and Meter #3854

Merged
merged 2 commits into from
Jan 23, 2023
Merged

ebpf/PSA: Support for wide fields in Register and Meter #3854

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7554cb9
ebpf/PSA: Support for wide fields in Register and Meter
tatry Jan 19, 2023
404d7fc
fix typos
tatry Jan 23, 2023
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
Next Next commit
ebpf/PSA: Support for wide fields in Register and Meter 
This commit allows to use Registers and Meters with fields wider than 64 bits.
  • Loading branch information
@tatry
tatry committed Jan 19, 2023
commit 7554cb932dfae7db0b72a08c3747f1008b27bfa6
45 changes: 42 additions & 3 deletions backends/ebpf/codeGen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,20 @@ void CodeGenInspector::substitute(const IR::Parameter *p, const IR::Parameter *w
}

bool CodeGenInspector::preorder(const IR::Constant *expression) {
builder->append(Util::toString(expression->value, 0, false, expression->base));
return true;
unsigned width = EBPFInitializerUtils::ebpfTypeWidth(typeMap, expression);

if (EBPFScalarType::generatesScalar(width)) {
builder->append(Util::toString(expression->value, 0, false, expression->base));
return true;
}

cstring str = EBPFInitializerUtils::genHexStr(expression->value, width, expression);
builder->append("{ ");
for (size_t i = 0; i < str.size() / 2; ++i)
builder->appendFormat("0x%s, ", str.substr(2 * i, 2));
builder->append("}");

return false;
}

bool CodeGenInspector::preorder(const IR::StringLiteral *expression) {
Expand Down Expand Up @@ -329,9 +341,12 @@ bool CodeGenInspector::preorder(const IR::AssignmentStatement *a) {
}

if (memcpy) {
builder->append("memcpy(&");
builder->append("__builtin_memcpy(&");
visit(a->left);
builder->append(", &");
if (a->right->is<IR::Constant>()) {
builder->appendFormat("(u8[%u])", scalar->bytesRequired());
}
visit(a->right);
builder->appendFormat(", %d)", scalar->bytesRequired());
} else {
Expand Down Expand Up @@ -429,4 +444,28 @@ void CodeGenInspector::widthCheck(const IR::Node *node) const {
node, tb->size);
}

unsigned EBPFInitializerUtils::ebpfTypeWidth(P4::TypeMap *typeMap, const IR::Expression *expr) {
auto type = typeMap->getType(expr);
if (type == nullptr) type = expr->type;
if (type->is<IR::Type_InfInt>()) return 32; // let's assume 32 bit for int type
Copy link
Contributor

Choose a reason for hiding this comment

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

the only place where this can happen as far I know is in rhs of shifts.

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 can't recall where exactly hit the problem with this type.
For now I am able to make this statement true with slice, like field[127:64] (which is translated using bit shifts).

auto ebpfType = EBPFTypeFactory::instance->create(type);
if (auto scalar = ebpfType->to<EBPFScalarType>()) {
unsigned width = scalar->implementationWidthInBits();
unsigned alignment = scalar->alignment() * 8;
unsigned units = ROUNDUP(width, alignment);
return units * alignment;
}
return 8; // assume 1 byte if not available such information
}

cstring EBPFInitializerUtils::genHexStr(const big_int &value, unsigned width,
const IR::Expression *expr) {
// the required length of hex string, must be an even number
unsigned nibbles = 2 * ROUNDUP(width, 8);
auto str = value.str(0, std::ios_base::hex);
if (str.size() < nibbles) str = std::string(nibbles - str.size(), '0') + str;
BUG_CHECK(str.size() == nibbles, "%1%: value size does not match %2% bits", expr, width);
return str;
}

} // namespace EBPF
9 changes: 9 additions & 0 deletions backends/ebpf/codeGen.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,15 @@ class CodeGenInspector : public Inspector {
void widthCheck(const IR::Node *node) const;
};

class EBPFInitializerUtils {
public:
// return *real* number of bits required by type
static unsigned ebpfTypeWidth(P4::TypeMap *typeMap, const IR::Expression *expr);

// Generate hex string and prepend it with zeroes when shorter than required width
static cstring genHexStr(const big_int &value, unsigned width, const IR::Expression *expr);
};

} // namespace EBPF

#endif /* _BACKENDS_EBPF_CODEGEN_H_ */
39 changes: 29 additions & 10 deletions backends/ebpf/ebpfParser.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,13 +242,15 @@ bool StateTranslationVisitor::preorder(const IR::SelectCase *selectCase) {
return false;
}

void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, cstring field,
unsigned alignment, EBPFType *type) {
void StateTranslationVisitor::compileExtractField(const IR::Expression *expr,
const IR::StructField *field, unsigned alignment,
EBPFType *type) {
unsigned widthToExtract = dynamic_cast<IHasWidth *>(type)->widthInBits();
auto program = state->parser->program;
cstring msgStr;
cstring fieldName = field->name.name;

msgStr = Util::printf_format("Parser: extracting field %s", field);
msgStr = Util::printf_format("Parser: extracting field %s", fieldName);
builder->target->emitTraceMessage(builder, msgStr.c_str());

if (widthToExtract <= 64) {
Expand Down Expand Up @@ -277,7 +279,7 @@ void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, cs
unsigned shift = loadSize - alignment - widthToExtract;
builder->emitIndent();
visit(expr);
builder->appendFormat(".%s = (", field.c_str());
builder->appendFormat(".%s = (", fieldName.c_str());
type->emit(builder);
builder->appendFormat(")((%s(%s, BYTES(%s))", helper, program->packetStartVar.c_str(),
program->offsetVar.c_str());
Expand All @@ -293,6 +295,23 @@ void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, cs
builder->append(")");
builder->endOfStatement(true);
} else {
if (program->options.arch == "psa" && widthToExtract % 8 != 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

yes, in my implementation I never figured out how all this stuff should be handled.

// To explain the problem in error lets assume that we have bit<68> field with value:
// 0x11223344556677889
// ^ this digit will be parsed into a half of byte
// Such fields are parsed into a table of bytes in network byte order, so possible
// values in dataplane are (note the position of additional '0' at the end):
// 0x112233445566778809
// 0x112233445566778890
// To correctly insert that padding, the length of field must be known, but tools like
// nikss-ctl (and the nikss library) don't consume P4info.txt to have such knowledge.
// There is also a bug in (de)parser causing that such fields aren't deparsed correctly.
Copy link
Contributor

Choose a reason for hiding this comment

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

causing such fields to be deparsed incorrectly.

::error(ErrorType::ERR_UNSUPPORTED_ON_TARGET,
"%1%: fields wider than 64 bits must have size in multiply of 8 bits (1 byte) "
Copy link
Contributor

Choose a reason for hiding this comment

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

a size multiple of 8 bits

"due to ambiguous padding in the LSB byte when the condition is not met",
field);
}

// wide values; read all bytes one by one.
unsigned shift;
if (alignment == 0)
Expand All @@ -310,7 +329,7 @@ void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, cs
for (unsigned i = 0; i < bytes; i++) {
builder->emitIndent();
visit(expr);
builder->appendFormat(".%s[%d] = (", field.c_str(), i);
builder->appendFormat(".%s[%d] = (", fieldName.c_str(), i);
bt->emit(builder);
builder->appendFormat(")((%s(%s, BYTES(%s) + %d) >> %d)", helper,
program->packetStartVar.c_str(), program->offsetVar.c_str(), i,
Expand Down Expand Up @@ -343,13 +362,13 @@ void StateTranslationVisitor::compileExtractField(const IR::Expression *expr, cs
expr->to<IR::Member>()->expr->to<IR::PathExpression>()->path->name.name)) {
exprStr = exprStr.replace(".", "->");
}
cstring tmp = Util::printf_format("(unsigned long long) %s.%s", exprStr, field);
cstring tmp = Util::printf_format("(unsigned long long) %s.%s", exprStr, fieldName);

msgStr =
Util::printf_format("Parser: extracted %s=0x%%llx (%u bits)", field, widthToExtract);
msgStr = Util::printf_format("Parser: extracted %s=0x%%llx (%u bits)", fieldName,
widthToExtract);
builder->target->emitTraceMessage(builder, msgStr.c_str(), 1, tmp.c_str());
} else {
msgStr = Util::printf_format("Parser: extracted %s (%u bits)", field, widthToExtract);
msgStr = Util::printf_format("Parser: extracted %s (%u bits)", fieldName, widthToExtract);
builder->target->emitTraceMessage(builder, msgStr.c_str());
}

Expand Down Expand Up @@ -428,7 +447,7 @@ void StateTranslationVisitor::compileExtract(const IR::Expression *destination)
"Only headers with fixed widths supported %1%", f);
return;
}
compileExtractField(destination, f->name, alignment, etype);
compileExtractField(destination, f, alignment, etype);
alignment += et->widthInBits();
alignment %= 8;
}
Expand Down
4 changes: 2 additions & 2 deletions backends/ebpf/ebpfParser.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ class StateTranslationVisitor : public CodeGenInspector {
P4::P4CoreLibrary &p4lib;
const EBPFParserState *state;

void compileExtractField(const IR::Expression *expr, cstring name, unsigned alignment,
EBPFType *type);
void compileExtractField(const IR::Expression *expr, const IR::StructField *field,
unsigned alignment, EBPFType *type);
virtual void compileExtract(const IR::Expression *destination);
void compileLookahead(const IR::Expression *destination);
void compileAdvance(const P4::ExternMethod *ext);
Expand Down
14 changes: 11 additions & 3 deletions backends/ebpf/ebpfType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ void EBPFScalarType::declare(CodeBuilder *builder, cstring id, bool asPointer) {
builder->append(id);
} else {
if (asPointer)
builder->append("u8*");
builder->appendFormat("u8* %s", id.c_str());
else
builder->appendFormat("u8 %s[%d]", id.c_str(), bytesRequired());
}
Expand All @@ -144,9 +144,17 @@ void EBPFScalarType::declareInit(CodeBuilder *builder, cstring id, bool asPointe
builder->append(id);
} else {
if (asPointer)
builder->append("u8*");
builder->appendFormat("u8* %s = NULL", id.c_str());
else
builder->appendFormat("uint8_t %s[%d]", id.c_str(), bytesRequired());
builder->appendFormat("u8 %s[%d] = {0}", id.c_str(), bytesRequired());
}
}

void EBPFScalarType::emitInitializer(CodeBuilder *builder) {
if (generatesScalar(width)) {
builder->append("0");
} else {
builder->append("{ 0 }");
}
}

Expand Down
2 changes: 1 addition & 1 deletion backends/ebpf/ebpfType.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ class EBPFScalarType : public EBPFType, public IHasWidth {
void emit(CodeBuilder *builder) override;
void declare(CodeBuilder *builder, cstring id, bool asPointer) override;
void declareInit(CodeBuilder *builder, cstring id, bool asPointer) override;
void emitInitializer(CodeBuilder *builder) override { builder->append("0"); }
void emitInitializer(CodeBuilder *builder) override;
unsigned widthInBits() override { return width; }
unsigned implementationWidthInBits() override { return bytesRequired() * 8; }
// True if this width is small enough to store in a machine scalar
Expand Down
3 changes: 2 additions & 1 deletion backends/ebpf/psa/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -619,7 +619,8 @@ table caching pass `--table-caching` to the compiler.

We list the known bugs/limitations below. Refer to the Roadmap section for features planned in the near future.

- Larger bit fields (e.g. IPv6 addresses) may not work properly.
- Fields wider than 64 bits must have size multiply of 8 bits, otherwise they may have unexpected value in the LSB byte.
Copy link
Contributor

Choose a reason for hiding this comment

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

multiple

These fields may not work with all the externs and not all the operations on them are possible.
- We noticed that `bpf_xdp_adjust_meta()` isn't implemented by some NIC drivers, so the `meta` XDP2TC mode may not work
with some NICs. So far, we have verified the correct behavior with Intel 82599ES. If a NIC doesn't support the `meta` XDP2TC mode you can use `head` or `cpumap` modes.
- `lookahead()` with bit fields (e.g., `bit<16>`) doesn't work.
Expand Down
58 changes: 9 additions & 49 deletions backends/ebpf/psa/ebpfPsaTable.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,32 +137,6 @@ class EBPFTablePSAImplementationPropertyVisitor : public EBPFTablePsaPropertyVis
}
};

class EBPFTablePSAInitializerUtils {
public:
// return *real* number of bits required by type
static unsigned ebpfTypeWidth(P4::TypeMap *typeMap, const IR::Expression *expr) {
auto type = typeMap->getType(expr);
auto ebpfType = EBPFTypeFactory::instance->create(type);
if (auto scalar = ebpfType->to<EBPFScalarType>()) {
unsigned width = scalar->implementationWidthInBits();
unsigned alignment = scalar->alignment() * 8;
unsigned units = ROUNDUP(width, alignment);
return units * alignment;
}
return 8; // assume 1 byte if not available such information
}

// Generate hex string and prepend it with zeroes when shorter than required width
static cstring genHexStr(const big_int &value, unsigned width, const IR::Expression *expr) {
// the required length of hex string, must be an even number
unsigned nibbles = 2 * ROUNDUP(width, 8);
auto str = value.str(0, std::ios_base::hex);
if (str.size() < nibbles) str = std::string(nibbles - str.size(), '0') + str;
BUG_CHECK(str.size() == nibbles, "%1%: value size does not match %2% bits", expr, width);
return str;
}
};

// Generator for table key/value initializer value (const entries). Can't be used during table
// lookup because this inspector expects only constant values as initializer.
class EBPFTablePSAInitializerCodeGen : public CodeGenInspector {
Expand Down Expand Up @@ -191,22 +165,8 @@ class EBPFTablePSAInitializerCodeGen : public CodeGenInspector {
expr->apply(*this);
}

bool preorder(const IR::Constant *expr) override {
unsigned width = EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, expr);

if (EBPFScalarType::generatesScalar(width)) return CodeGenInspector::preorder(expr);

cstring str = EBPFTablePSAInitializerUtils::genHexStr(expr->value, width, expr);
builder->append("{ ");
for (size_t i = 0; i < str.size() / 2; ++i)
builder->appendFormat("0x%s, ", str.substr(2 * i, 2));
builder->append("}");

return false;
}

bool preorder(const IR::Mask *expr) override {
unsigned width = EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, expr->left);
unsigned width = EBPFInitializerUtils::ebpfTypeWidth(typeMap, expr->left);

if (EBPFScalarType::generatesScalar(width)) {
visit(expr->left);
Expand All @@ -217,8 +177,8 @@ class EBPFTablePSAInitializerCodeGen : public CodeGenInspector {
auto rc = expr->right->to<IR::Constant>();
BUG_CHECK(lc != nullptr, "%1%: expected a constant value", expr->left);
BUG_CHECK(rc != nullptr, "%1%: expected a constant value", expr->right);
cstring value = EBPFTablePSAInitializerUtils::genHexStr(lc->value, width, expr->left);
cstring mask = EBPFTablePSAInitializerUtils::genHexStr(rc->value, width, expr->right);
cstring value = EBPFInitializerUtils::genHexStr(lc->value, width, expr->left);
cstring mask = EBPFInitializerUtils::genHexStr(rc->value, width, expr->right);
builder->append("{ ");
for (size_t i = 0; i < value.size() / 2; ++i)
builder->appendFormat("(0x%s & 0x%s), ", value.substr(2 * i, 2),
Expand All @@ -241,7 +201,7 @@ class EBPFTablePSAInitializerCodeGen : public CodeGenInspector {
cstring fieldName = ::get(table->keyFieldNames, key);
cstring matchType = key->matchType->path->name.name;
auto expr = currentEntry->keys->components[currentKeyEntryIndex];
unsigned width = EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, key->expression);
unsigned width = EBPFInitializerUtils::ebpfTypeWidth(typeMap, key->expression);
bool isLPMMatch = matchType == P4::P4CoreLibrary::instance.lpmMatch.name;
bool genPrefixLen = !tableHasTernaryMatch && isLPMMatch;
bool doSwapBytes = genPrefixLen && EBPFScalarType::generatesScalar(width);
Expand Down Expand Up @@ -341,16 +301,16 @@ class EBPFTablePSATernaryTableMaskGenerator : public Inspector {

bool preorder(const IR::Constant *expr) override {
// exact match, set all bits as 'care'
unsigned bytes = ROUNDUP(EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, expr), 8);
unsigned bytes = ROUNDUP(EBPFInitializerUtils::ebpfTypeWidth(typeMap, expr), 8);
for (unsigned i = 0; i < bytes; ++i) mask += "ff";
return false;
}
bool preorder(const IR::Mask *expr) override {
// Available value and mask, so use only this mask
BUG_CHECK(expr->right->is<IR::Constant>(), "%1%: Expected a constant value", expr->right);
auto &value = expr->right->to<IR::Constant>()->value;
unsigned width = EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, expr->right);
mask += EBPFTablePSAInitializerUtils::genHexStr(value, width, expr->right);
unsigned width = EBPFInitializerUtils::ebpfTypeWidth(typeMap, expr->right);
mask += EBPFInitializerUtils::genHexStr(value, width, expr->right);
return false;
}
};
Expand All @@ -365,7 +325,7 @@ class EBPFTablePSATernaryKeyMaskGenerator : public EBPFTablePSAInitializerCodeGe
// MidEnd transforms 0xffff... masks into exact match
// So we receive there a Constant same as exact match
// So we have to create 0xffff... mask on our own
unsigned width = EBPFTablePSAInitializerUtils::ebpfTypeWidth(typeMap, expr);
unsigned width = EBPFInitializerUtils::ebpfTypeWidth(typeMap, expr);
unsigned bytes = ROUNDUP(width, 8);

if (EBPFScalarType::generatesScalar(width)) {
Expand All @@ -383,7 +343,7 @@ class EBPFTablePSATernaryKeyMaskGenerator : public EBPFTablePSAInitializerCodeGe
bool preorder(const IR::Mask *expr) override {
// Mask value is our value which we want to generate
BUG_CHECK(expr->right->is<IR::Constant>(), "%1%: expected constant value", expr->right);
EBPFTablePSAInitializerCodeGen::preorder(expr->right->to<IR::Constant>());
CodeGenInspector::preorder(expr->right->to<IR::Constant>());
return false;
}
};
Expand Down
Loading