Emitting of arbitrary data is not implemented

[vgurevich]

P4_16 spec and core.p4 define two methods for packet_out

extern packet_out {
    void emit<T>(in T hdr);
    void emit<T>(in bool condition, in T data);
}

The compiler doesn't recognize the second method (conditional emit of arbitrary data) and reports the following error:

l3_switch.p4(416): error: packet.emit: argument must be a header, stack or union, or a struct of such types
        packet.emit(hdr.ethernet.isValid(), 16w0x0001);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Searching through the testcases didn't turn up any examples for this construct either.

[mihaibudiu]

This may also be a spec bug, I don't know whether we want to support arbitrary data. We agreed that the first emit only supports headers and derivatives. I am not sure this can even be implemented without some kind of special support from bmv2.

[cc10512]

The spec clearly states that emitting arbitrary data is allowed. BMV2 currently only supports headers.

[jafingerhut]

I recall at least one conversation at a recent language design meeting where the topic of whether emit() should work on bit vectors (directly, not inside of a header), and it was decided that should be decided post P4-16 spec release, and at least the initial P4-16 spec release would not allow that.

It seems reasonable to me if the same restrictions apply equally to both the 1-arg and 2-arg variants of emit. I can open a PR on the spec for this and see what happens.

[jafingerhut]

But wait, isn't the issue here that @vgurevich is trying the 2-argument emit, and it is not working, even when he restricts himself to a type that the 1-argument emit supports?

[vgurevich]

First of all, I'd like to know how do we plan to emit variable-length headers :) Something tells me we will need a two-argument emit for that.

Emitting arbitrary data, I think is also quite useful. If we decide to postpone it, it's fine, but then it will be a spec bug :) In general, I think that real deparsers (e.g. in PSA) will be much more complex than the spec currently suggests.

[jnfoster]

Why? Since the data is already stored in a typed/structured representation, won't emitting be trivial? (Unlike parsing where we have to go from a sequence of bits to typed structures.) I must be missing something. N

…

On Sun, May 14, 2017 at 11:44 AM Vladimir Gurevich ***@***.***> wrote: First of all, I'd like to know how do we plan to emit variable-length headers :) Something tells me we will need a two-argument emit for that. Emitting arbitrary data, I think is also quite useful. If we decide to postpone it, it's fine, but then it will be a spec bug :) In general, I think that real deparsers (e.g. in PSA) will be much more complex than the spec currently suggests. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#629 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABwi0qbbbgiQqZitOT_ffcfUgexzmVQQks5r50uAgaJpZM4NaYE4> .

[jnfoster]

Also, why do we need conditional emit at all? We have general purpose conditionals. I'd rather write if(b) emit(d); than emit(b, d); anyway... If the latter is what a target supports, a compiler can transform code block comprising conditionals+emit statements into blocks of only conditional emits via a fairly trivial code transformation.

…

-N On Sun, May 14, 2017 at 11:46 AM Nate Foster <jnfoster@cs.cornell.edu> wrote:

Why? Since the data is already stored in a typed/structured representation, won't emitting be trivial? (Unlike parsing where we have to go from a sequence of bits to typed structures.) I must be missing something. N On Sun, May 14, 2017 at 11:44 AM Vladimir Gurevich < ***@***.***> wrote: > First of all, I'd like to know how do we plan to emit variable-length > headers :) Something tells me we will need a two-argument emit for that. > > Emitting arbitrary data, I think is also quite useful. If we decide to > postpone it, it's fine, but then it will be a spec bug :) In general, I > think that real deparsers (e.g. in PSA) will be much more complex than the > spec currently suggests. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <#629 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/ABwi0qbbbgiQqZitOT_ffcfUgexzmVQQks5r50uAgaJpZM4NaYE4> > . >

[jafingerhut]

@vgurevich The 2-argument emit is not needed for a header containing varbit fields. The spec allows headers containing varbit<W> fields to be emit'ed via either the 1- or 2-argument emit. Whether the current implementation does it correctly I don't happen to know, but it ought to.

The 2-argument emit seems to me completely superfluous, if it is limited to the same types that the 1-argument emit supports, because it can trivially be implemented via the 1-argument emit with an 'if' wrapped around it in the deparser control block. I am perfectly happy if the 2-argument emit is limited to the same types supported by the 1-argument emit in the P4-16 1.0 spec.

Targets are allowed to extend either or both of the emit variants to work on other types if they want to.

[vgurevich]

@jnfoster : I do agree that the conditional part of 2-argument emit is superfluous. I think it came about because the 1-argument emit is expressed via 2-argument one. I mean, one could also argue that the check for the header validity in 1-argument emit is superfluous, but the fact is that with this check the code is much more compact and easier to read.

More importantly, however, is the fact that the 1-argument emit is explicitly defined for headers and the types derived from headers (header_unions and structs) only, precisely because it does this validity check. It can be stretched to handle data ("if a type doesn't have isValid() method it is assumed to return true"), but that might be a little too much.

[vgurevich]

@jafingerhut I think you are right. Section 8.8 specifically suggests that this should be possible. I think we should try it.

[cc10512]

I thought there was a desire not to introduce control flow in the deparser and thus, emit requires the valid bit in headers. @jnfoster makes the argument that he prefers the explicit if statement. I do too, but this is a matter of taste.
I guess you can put arbitrary data using emit, and assume that the valid bit is always true, but then you don't need the condition at all. The question with allowing non-header data in the deparser is what do you do with the packet at the other end? Someone will have to parse the packet header, and then someone will have to declare a header type for it. Can you give an example of where this would work without declaring a header?

[jafingerhut]

@cc10512 I will let @jnfoster speak for himself, but I would guess he is not proposing eliminating the header validity check in the 1-argument emit. He is saying that 2-argument emit can be written with 1-argument emit wrapped inside of an 'if' statement.

The only possible reason I can imagine wanting to avoid explicit 'if' statements in a deparser are target-specific restrictions on deparser complexity. I would think that it should be just as easy for a compiler for a target to detect too much complexity in a deparser control block, as it can in other control blocks (even if that maximum implemented complexity is lower in a deparser than in other control blocks).

[mihaibudiu]

In the spirit of simplicity I vote for removing the 2-argument emit altogether from the spec.

[vgurevich]

@mbudiu-vmw I guess that's OK. If someone wants to output some custom stuff, it won't hurt to require them to define a header.

We will probably have to revisit this a little later anyway, but for now we are good as long as the other issues (emitting a struct, etc.) are resolved.

[jnfoster]

I think the proposal is: packet_out has one method declared with the following type;

void emit<T>(in T data);

that can be used with any type whatsoever.

In the short term, a custom header might be needed until p4c catches up. But long-term, I don't think it would be necessary.

Did I get that right?

[jafingerhut]

@jnfoster and this 1-argument emit is still like the currently defined one, in that it uses the valid bit of a header, header_union, or the multiple header valid bits in a header stack to control whether anything is emitted? If so, sounds good to me. Keeping that header validity check inside of emit's implementation does seem very convenient and useful.

[jnfoster]

Yep. In this design, emit would be a "magical" polymorphic function that needs to use type introspection to do its job (as does extract, by the way).

I agree that emit should do the right thing wrt validity bits.