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

P4_14 (v1.0.4): variable length header in add_header #429

Closed
kheradmand opened this issue Sep 25, 2017 · 2 comments
Closed

P4_14 (v1.0.4): variable length header in add_header #429

kheradmand opened this issue Sep 25, 2017 · 2 comments

Comments

@kheradmand
Copy link

kheradmand commented Sep 25, 2017

In the specification of add_header primitive action, it is stated that

If the header instance was invalid, all its fields are initialized to 0.

I was wondering if we have a header like this:

header_type ht {
  fields {
    ...
    opt : *;
  }
  length : <length_exp>
  max_length : <max_length>
}

header ht h;

and (while h is invalid) we call

add_header(h);

what should happen in a case where:

  • eval(<length_exp>) > <max_length>
  • eval(<length_exp> * 8) < <sum_of_fixed_width_fields>

(As far as I understood) In the parsing phase, p4_pe_header_too_long or p4_pe_header_too_short will be thrown in such cases. But it is not clear to me what should happen in match-action phase. Is it compile time error? runtime error? undefined behavior?

@kheradmand
Copy link
Author

Well, an easy (but probably not preferable) way around this is to just disallow add_header with variable length headers.

@vgurevich
Copy link
Contributor

I think that the first problem is that we do not know the exact semantics of the length attribute. The wording in the spec really doesn't tell us much about how it is supposed to be used (and I quote):

  • The length attribute specifies an expression whose evaluation gives the length of the header in bytes for variable length headers.
    – It must be present if the header has variable length (some field has width "*").
    – A compilerwarning must be generated if it is present for a fixed length header.
    – Fields referenced in the length attribute must be located before the variable length field.

In reality, this word "specifies" means nothing. It is not clear whether it is used for parsing, deparsing, processing in the controls or anything else. We can only do our best guess (and P4_16 spec is doing a much better job at this).

Also, the spec is quite silent about which primitives (if any) can be used with the variable length fields.

So, my best guess will be the following:

  1. Variable length fields (as well as the length attribute) are only used for parsing/deparsing
  2. Either there are no primitives that can be used with such fields or, at best, you can use modify_field, probably copying one such field into another
  3. If modify_field is allowed, the copying should always done over the maximum allowed size for that field
  4. The fields that are evaluated in the length attribute must be updated explicitly.

Based on that, disallowing add_header(h) would probably bee too harsh. For example, it should totally be possible to have two headers of the same type, e.g. h1 and h2. Suppose header h1 was extracted and is valid. It should be totally possible to either copy h1 into h2 or (which is the same), add h2 and then copy some fields from h1 there (including h1.op), fill other fields with whatever you need and be done.

Thanks,
Vladimir

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants