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

Handle pna_main_parser_input_metadata_t fields in the tc backend's parser #4955

Merged

Conversation

vbnogueira
Copy link
Contributor

@vbnogueira vbnogueira commented Oct 9, 2024

As of now, referencing a field of pna_main_parser_input_metadata_t will cause the it to be generated in eBPF without any translation.

For example:

parser Ingress_Parser(
        packet_in pkt,
        out   my_ingress_headers_t  hdr,
        inout my_ingress_metadata_t meta,
        in    pna_main_parser_input_metadata_t istd)
{
    state mystate {
        ...
        meta->ingress_port = istd.input_port;
        ...
    }
    ...
}

Will generate something like:

mystate: {
    ...
    meta->ingress_port = istd.input_port;
    ...
}

This will cause a compilation error because istd is not defined in the ebpf code. This PR fixes this by either translating the fields to something ebpf understands or simply throwing an error if the field is unsupported.

@vbnogueira
Copy link
Contributor Author

@komaljai please take a look when you have some time

@komaljai komaljai added the p4tc Topics related to the P4-TC back end label Oct 10, 2024
@vbnogueira vbnogueira force-pushed the handle_pna_main_parser_input_metadata_t_tc branch 2 times, most recently from cbb4953 to 11acc9d Compare October 10, 2024 16:03
@komaljai komaljai force-pushed the handle_pna_main_parser_input_metadata_t_tc branch 2 times, most recently from f705d86 to d91defa Compare October 17, 2024 11:05
…rser

As of now, referencing a field of pna_main_parser_input_metadata_t will
cause the it to be generated in eBPF without any translation.

For example:

parser Ingress_Parser(
        packet_in pkt,
        out   my_ingress_headers_t  hdr,
        inout my_ingress_metadata_t meta,
        in    pna_main_parser_input_metadata_t istd)
{
    state mystate {
        ...
        meta->ingress_port = istd.input_port;
        ...
    }
    ...
}

Will generate something like:

mystate: {
    ...
    meta->ingress_port = istd.input_port;
    ...
}

This will cause a compilation error because istd is not defined in the ebpf
code. This commit fixes this by either translating the fields to something
ebpf understands or simply throwing an error if the field is unsupported.

Signed-off-by: Victor Nogueira <[email protected]>
@vbnogueira vbnogueira force-pushed the handle_pna_main_parser_input_metadata_t_tc branch from d91defa to 61c3079 Compare October 17, 2024 15:09
@vbnogueira vbnogueira force-pushed the handle_pna_main_parser_input_metadata_t_tc branch from 61c3079 to bdac2d7 Compare October 17, 2024 15:13
@vbnogueira
Copy link
Contributor Author

vbnogueira commented Oct 18, 2024

@komaljai the bazel test is failing, but it seems to be unrelated to the changes in this PR (seems to be something in p4_dpdk)
If that is the case, could we merge this without this test passing?

@fruffy
Copy link
Collaborator

fruffy commented Oct 18, 2024

This might be a cache problem, let me clear the actions cache.

@komaljai komaljai added this pull request to the merge queue Oct 21, 2024
Merged via the queue into p4lang:main with commit da04995 Oct 21, 2024
18 checks passed
@vbnogueira vbnogueira deleted the handle_pna_main_parser_input_metadata_t_tc branch November 4, 2024 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4tc Topics related to the P4-TC back end
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants