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

What happens with enums in the parser? #2331

Closed
fruffy opened this issue Apr 21, 2020 · 6 comments
Closed

What happens with enums in the parser? #2331

fruffy opened this issue Apr 21, 2020 · 6 comments
Labels
question This is a topic requesting clarification.

Comments

@fruffy
Copy link
Collaborator

fruffy commented Apr 21, 2020

More questions. I have been looking at the sample program psa-example-digest-bmv2.p4 recently, in particular at the line where an enum is a member of the header that is being parsed.
This seems to be okay according to the type nesting rules of the spec, although the simple_switch backend does not accept such combinations, only psa.

I am bit unclear what this means though. As an example take a stripped version of the psa-example program. In the Inlining pass, the sub-parser with out parameters is inlined from

enum bit<16> dummy_enum {
    a = 16w0x1,
    b = 16w0x2
}
header H {
    bit<8>     a;
    dummy_enum test;
}
struct Headers {
    ethernet_t eth_hdr;
    H          h;
    dummy_enum test;
}

parser sub_p(packet_in pkt, out Headers hdr) {
    state start {
        transition accept;
    }
}
parser IngressParserImpl(packet_in pkt, out Headers hdr, inout metadata meta, in psa_ingress_parser_input_metadata_t istd, in empty_metadata_t resubmit_meta, in empty_metadata_t recirculate_meta) {
    @name("sub_parser") sub_p() sub_parser_0;
    state start {
        pkt.extract<ethernet_t>(hdr.eth_hdr);
        pkt.extract<H>(hdr.h);
        sub_parser_0.apply(pkt, hdr);
        transition accept;
    }
}

to

parser IngressParserImpl(packet_in pkt, out Headers hdr, inout metadata meta, in psa_ingress_parser_input_metadata_t istd, in empty_metadata_t resubmit_meta, in empty_metadata_t recirculate_meta) {
    state start {
        pkt.extract<ethernet_t>(hdr.eth_hdr);
        pkt.extract<H>(hdr.h);
        hdr.eth_hdr.setInvalid();
        hdr.h.setInvalid();
        transition sub_p_start;
    }
    state sub_p_start {
        transition start_0;
    }
    state start_0 {
        transition accept;
    }
}

Ultimately, enums are converted to typical bitvectors:

header H {
    bit<8>  a;
    bit<16> test;
}

struct Headers {
    ethernet_t eth_hdr;
    H          h;
    bit<16>    test;
}

What are the values of the enum in the Headers struct and is it meant to be parsed? Is it possible to set it as invalid? Or what happens if this data structure is passed into a function with out parameters? I can't quite figure this out.

@jafingerhut
Copy link
Contributor

The v1model architecture implemented by simple_switch has this in its documentation: https://github.com/p4lang/p4c/blob/master/p4include/v1model.p4#L711-L712

"H must be a struct where every one if its members is of type header, header stack, or header_union."

I do not believe that the p4c compiler enforces this today, but it is definitely the intent that type H is a struct that only contains members with one of those types. So the example you give with a member that has type dummy_enum would not be supported, and would ideally give an error from compiler code specific to the v1model architecture the checked that restriction.

The same goes for the PSA architecture, although it might not be explicitly documented right now, as it is for the v1model architecture.

headers, both normal headers and elements of header stacks, have a 'hidden' valid bit, but enum types do not, so it is illegal to try to call setValid, setInvalid, or isValid on them.

The P4_16 language specification does say (see below) that all direction out parameters, and all local variables, that have a type that is a header, are initially invalid. The values of all other fields are undefined then, but the validity of a header is one of the few things that the P4_16 language specification gurrantees will be initialized, even with no explicit statements that explicitly do so in the P4 program.

Section 6.7 "Calling convention: call by copy in/copy out" of the P4_16 language spec says this:

"out parameters are uninitialized (parameters of type header or header_union are set to “invalid”) and are treated as l-values (See Section 6.6) within the body of the method or function. An arguments passed as an out parameter must be an l-value; after the execution of the call, the value of the parameter is copied to the corresponding storage location for that l-value."

I had a harder time finding a justification for the guarantee that local variables with type header are initialized to invalid, but I think it is the following: Section 7.2.2 "Header types" says:

"When a header is created its “validity” bit is automatically set to false."

Please ask again if I missed any aspect of your question.

@jafingerhut
Copy link
Contributor

Here is an issue I created for the PSA specification that suggests the restriction on the type of H should be similar to those described above for v1model: p4lang/p4-spec#722. It is not yet part of any published PSA specification, though.

@fruffy
Copy link
Collaborator Author

fruffy commented Apr 21, 2020

Thank you! Did not know about that spec issue, that simplifies things a lot, great.
So in summary:
If an enum is contained in an header and the header is set invalid, the same semantics as discussed in #2323 apply, correct?

@mihaibudiu
Copy link
Contributor

Your enum is not within a header.
There is nothing special about enums in parsers; they are treated the same wherever they are.
v1model and psa put some constraints on the structure of the "headers" parameter, but that has nothing to do with the language.

@mihaibudiu mihaibudiu added the question This is a topic requesting clarification. label Apr 21, 2020
@jafingerhut
Copy link
Contributor

Well, he has two enums in his example. One is inside of a header, and one was not. Most of my comment was addressed to the fact that the enum that was inside of the Headers struct, but not inside of a header type, is not really supported, so should not be in any valid example code, even though the p4c compiler will not give any errors or warnings about it.

For most practical purposes an enum backed by a bit<W> type, aka a serializable enum, is nearly the same as a bit<W> type, whether such a field is within a header or not. There are some details about whether it requires a cast to assign it to a variable/field with a true bit<W> type, or vice versa, but except for that, not much difference at all.

So yes, all of the same issues for when a field inside of a header as discussed in #2323 apply here, too.

@fruffy
Copy link
Collaborator Author

fruffy commented Apr 22, 2020

Thanks, that answers my questions.

@fruffy fruffy closed this as completed Apr 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question This is a topic requesting clarification.
Projects
None yet
Development

No branches or pull requests

3 participants