search

p4lang / p4runtime

Specification documents for the P4Runtime control-plane API
Apache License 2.0
140 stars 86 forks source link

[Deprecated] Add support for different meter types. #473

Open jonathan-dilorenzo opened 4 months ago

jonathan-dilorenzo commented 4 months ago

Currently, the P4Runtime specification only supports trTCM (two rate two color meters) as per RFC 2698. However, in real hardware these sometimes use more hardware resources compared to single rate two color meters.

This PR is trying to introduce a way of restricting the meters clients can program to not use the full power of trTCM if they don't need it in order to save resources. I'm imagining the type of the meter being populated by users in the P4 program through an annotation on the meter itself (e.g. @single_rate_two_color or @two_rate_three_color).

Obviously, this is currently a WIP and I'd want to add something to the spec too, but I wanted to check the proto idea first.

jonathan-dilorenzo commented 4 months ago

Updated the specification with the related text.

jonathan-dilorenzo commented 2 months ago

Plan from April WG meeting:

  1. Require pir == cir and pburst == cburst.
  2. Add Single Rate Three Color Marker (RFC 2697) type.
  3. Add an EBS to MeterConfig to support 2
jonathan-dilorenzo commented 2 months ago

All of the AIs should now be fixed. PTAL!

jonathan-dilorenzo commented 3 weeks ago

Looks like I somehow have to rebase this on main. Will figure it out :'(

jonathan-dilorenzo commented 2 weeks ago

I don't understand why this is complaining. I thought rebasing onto main and then pushing would resolve the merge conflicts :'(. If anyone knows git better than I (a trivial task), is there something clever I can do here to resolve this locally and then push? Resolving conflict of binaries is... painful ;).

smolkaj commented 2 weeks ago

I don't know where your merge conflicts are.

But can you just check out the version from main instead of merging?

git checkout origin/main <file>
smolkaj commented 2 weeks ago

Unfortunately not really reviewable at the moment since it seems to contain changes from other PRs in the diff.

jonathan-dilorenzo commented 1 week ago

Deprecating this in favor of https://github.com/p4lang/p4runtime/pull/484, since apparently, merge conflicts are beyond my capabilities ;D