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

Support for specifying type translations as part of the forwarding configuration. #373

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

smolkaj
Copy link
Member

@smolkaj smolkaj commented Feb 9, 2022

No description provided.

@chrispsommers
Copy link
Collaborator

@smolkaj is this still in play?

@smolkaj
Copy link
Member Author

smolkaj commented Apr 13, 2023

We still want something like this eventually, but we aren't currently actively working on it.

@smolkaj
Copy link
Member Author

smolkaj commented Aug 2, 2023

I think we'd also want to add a semi-static & semi-dynamic mapping.

@antoninbas any thoughts on whether it would be more or less preferable to add this as a separate RPC?

@antoninbas
Copy link
Member

I think we'd also want to add a semi-static & semi-dynamic mapping.

@antoninbas any thoughts on whether it would be more or less preferable to add this as a separate RPC?

In my mind, it was going to be decoupled from the ForwardingPipelineConfig: even if a new P4 pipeline was pushed, the mapping would typically remain the same as it would be specific to the switch hardware (and the P4 architecture) and not to the P4 program.

I am also not clear about how the dynamic mapping would work. Your comment in the Protobuf implies that the switch / server would be responsible for allocating "P4 values", which does not match my current understanding.

It's probably something worth discussing at the next WG meeting. I doubt I have the most comprehensive perspective on this.

@smolkaj
Copy link
Member Author

smolkaj commented Aug 3, 2023

In my mind, it was going to be decoupled from the ForwardingPipelineConfig: even if a new P4 pipeline was pushed, the mapping would typically remain the same as it would be specific to the switch hardware (and the P4 architecture) and not to the P4 program.

Makes sense, I agree.

I am also not clear about how the dynamic mapping would work. Your comment in the Protobuf implies that the switch / server would be responsible for allocating "P4 values", which does not match my current understanding.

That's right. This is meant as a convenience. Consider the following use case:

table 1 (route lookup): matches on dst IP and sets a "route ID" (some user metadata field)
table 2 (route execution): matches on "route ID" and sets the egress port/src mac/dst mac appropriately.

Maybe in our SDN controller, we want to give these routes semantically meaningful names, so we define

@p4runtime_translation("", string)
type bit<10> route_id_t; 

Now we can program routes as follows:

Table 1:

  • 10.0.0.0/8 -> "internal VM route"
  • 0.0.0.0.0/32 -> "default route"

Table 2:

  • "internal VM route" -> egress_port:1, src_mac: x, dst_mac:y
  • "default route" -> egress_port:42, src_mac: a, dst_mac:b

Internally, the switch will map the route IDs to bit<10>. It may use the mapping

"internal VM route" <-> 1
"default" <-> 2

Or, it could use

"internal VM route" <-> 2
"default" <-> 1

Or it could use any other bijection between strings and bit<10>. The interesting observation is that it doesn't matter which mapping the switch uses, since there will be no observable difference from the controller (as long as the mapping is a bijection):

  • the packet input-output behavior will be the same either way
  • INSERT/MODIFY/DELETE operations via P4Runtime will behave the same either way

Since the mapping is irrelevant, it would be very convenient if we didn't have to specify it explicitly.

@antoninbas
Copy link
Member

@smolkaj thanks for the reminder. I think we discussed this before, but I was very much focused on the special case of port translation.

When we added the p4runtime_translation annotation, I don't think we considered the "dynamic" case. I also think we were focused on the case where the annotated type is defined as part of the architecture.

I believe that both cases ("static" and "dynamic") can be supported using a separate RPC. As you pointed out, the only guarantee for the "dynamic" case is that the mapping is a bijection. The underlying values can stay the same or be re-generated when a new ForwardingPipelineConfig is sent to the switch (implementation-dependent), even for a RECONCILE_AND_COMMIT.

With a separate RPC however, there are a few open questions:

  1. what happens if there is no available mapping data (mapping was not defined, or a value is missing in a static mapping)?
  2. can the mapping data be updated dynamically after a ForwardingPipelineConfig has been sent to the switch and table entries have been inserted?

With your current proposal, things become easier (the second question does not exist), but static mappings have to be included for each ForwardingPipelineConfig. The port translation mapping has to be provided by the P4Runtime controller, which may not be an issue for you.

@smolkaj
Copy link
Member Author

smolkaj commented Aug 3, 2023

RE dynamic mappings: For better or worse, we started using them in SONIC/PINS. I am starting to wonder if that was a mistake, as explaining them to people is rather tricky, and so is reasoning about their correctness. At further thought, I think it might be a mistake to standardize them, and perhaps instead we should stop using them. Either way, perhaps it would be best to not include them for the initial proposal, we could always revisit that later.

  1. what happens if there is no available mapping data (mapping was not defined, or a value is missing in a static mapping)?

I guess we should return a FAILED_PRECONDITION error in that case?
https://grpc.github.io/grpc/core/md_doc_statuscodes.html

  1. can the mapping data be updated dynamically after a ForwardingPipelineConfig has been sent to the switch and table entries have been inserted?

This is a trickier one. In SONIC/PINS, for ports, we decided that no, this is not legal. But it is not entirely trivial to implement. More precisely, it is illegal if there is an entry using the port for which the mapping is being changed.

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

Successfully merging this pull request may close these issues.

3 participants