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

[P4TC] Align key components to the nearest 8-bit size #5024

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

komaljai
Copy link
Contributor

@komaljai komaljai commented Nov 21, 2024

As of now, whenever there is a key component that is a bit-type which doesn't correspond to primitive C type (u8, u16, u32, ...),
the compiler generates it as using the closest primitive C type that is larger then X in bit
For example -

bit<24> dest_op;

dest_op here is converted into u32 like below -

u32 dest_op;

This won't work, because the P4TC side is expecting the bit<24> (dest_op) to be contained in 3 bytes and not 4. All key components are expected to be 8-bit aligned. The new declaration for 'dest_op' would be -

u8 dest_op[3];

For all operations, compiler will call function 'getPrimitiveXX()' which will convert u8[] into primitive type (u32 or u64).
For reverse assignment operations compiler calls 'storePrimitiveXX()' which converts primitive types to u8 [ ] array.

@vbnogueira @jhsmt

@komaljai komaljai added the p4tc Topics related to the P4-TC back end label Nov 21, 2024
Copy link
Contributor

@vbnogueira vbnogueira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small compilation issues
Will do another round after running more complex tests

testdata/p4tc_samples_outputs/calculator_parser.c Outdated Show resolved Hide resolved
testdata/p4tc_samples_outputs/calculator_parser.h Outdated Show resolved Hide resolved
backends/tc/ebpfCodeGen.cpp Outdated Show resolved Hide resolved
backends/tc/ebpfCodeGen.cpp Outdated Show resolved Hide resolved
backends/tc/ebpfCodeGen.cpp Outdated Show resolved Hide resolved
backends/tc/ebpfCodeGen.cpp Outdated Show resolved Hide resolved
@vbnogueira
Copy link
Contributor

As we discussed on the meeting, here's an example that shows the sum bug sum_example.tar.gz
You'll see the bug in generated/sum_example_control_blocks.c line 85

@komaljai komaljai force-pushed the align_key_bytewise branch 2 times, most recently from cb20ec1 to e5a365e Compare December 12, 2024 10:27
@vbnogueira
Copy link
Contributor

The PR looks ok, but can you rebase on top of main when you have some time?
There are fixes in the merged skb metadata PR which I need to test this PR with

@komaljai komaljai force-pushed the align_key_bytewise branch 3 times, most recently from 998f749 to ffca7b6 Compare January 7, 2025 06:20
@komaljai komaljai requested review from vbnogueira and fruffy and removed request for vbnogueira January 7, 2025 06:20
@komaljai
Copy link
Contributor Author

Hi @vbnogueira , @fruffy , Can you review the PR once?

@vbnogueira
Copy link
Contributor

Hi @vbnogueira , @fruffy , Can you review the PR once?

Sorry, we got a bit overwhelmed, but will do today

Copy link
Contributor

@vbnogueira vbnogueira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aside from these small issues, the PR seems ok

@komaljai komaljai requested a review from vbnogueira January 16, 2025 13:11
Copy link
Contributor

@vbnogueira vbnogueira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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.

2 participants