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

Add support for @p4runtime_translation #585

Open
fruffy opened this issue Jan 21, 2023 · 17 comments
Open

Add support for @p4runtime_translation #585

fruffy opened this issue Jan 21, 2023 · 17 comments

Comments

@fruffy
Copy link
Contributor

fruffy commented Jan 21, 2023

Hello,
I am running a PTF Python program using simple_switch_grpc and the P4Runtime API. The program is relatively simple, but I am seeing the following error.

base_test.P4RuntimeWriteException: Error(s) during Write:
	* At index 0: INVALID_ARGUMENT, 'Bytestring provided does not fit within 0 bits'

I am not sure what this means or how it can be fixed. Is there something that needs to be changed for the PTF test?

The only thing special about the P4 program are the following lines:

@p4runtime_translation("p4.org/psa/v1/PortId1_t" , 32) type PortIdUint_t PortId1_t;
@p4runtime_translation("p4.org/psa/v1/PortId2_t" , bit < 32 >) type PortIdUint_t PortId2_t;
@p4runtime_translation("p4.org/psa/v1/PortId3_t" , string) type PortIdUint_t PortId3_t;

issue2283_1-bmv2.json.txt
issue2283_1-bmv2.p4.txt
issue2283_1-bmv2.p4info.txt
issue2283_1-bmv2.py.txt
test_log.txt

@jafingerhut
Copy link
Contributor

It looks like you may be testing this input file? https://github.com/p4lang/p4c/blob/main/testdata/p4_16_samples/issue2283_1-bmv2.p4

In looking at the P4Info file produced by the latest version of p4c here: https://github.com/p4lang/p4c/blob/main/testdata/p4_16_samples_outputs/issue2283_1-bmv2.p4.p4info.txt

Note that there is a bitwidth field for fields port1 and port2, but not for port3, the one that has type PortId3_t where the p4runtime_translation annotation gives it the control plane type 'string'.

I suspect this lack of 'bitwidth' field is the root cause of the error you are seeing.

It sounds like perhaps there is not fully implemented support for type 'string' in a p4runtime_translation annotation?

@jafingerhut
Copy link
Contributor

As a check, you could try commenting out type PortId3_t and the uses of field port3 in that test program, and see if things work better.

@fruffy
Copy link
Contributor Author

fruffy commented Jan 22, 2023

Removing the @p4runtime_translation("p4.org/psa/v1/PortId3_t" , string) gets rid of the `Bytestring provided does not fit within 0 bits' error.

I tried to use a key with a string type, but I always get the error complaining about 0 bit. I am not quite sure what kind of configuration message is expected here.

Even after removing the string type, I get errors. This is caused by @p4runtime_translation overriding the control plane API. This is intentional, right? The error I get is
* At index 0: UNKNOWN, 'Error when adding match entry to target'
This happens even when I force the exact key to be 32-bit wide, after removing the string-based key.

@jafingerhut
Copy link
Contributor

The intent of the p4runtime_translation annotation is that the P4Runtime server (near the network device) is allowed to translate, i.e. change, the value provided by the P4Runtime client before it is programmed into the device's data plane, e.g. because the control plane wants to use 32-bit values for port ids, but the data plane only uses 9-bit values.

The default translation is no translation, which is probably what is happening in your testing. Thus 32-bit control plane values that do not fit in a 9-bit value are probably giving that error message.

I am not aware of whether the open source PI implementation, or the behavioral-model P4Runtime server code, has options to do fancier translations than the no-op / identity translation. If not, then it would be best to restrict oneself to values that fit within the data plane size.

@jafingerhut
Copy link
Contributor

How does one know what the data plane size is, you might reasonably next ask? I don't think you can tell from the P4Info file output by the compiler, because the intent is that the P4Info file is a controller-centric view of the API of the data plane, and in some cases omits details that are specific to the data plane.

@fruffy
Copy link
Contributor Author

fruffy commented Jan 22, 2023

So, in this case I have a key that is 16 bit wide in the data plane, but the @p4runtime_translation specifies 32 bit. Which control plane configuration is needed here? I tried keys that are either 16 and 32 bit, but both are rejected with an error message. The only time I can get the key to go through is when I set the annotation bit width to the same width as the key.

Is the implication that the open source P4Runtime server simply does not offer support?

@jafingerhut
Copy link
Contributor

I just tried running a PTF test that uses the P4Runtime API with only a few table add write requests, and passes perfectly fine without any modifications to it (and no uses of p4runtime_translation annotations), and simply by making one exact match field of one table have a type that is a type defined via type, with a p4runtime_translation annotation, I get a similar error message when the PTF test attempts to add an entry to that table.

It appears there might be a problem with simple_switch_grpc's implementation of handling such table add messages.

@antoninbas
Copy link
Member

There is no support for translation in the P4Runtime server (https://github.com/p4lang/PI)
The p4runtime_translation annotation is handled correctly by P4Info-generation in the p4c compiler, but that's about it.

@fruffy
Copy link
Contributor Author

fruffy commented Jan 23, 2023

Ah that is unfortunate. Thanks for letting me know. What is your estimate, how difficult is it to implement?

@jafingerhut
Copy link
Contributor

@smolkaj Do you use p4runtime_translation at all in your work?

@smolkaj
Copy link
Member

smolkaj commented Jan 24, 2023

Yes, we use p4runtime_translation in SAI P4/PINS. We would love to have support for it in BMv2, but haven't gotten around to working on it ourselves. Instead, we have various workaround -- admittedly, the current state of affairs is an ugly mess.

What is your estimate, how difficult is it to implement?

+1, do you have a sense @antoninbas?

@smolkaj
Copy link
Member

smolkaj commented Jan 24, 2023

@kheradmandG @jonathan-dilorenzo for visibility

@antoninbas
Copy link
Member

It's not overly complicated to support this in PI / bmv2, but in my view adding support is dependent on the definition of a scheme to specify translation mappings. So in a way this is blocked by p4lang/p4runtime#373

@smolkaj
Copy link
Member

smolkaj commented Jan 25, 2023

Makes sense, I agree. Let me see if we can somehow prioritize this.

@antoninbas
Copy link
Member

I'll move this issue to p4lang/PI

@antoninbas antoninbas transferred this issue from p4lang/behavioral-model Feb 10, 2023
@smolkaj
Copy link
Member

smolkaj commented Aug 2, 2023

@fruffy / @antoninbas can we rename this issue to something like "Add support for p4runtime_translation"?

@smolkaj
Copy link
Member

smolkaj commented Aug 2, 2023

@kyechou for visibility

@fruffy fruffy changed the title simple_switch_grpc: 'Bytestring provided does not fit within 0 bits' Add support for @p4runtime_translation Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants