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

p4c-dpdk compiling 2023-Sep-13 version of DASH P4 code produces spec file that fails to load #4156

Closed
jafingerhut opened this issue Sep 13, 2023 · 3 comments
Labels
bug This behavior is unintended and should be fixed. dpdk Topics related to the DPDK back end

Comments

@jafingerhut
Copy link
Contributor

Note: This issue was discovered using the latest version of p4c-dpdk as of 2023-Sep-13 (commit SHA mentioned below). It is true that this version of p4c-dpdk DOES FIX these earlier issues:

However, there is a newer version of DASH P4 code since 2023-Apr when those issues were filed, and when testing with the latest version, p4c-dpdk produces a .spec file that fails to load into the P4-DPDK software switch. Hence the creation of this issue to track this.

Version of p4c source used to build p4c-dpdk:

$ git log -n 1 | cat
commit 5b76a26e4afbeb041d4eb47faa32e589521a4a50
Author: Usha Gupta <[email protected]>
Date:   Wed Sep 13 01:39:56 2023 +0530

    Fix whole header move instructions for DPDK (#4153)

I compiled p4c-dpdk binary from that source code.

Version of DASH P4 source code tested with:

$ git clone https://github.com/sonic-net/DASH
$ cd DASH
$ git log -n 1 | head -n 3
commit 3bbe99f19cb047770e43d63204ff1e94ffadca27
Author: Kamil Cudnik <[email protected]>
Date:   Thu Sep 7 19:34:04 2023 +0200

Compile the DASH P4 code, without changes from the version at the commit SHA shown above, for DPDK:

$ cd DASH/dash-pipeline/bmv2
$ mkdir -p pipe
$ p4c-dpdk -DTARGET_DPDK_PNA -DPNA_CONNTRACK \
    --arch pna \
    --p4runtime-files p4Info.txt \
    --bf-rt-schema bf-rt.json \
    --context pipe/context.json \
    -o pipe/dash_pipeline.spec \
    dash_pipeline.p4

This produced the following output files, included in the attached zip file:

  • p4Info.txt
  • bf-rt.json
  • pipe/context.json
  • pipe/dash_pipeline.spec

In earlier issues, the root cause was something that P4-DPDK did not expect to find in the .spec file and/or the context.json file. If that is the case here, I do not yet know what portion of those files might be the root cause of the loading failure. Hopefully someone can add a comment to this issue with more details when they find out.
dash-pipeline-output-2023-sep-13.zip

@jafingerhut jafingerhut added bug This behavior is unintended and should be fixed. dpdk Topics related to the DPDK back end dash-blocker labels Sep 13, 2023
@jafingerhut
Copy link
Contributor Author

jafingerhut commented Sep 14, 2023

Further investigation from some Intel colleagues revealed two causes for failing to load the spec file into the P4-DPDK software switch:

(a) attempting to perform arithmetic on 128-bit fields, which is not supported as of 2023-Sep in spec file instructions
(b) attempting to use a compl instruction in spec file, to do bit-wise negation of a value, i.e. the same effect as calculating ~x in C/C++ on an int x variable. This is not implemented as of 2023-Sep in spec file instructions.

Fixing (b) could be done either by implementing compl in P4-DPDK, or by replacing occurrences of it in p4c-dpdk back end with an expression like x ^ 0xffffffff (if x is a 32-bit value -- the constant should have as many 1 bits as the value x is wide).

For a P4 program that wants to do both of the following things:

(1) perform lpm matching on 128-bit fields, like IPv6 addresses, and
(2) in at least one expression of the program anywhere, perform arithmetic on 128-bit fields, e.g. (x & y) with 128-bit operands x and y, where one of those fields is either used in a table key with match kind lpm, and/or assigned to other fields that are

then my strong recommendation would be that 128-bit operations are supported by spec file instructions like bit-wise and, or, addition, subtraction, shifting, etc. Yes we might be able to do all of (2) by changing the P4 source code to break all larger-than-64-bit wide fields into multiple at-most-64-bit-wide fields, but then you cannot do (1), because you can have at most one lpm field in a table.

@jafingerhut
Copy link
Contributor Author

jafingerhut commented Sep 14, 2023

In case anyone is curious, a hacked-up version of the 2023-Sep-07 version of the DASH P4 program that eliminates all 128-bit wide fields, replacing them with a pair of 64-bit wide fields, is in this branch of my personal fork of the DASH project.

I mention it here only in case it helps someone experiment with any p4c-dpdk changes to get DASH P4 code compiling with it, but also as motivation for why supporting 128-bit operations in spec files would save P4 developers and p4c-dpdk back end developers a significant amount of work, not just for that program, but for any future P4 program written that wants to do arithmetic at least once on IPv6 addresses.

@jafingerhut
Copy link
Contributor Author

This issue has been fixed as of 2023-Sep-23 by modifying p4c-dpdk to reject P4 programs at compile time that attempt to perform arithmetic on operands larger than 64 bits. That fixes the symptoms reported by this issue. Closing this issue as resolved, but opening #4174 for the longer-term enhancement request of supporting such P4 programs in P4-DPDK.

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. dpdk Topics related to the DPDK back end
Projects
None yet
Development

No branches or pull requests

1 participant