p4lang / p4c

P4_16 reference compiler
https://p4.org/
Apache License 2.0
679 stars 444 forks source link

Is struct as action parameter supported by any arch/target? #3760

Open qobilidop opened 1 year ago

qobilidop commented 1 year ago

I know that struct as action parameter is not supported by v1model/bmv2 yet. I heard this is a bmv2-specific limitation though. So I have the following questions to clarify:

  1. Is this currently a P4 language limitation or bmv2-specific limitation?
  2. If it's bmv2-specific, is there any other arch/target that supports struct action parameters already?

Here is a minimal example P4 code I wish to compile:

// example.p4
#include <core.p4>
#include <v1model.p4>

header eth_h {
    bit<48> dst_addr;
    bit<48> src_addr;
    bit<16> ether_type;
}

header custom_h {
    bit<8> a;
    bit<8> b;
}

struct header_t {
    eth_h eth;
    custom_h custom;
}

struct metadata_t {}

struct action_param_t {
    bit<8> a;
    bit<8> b;
}

// Parser
parser P(packet_in pkt, out header_t hdr, inout metadata_t meta, inout standard_metadata_t std_meta) {
    state start {
        pkt.extract(hdr.eth);
        pkt.extract(hdr.custom);
        transition accept;
    }
}

// VerifyChecksum
control VC(inout header_t hdr, inout metadata_t meta) { apply {} }

// Ingress
control I(inout header_t hdr, inout metadata_t meta, inout standard_metadata_t std_meta) {
    // This doesn't compile.
    action process_packet(action_param_t p) {
        hdr.custom.a = p.a;
        hdr.custom.b = p.b;
    }

    // This compiles.
    // action process_packet(bit<8> a, bit<8> b) {
    //     hdr.custom.a = a;
    //     hdr.custom.b = b;
    // }

    table forward {
        key = {
            hdr.eth.dst_addr: lpm;
        }
        actions = {
            process_packet;
        }
    }

    apply {
        forward.apply();
    }
}

// Egress
control E(inout header_t hdr, inout metadata_t meta, inout standard_metadata_t std_meta) { apply {} }

// ComputeChecksum
control CC(inout header_t hdr, inout metadata_t meta) { apply {} }

// Deparser
control D(packet_out pkt, in header_t hdr) {
    apply {
        pkt.emit(hdr.eth);
        pkt.emit(hdr.custom);
    }
}

V1Switch(P(), VC(), I(), E(), CC(), D()) main;

When I tried to compile it with v1model/bmv2, I got this error:

$ p4c --arch v1model --target bmv2 example.p4
example.p4(44): [--Werror=invalid] error: p: action parameters must be bit<> or int<> on this target
    action process_packet(action_param_t p) {
apinski-cavium commented 1 year ago

Is this currently a P4 language limitation or bmv2-specific limitation?

Looks to be BMV2 specific error message: backends/bmv2/common/action.cpp: "%1%: action parameters must be bit<> or int<> on this target", p);

The language supports it for sure.

jfingerh commented 1 year ago

Note that even if bmv2 were enhanced to support it, if you wanted to use P4Runtime API to configure the device, it is not supported by P4Runtime API, either. Of course P4Runtime API could be enhanced for that purpose.

qobilidop commented 1 year ago

@apinski-cavium Thanks for the pointer!

@jfingerh Thanks for the info! That's indeed where I'm getting at eventually. I was wondering how the struct action param would be represented in a p4info file based on the P4Runtime spec. Now I know the spec is not there yet.

apinski-cavium commented 1 year ago

I could see the struct being flatten into the parameters and then using param.field as the names of the actions parameters. That would be an easy spec change for P4Runtime too.

hesingh commented 1 year ago

The only catch with flattening struct to action parameters is where does one get "in/out/inout" parameter property from? Or do you not set any such property?

mihaibudiu commented 1 year ago

Only the directionless parameters appear in the api. Naming them is the hardest problem

hesingh commented 1 year ago

A struct name is unique to a P4 program and so is a table name. Use either or both with each field of the struct to name an action parameter. Add a new pass to p4c frontend to flatten struct used as action parameter.

hesingh commented 1 year ago

Also, it's not just flattening the struct, but also changing the references to the struct in the action body. Good thing that the p4c midend already has passes that not only flatten nested struct/header but also change references. See flatten* passes.

mihaibudiu commented 1 year ago

A struct type name is unique, but field names may clash with other parameters.

hesingh commented 1 year ago

I am assuming a struct is the only parameter of the action, so no other parameter exists to clash with. See my flattening example below.

struct action_param_t {
    bit<8> a;
    bit<8> b;
}

  action process_packet(bit<8> action_param_t_a, bit<8> action_param_t_b) {
        hdr.custom.a = action_param_t_a;
        hdr.custom.b = action_param_t_b;
    }
hesingh commented 1 year ago

Note a struct can be nested, can have a header, enum, header_union, and header stack Derived types. So the flattening has more work to do.

hesingh commented 1 year ago

P4Runtime does not support struct for an action parameter.

https://p4.org/p4-spec/docs/p4runtime-spec-working-draft-html-version.html#sec-action