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

emit() does not support structs #630

Closed
vgurevich opened this issue May 14, 2017 · 3 comments
Closed

emit() does not support structs #630

vgurevich opened this issue May 14, 2017 · 3 comments
Labels
bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed.

Comments

@vgurevich
Copy link

According to p4_16 specification:

For derived type, emit recursively proceeds on fields:
. . .
If the argument is a struct containing multiple fields, the emit is recursively applied to each component of the struct in the order of their declaration in the struct.

However, the attempt to pass a struct to emit results in the following message:

struct my_headers_t {
    ethernet_t     ethernet;
    vlan_tag_t [2] vlan_tag;
    mpls_t [5]     mpls;
    arp_t          arp;
    arp_ipv4_t     arp_ipv4;
    ipv4_t         ipv4;
    icmp_t         icmp;
}

. . . 

control MyDeparser(
    packet_out      packet,
    in my_headers_t hdr)
{
    apply {
        packet.emit(hdr);
   }
}

results in the following message:

l3_switch.p4(413): error: hdr: emit only supports header and stack arguments, not struct my_headers_t
        packet.emit(hdr);
                    ^^^
l3_switch.p4(110)
struct my_headers_t {
       ^^^^^^^^^^^^
@mihaibudiu
Copy link
Contributor

There is a PR queued with this feature.

@mihaibudiu
Copy link
Contributor

I lied, the PR is not yet submitted, it is only a commit in my repo, until I figure out some issues with BMv2.
I may split it into two PRs if a solution for this is needed soon.

@mihaibudiu mihaibudiu added the bug This behavior is unintended and should be fixed. label May 14, 2017
@vgurevich
Copy link
Author

It is definitely an extremely useful feature that can relieve a lot of people from the necessity of spending time writing trivial deparsers. Once we allow header_unions in structs it will be doubly-useful and will also make code a lot safer.

I wanted to demonstrate it in the training slides, but... So far, I decided to keep the slide, but put a "Not Implemented yet" note :(

mihaibudiu pushed a commit to mihaibudiu/p4c-clone that referenced this issue May 15, 2017
@mihaibudiu mihaibudiu added the fixed This topic is considered to be fixed. label May 15, 2017
mihaibudiu pushed a commit to mihaibudiu/p4c-clone that referenced this issue May 16, 2017
mihaibudiu pushed a commit that referenced this issue May 24, 2017
* Support emits of structs; test parsing of varbit fields

* Fixes issue #447 and #630

* Updated reference outputs

* Additional tests and bug-fixes for varbits
mihaibudiu pushed a commit that referenced this issue May 30, 2017
* Fix for issue #638

* IRGen fixes

- support for autogenerating methods in nested classes
- support for friend functions
- misc bug fixes

* cpplint fixes

* Update comment

* Minor change in bmv2 backend: set action_const (#652)

In the bmv2 JSON, a match-table can indicate that its default action is
constant (using 'action_const') and that the default action entry
(action + action parameter values) is constant (using
'action_entry_const'). `action_entry_const == true => action_const ==
true`. The bmv2 backend was setting 'action_entry_const' correctly, but
'action_const' was always set to false. Note that P4_16 does not make a
difference between the 2, the presence of `const` in the P4_16 source
implies that both attributes should be true.

* Fix for issue #629 (#636)

* Support emits of structs; test parsing of varbit fields (#634)

* Support emits of structs; test parsing of varbit fields

* Fixes issue #447 and #630

* Updated reference outputs

* Additional tests and bug-fixes for varbits

* Fix for issue #638
* Allow empty actions list in tables
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This behavior is unintended and should be fixed. fixed This topic is considered to be fixed.
Projects
None yet
Development

No branches or pull requests

2 participants