-
Notifications
You must be signed in to change notification settings - Fork 21
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
Example: crypto inline method addition to crypto_accelerator object #59
base: main
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
examples/ipsec-inline.p4
Outdated
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that's possible. This is just an example, to focus on how the inline model works
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add comment on preconditions and postconditions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add side effects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it may need packet in as well
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.