Add support for Checksum extern and CRC16/32 algorithms to eBPF backend #3167
Conversation
| @@ -33,8 +33,35 @@ void DeparserBodyTranslatorPSA::processFunction(const P4::ExternFunction *functi | |||
| } | |||
| } | |||
|
|
|||
| void DeparserBodyTranslatorPSA::processMethod(const P4::ExternMethod *method) { | |||
| auto dprs = dynamic_cast<const EBPFDeparserPSA*>(deparser); | |||
There was a problem hiding this comment.
Do you need to CHECK_NULL?
Maybe you want to implement the to<> method for these classes?
backends/ebpf/psa/ebpfPsaParser.cpp
Outdated
| @@ -108,6 +116,25 @@ EBPFPsaParser::EBPFPsaParser(const EBPFProgram* program, const IR::ParserBlock* | |||
| visitor = new PsaStateTranslationVisitor(program->refMap, program->typeMap, this); | |||
| } | |||
|
|
|||
| void EBPFPsaParser::emitDeclaration(CodeBuilder* builder, const IR::Declaration* decl) { | |||
| if (decl->is<IR::Declaration_Instance>()) { | |||
There was a problem hiding this comment.
I see this pattern a lot, perhaps you can factor it out into a convenience function getSpecializedTypeArgument()?
There was a problem hiding this comment.
I added EBPFObject::getSpecializedTypeName() function.
backends/ebpf/psa/ebpfPsaParser.cpp
Outdated
| auto typeSpec = di->type->to<IR::Type_Specialized>(); | ||
| cstring name = di->name.name; | ||
|
|
||
| if (typeSpec != nullptr && |
There was a problem hiding this comment.
this may be a utility function too.
you may want to review the code committed previously for these patterns too.
| @@ -0,0 +1,133 @@ | |||
| /* | |||
There was a problem hiding this comment.
should we also add symlinks from testdata/p4_16_samples to these files?
There was a problem hiding this comment.
I think we shouldn't. What would be the value of adding PTF testfiles to testdata/p4_16_samples ?
There was a problem hiding this comment.
These programs would be used to test the p4test compiler. More tests are always good.
| ::error(ErrorType::ERR_UNEXPECTED, "Expected exactly 1 argument %1%", block); | ||
| return; | ||
| } | ||
| int type = di->arguments->at(0)->expression->to<IR::Constant>()->asInt(); |
There was a problem hiding this comment.
can it not be a constant?
maybe you should check
There was a problem hiding this comment.
or you can at least use checkedTo
|
|
||
| cstring CRCChecksumAlgorithm::reflect(cstring str) { | ||
| BUG_CHECK(crcWidth <= 64, "CRC checksum width up to 64 bits is supported"); | ||
| unsigned long long poly = std::stoull(str.c_str(), nullptr, 16); |
There was a problem hiding this comment.
is this enough validation?
|
|
||
| void CRCChecksumAlgorithm::emitUpdateMethod(CodeBuilder* builder, int crcWidth) { | ||
| // Note that this update method is optimized for our CRC16 and CRC32, custom | ||
| // version may require other method of update. To deal with byte order data |
| concatenateBits = true; | ||
| if (width > remainingBits) { | ||
| ::error(ErrorType::ERR_UNSUPPORTED, | ||
| "Sub-byte fields have to be aligned to bytes %1%", field); |
There was a problem hiding this comment.
I don't think I would understand this error message as a user of the compiler.
|
@mbudiu-vmw I think I've addressed all of the comments. |
|
Unfortunately this branch now has conflicts, which you will have to resolve before we can merge. |
Co-authored-by: Tomasz Osiński <[email protected]> Co-authored-by: Jan Palimąka <[email protected]>
Co-authored-by: Tomasz Osiński <[email protected]>
kmateuszssak
force-pushed
the
checksum-crc16-crc32
branch
from
ec7f559
to
bfb369d
Compare
This PR adds Checksum extern and adds support for CRC16 and CRC32 algorithms.
Co-authored-by: Tomasz Osiński [email protected]
Co-authored-by: Jan Palimąka [email protected]