Example: crypto inline method addition to crypto_accelerator object#59
Example: crypto inline method addition to crypto_accelerator object#59loalan wants to merge 1 commit intop4lang:mainfrom
Conversation
|
We should separate out hidden/implicit side effects such as 'reparse' and associated state modification from crypto.encrypt/decrypt methods and generalize it so that it can be used for other accelerators as well. I see the following advantages of this -
Overall I think it will keep accelerator functions clean and code slightly more portable across targets. |
examples/crypto-inline.p4
Outdated
| } | ||
|
|
||
| enum bit<2> crypto_mode_e { | ||
| TUNNEL = 0, |
There was a problem hiding this comment.
I think the hardware accelerator should be wire protocol agnostic. We can specify offset/len of the cipher-text in the packet instead of defining these modes. That will allow programmer to use the engine for other protocols.. such as DTLS or any other custom protocol that uses AES-GCM crypto.
There was a problem hiding this comment.
I suppose the difference here is in the async model encrypt/decrypt is in the HW block, and encap/decap is in P4.
In the inline model both encap/encrypt and decrypt/decap are in the HW block. This is because both deparsing and encryption are triggered inline, prior to the P4 deparse stage. Similar for decrypt and removal of the IPSec headers, this executes inline prior to P4 deparser. (Subsequently there is no need to reinject in order check results and to run it through the deparser).
This is also somewhat different from existing P4 implementations, but seem to be what is needed to make this packet transformation inline.
|
IMHO: This is indeed good example for what we may expect to see with inline accelerators; but there are other (than IPSec) inline accelerations that will need to be expressed as well. The value of inline acceleration in this domain is clear. Also, most not-trivial accelerators will likely change a multitude of things. My hope is that the solution we land of is general enough to be reused in other inline accelerators that will be needed in the near future by all vendors. |
045fb15 to
c005d11
Compare
| key = { | ||
| // For encrypt case get sa_idx from parser | ||
| // for decrypt case esp hdr spi value will be used as sa_idx | ||
| main_meta.sa_index : exact; |
There was a problem hiding this comment.
Would it make sense to have separate match-action tables for encryption and decryption?
For example, encrypt table could match on the destination IP address, while decrypt table could match on source and destination IP address, as well as SPI value. (This is similar to the approach proposed in P4-IPsec)
There was a problem hiding this comment.
Sure, that's possible. This is just an example, to focus on how the inline model works
There was a problem hiding this comment.
Actually, what you proposed is simpler than setting parser metadata. I'll look at rewriting the example app.
3ca66b7 to
e541c5c
Compare
| // packet. | ||
| // H - inout Headers that is the output of the parser block | ||
| // M - inout Metadata that is from the parser block and shared with the control | ||
| crypto_results_e encrypt_inline<H,M,T,S,I>(inout H hdr, inout M meta, |
There was a problem hiding this comment.
add a comment of order of operations. e.g. deparser or byte stream update must occur when the extern is invoked
There was a problem hiding this comment.
add comment on preconditions and postconditions
e541c5c to
0b9c78a
Compare
…). The example adds two methods for encrypt/decrypt that assumes that inline accelerators operate immediately on the packet (e.g. deparse, decrypt and reparse). Packet recirculation is not necessary for either inline method. The example shows the use of inline encrypt and decrypt, as well as how the crypto accelerator results can be used.
0b9c78a to
37ca96c
Compare
|
https://p4.org/p4-spec/docs/P4-16-working-spec.html#sec-packet-data-extraction |
thomascalvert-xlnx
left a comment
There was a problem hiding this comment.
I suspect that the programming model proposed here (pseudo-recirculation) will be a source of confusion and bugs. For example, pre-conditions in the control block (leading up to the decrypt_inline call) could change, resulting in broken programmer's assumptions as well as inability to perform static analysis. Most targets are likely to implement this functionality by recirculating anyway, so it's not clear to me what we are gaining here with this clunky passing of pkt/hdr/meta arguments (imho).
The same effect can be easily achieved explicitly by the programmer:
control {
if (recirculating == false) {
(...) // set things up
result = decrypt_inline(...);
if (result == SUCCESS) {
recirc(); // or whatever
} else {
(...) // deal with error
}
}
else {
(...) // continue processing decrypted packet
}
}
With the advantage that it can then be used for other purposes too.
| in T enable_auth, | ||
| in bit<32> spi, | ||
| in S seq, | ||
| in I iv); |
There was a problem hiding this comment.
Why include all these arguments rather than reuse the set_iv(), set_key(), etc. methods above?
| @@ -122,6 +128,55 @@ extern crypto_accelerator { | |||
| void enable_encrypt<T>(in T enable_auth); | |||
| void enable_decrypt<T>(in T enable_auth); | |||
|
|
|||
There was a problem hiding this comment.
Do we expect any targets to support both enable_encrypt and encrypt_inline ? If not, how would a program be written to be portable?
This example expands on the definition of crypto_accelerator (#53).
The example adds two methods for encrypt/decrypt that assumes that inline accelerators operate immediately on the packet (e.g. deparse, decrypt and reparse). Packet recirculation is not necessary for either inline method.
The example shows the use of inline encrypt and decrypt, as well as how the crypto accelerator results can be used.