Add generated rust code

[duskmoon314]

The rust code is generated using the neoeinstein/protoc-gen-prost, which leverages the prost crate for protobuf and tonic crate for client/server.

[duskmoon314]

I have also created a client wrapper based on this generated code. A basic example of inserting table entries and receiving digests with the wrapper is also available.

See duskmoon314/p4runtime-client-rs

---

Please let me know if there is anything I need to do to push this further.

[chrispsommers]

Hi @duskmoon314 Sorry for the delay. I've run your workflow and it passes. Thank you for your contribution. Given that rust is a new client binding for P4RT perhaps it would be best if you joined the next WG meeting (June 14th 2024) and presented this to the group so we can ask questions. Also, I'm not sure who else in the WG has enough Rust experience to properly review the submission. If anyone wants to volunteer as a reviewer it'd be appreciated!

[duskmoon314]

Thanks for your feedback and invitation. Unfortunately, the meeting time is midnight for me (UTC +8). I'm afraid I cannot attend the meeting.

I think I can prepare some documents on how the scripts work and what code is generated. Thus it would be convenient for the WG to know whether this is appropriate.

I will also announce this PR in the Rust community later and hope to find someone familiar with gRPC who is interested in reviewing it.

[duskmoon314]

I have created a gist of my notes on this PR: https://gist.github.com/duskmoon314/59ca06dd5cac6ed00a45a112d1ef5ba6

I may forget to include some details. Please point out if I need to add them to the notes.

[chrispsommers]

Hi @duskmoon314 thanks for the gist above, it is very detailed. We discussed your PR in today's WG meeting. We came to some conclusions:

1. A Rust client seems like a welcome addition to P4RT
2. We'll try to find another Rust practitioner (one who understands P4Runtime as well) to review the PR.
3. There is a question as to whether it is necessary to check in the generated Rust code at all. Scripts etc. used to generate the code, and a CI action to verify the code generates w/o errors, are clearly valuable. For reasons we couldn't recall in the meeting today, we check in the golang client code as a special case. It does create a bit more maintenance work. (Note, we don't check in generated Python code, for example.) I'd ask @antoninbas or another "P4RT historian" to refresh our memory as to why we made this exception. Does the same reasoning apply to generated Rust code?

[antoninbas]

The generated Golang code should be checked in, as this is the common practice and is based on how Golang dependencies are imported / consumed. When a downstream project wants to build a Golang application that includes a P4Runtime client, they only need to import github.com/p4lang/p4runtime as a dependency, and they get access to the generated client code. Golang does not rely on publishing binary artifacts, the code is imported directly from source version control remotes and built as part of the project which depends on it. For example, Kubernetes checks-in all the generated client code (e.g., https://github.com/kubernetes/kubernetes/blob/master/staging/src/k8s.io/api/node/v1/generated.pb.go). There is no central registry for Golang where modules are published. And asking all projects to generate their own client code is not typically done.

Python works differently, but it should be noted that we still check-in the code: https://github.com/p4lang/p4runtime/tree/main/py/p4. IMO, it is not strictly required however, and the code is not consumed directly from version control. Instead, we could generate the code on the fly before uploading to PyPI.

For Rust, it would seem that checking-in the generated code (like we do for Golang) would actually be the best approach. While we could just generate a crate on the fly and publish it to crates.io, it seems quite convenient to consume them directly from Github (https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#specifying-dependencies-from-git-repositories). IMO, that would be less of a burden that dealing with crates.io (the 2 are not mutually exclusive).

[duskmoon314]

Thanks for your feedback!

There is a question as to whether it is necessary to check in the generated Rust code at all.

I mimicked the go approach in creating this PR. I think checking in the generated code has two pros:

1. People can view the generated code. (Though this may not be very useful.)
2. Rust can use a crate (library, package) directly from GitHub (or other remote repo). Thus, users can test an unreleased main branch easily.

It does create a bit more maintenance work.

This, indeed, is the con of checking in the code. I hope to hear more details about the maintenance work of the generated Go code and see if we can make similar work easier in Rust.

[chrispsommers]

@antoninbas Thanks so much for the very detailed reply (and correcting my misconception about Python generated code); it was most helpful!

@duskmoon314 Based on Antonin's response and your own follow-up, I'm now firmly in favor of you checking in the generated Rust code for all the reasons cited. As far as "maintenance," it's not much, see this from the README. It is not uncommon for PR submitters to forget to do this step and the CI build fails. Other than that, the maintenance is minor.

When updating the Protobuf files in a pull request, you will also need to update the generated Go and Python files, which are hosted in this repository under go/ and py/. This can be done easily by running ./codegen/update.sh, provided docker is installed and your user is part of the "docker" group (which means that the docker command can be executed without sudo).

As part of your PR, can you update the README to include Rust generated code in the text, and also update the contents of codegen/ to regenerate the Rust code as well?

The only other matter is asking someone with some Rust knowledge to review this so we have some technical assurance. I've no doubts, but I'm not qualified.

[duskmoon314]

and also update the contents of codegen/ to regenerate the Rust code as well?

I'm sorry, but I don't quite understand what "regenerate" means here. Should I regenerate the Rust code on my machine, or do some scripts in codegen need to be modified?

[chrispsommers]

I went back and realized you'd already modified the scripts to generate the code, disregard my comment. Sorry for the confusion! It looks good to me, we just need someone to review the Rust aspects. Thanks!

[chrispsommers]

Hi @duskmoon314 I see some commits, are you still working on resolving the conversation comments?

[duskmoon314]

are you still working on resolving the conversation comments?

I just upgraded deps to the latest version. I haven't received feedback on whether to remove the serde support in this PR or how to use tonic-build with serde support, so I left the generated part as it is.

I will try to free up some time to find a better way to support serde. But I also think removing it is OK for this PR.

[chrispsommers]

@duskmoon314 where are we at here - what's your recommendation? We're aiming to freeze by Sept 13 for release 1.4.0. Thanks!

[duskmoon314]

Sorry for responding late.

where are we at here

I think here are two todos:

1. Whether to remove prost-serde_out (supporting of serde)
2. Maybe find more Rust experts (especially prost and tonic experts) to review.

For the first one, based on my current experience using the generated code, I think removing supporting of serde is reasonable:

1. Users can use prost::Message::decode to convert binary to rust structs, so protobuf JSON mapping is unnecessary.
2. The generated code of serde is too much and hard to review and maintain.

Thus, I will remove this in a new commit.

[duskmoon314]

Should I rebase and sign all commits? It seems this causes failure from DCO.

It's weird that I uploaded the newly generated code, but CI says there is a difference. 😢

[chrispsommers]

Yeah, the signoff requirement was recently adopted since we're under LFX. There are techniques to fix it in arrears, Andy just posted something about this yesterday. I can't guess at the reason for the generated code diff.

[duskmoon314]

Since a signature and a force push are needed, I rebased and squashed all commits.

But I forgot to update GitHub settings, so DCO still fails to validate the signature. 😢

The CI still says the generated Rust is not up to date; I will continue to find the reason.

---- Update ----

Oh, I misunderstood what DCO was saying; I thought I needed to sign the commit with the gpg key" I will add the "sing-of" part later.

[duskmoon314]

I found out the reason: The main branch has a newer version of the proto, and I need to update the branch.

All the previous commits are now squashed into one, based on the latest commit in the main branch. A signed-off line is also added.

[smolkaj]

Thanks, this looks great!

[chrispsommers]

@duskmoon314 Hi, this looks close. Can you please resolve all conversations? Thanks.

[duskmoon314]

Can you please resolve all conversations?

My mistake. I forgot to set that conversation as resolved. (It is resolved since serde is not supported in this PR.)

Please let me know if anything else I need to do.

[duskmoon314]

Thanks!

I noticed the CI has yet to be passed in the main branch. After a simple discovery, I found out that besides the main branch update, I forgot to pin the version of protoc-gen-prost whose new version caused the mismatching of generated code.

I will submit a new PR to fix this problem ASAP.