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

Should this proposed encrypt_inline extern object method require a parameter of type packet_in? #1192

Open
jfingerh opened this issue Nov 26, 2022 · 6 comments
Labels

Comments

@jfingerh
Copy link
Contributor

In the PNA work group, Alan Lo has proposed an extern object with a method that can do all of the following things described below when it is called (I may be omitting some details -- feel free to ask if you are curious and I can flesh it out more).

Reference: See the extern object method named encrypt_inline in this PR: p4lang/pna#59

In the current PNA proposal, there would be a "main parser", "main control", and "main deparser". "main control" corresponds roughly with the "ingress" or "egress" controls in switch architectures, but there are no separate controls based on packet direction, but a combined one.

This method encrypt_inline would be called from within the main control, and would cause the following to occur before it returned:

(a) invoke the main deparser, with the current values of all headers in the headers struct H, and the current values of user-defined metadata fields, resulting in a temporary and local value of type packet_out representing the sequence of bytes in that deparsed packet.
(b) pass this deparsed packet's byte sequence and some other extern-specified parameters to a fixed function block that encrypts or decrypts the packet, resulting in a stream of bytes stored in a local temporary instance of packet_in.
(c) This local temporary packet_in is then passed as input to the user's main parser, along with some yet-to-be-defined values of intrinsic metadata and user-defined metadata, resulting in new parsed headers and user-defined metadata, which are returned through inout parameters of encrypt_inline.
(d) Now the encrypt_line method returns, and the rest of the user's main control P4 code executes as normal.

I see two possible ways to think about this packet_in instance, and in particular the question of "how does the offset of the first non-extracted byte from the user's parser come to affect the deparsed packet payload?"

(1) This offset in the packet is passed via hidden means defined by the PNA architecture, and thus the encrypt_inline method can read its current value, and modify it as needed, via these same hidden means that are typically used by other P4 architectures that somehow deparse a packet and append the payload after the headers emit'ed by the deparser code, even though the deparser does not have any instance of packet_in as a parameter.

(2) The language design work group believes it would be somehow cleaner if encrypt_inline should have an instance of packet_in as a parameter. If so, then it seems that the main control should also have a parameter with type packet_in, so that this value can be passed by the user's P4 code to the encrypt_inline method.

When we last discussed this issue in the PNA work group, I think Nate Foster and I were leaning towards (2) as the correct answer, but while writing up this description, it occurred to me that (1) also seems very reasonable, given how deparsers somehow work today in all other P4 architectures.

If we think (2) is preferable for some reason, then it might be nice to generalize this text describing packet_in in the current P4 language spec:

Section 13.8 "Data Extraction"

"The packet_in extern is special: it cannot be instantiated by the user explicitly. Instead, the architecture supplies a separate instance for each packet_in argument to a parser instantiation."

@jfingerh
Copy link
Contributor Author

@loalan @jnfoster I created this issue to follow up on a recent discussion in PNA group.

@jnfoster
Copy link
Collaborator

I might be misremembering, but I don't recall expressing a preference for option (2) -- I'm afraid I don't yet understand the precise behavior of the proposed extern in enough detail to have an opinion.

Regarding hidden state related to the packet, I don't believe the offset and payload are not visible in the P4 program, except indirectly using information like the packet length. So, on its own, it doesn't seem to violate the language specification for these data to be modified by an extern. It may be a confusing design for programmers, and therefore, not the ideal design, but it also seems fundamental to any inline crypto extern which operates on the entire packet. So I'm not objecting yet.

Regarding the text in 13.8, I also don't think it is problematic. The packet_in extern does not have a constructor, so it's true that the user cannot instantiate a packet_in object. Instead, the architecture has to supply it. We are not promising that the non-visible (i.e., in P4) state of the extern can only be modified in P4 blocks that have a reference to an instance.

I think I will still have issues with the proposal, but I need to understand it first. One concern relates to state in the program's parser. For instance, suppose it had a local register. I believe it would violate the (only) restriction on externs mentioned in the specification to allow the value of that register to be modified by this silent invocation of the parser.

In general, extern functions are arbitrarily powerful: they can store information in global storage, spawn separate threads, “collude” with each other to share information — but they cannot access any variable in a P4 program. With copy-in/copy-out semantics the compiler can still reason about P4 programs that invoke extern functions.

This limitation could possibly be side-stepped if we said that the extern works on a fresh instance of the parser. But then how do we know which parser is meant, given that a program may define several?

Let's talk at tomorrow's Arch WG meeting...

@mihaibudiu
Copy link
Contributor

There is already a proposal to add a constructor to packet_in here: #883
This would synthesize a packet_in from a varbit value, allowing one to parse the varbit using parser constructs.

@jfingerh
Copy link
Contributor Author

Thanks for the pointer, Mihai, but I do not see that this proposal is aided by having a constructor for a packet_in instance.

If approach (1) in my comment above is reasonable, then the main control doesn't even need a parameter of type packet_in, either.

@jfingerh
Copy link
Contributor Author

I spoke with Nate Foster about this issue briefly, and I plan to talk one-on-one with Alan Lo about the proposal that led to this question, to see what next steps might be.

@mihaibudiu
Copy link
Contributor

Is there a mock program that uses the proposed API?

@mihaibudiu mihaibudiu added the PNA label May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants