-
Notifications
You must be signed in to change notification settings - Fork 447
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
Implement Meter extern for eBPF backend #3231
Conversation
Co-authored-by: Tomasz Osiński <tomasz@opennetworking.org> Co-authored-by: Jan Palimąka <jan.palimaka@orange.com>
@@ -270,6 +270,21 @@ If a deparser triggers the `pack()` method, an eBPF program inserts data defined | |||
A user space application is responsible for performing periodic queries to this map to read a Digest message. It can use either | |||
`psabpf-ctl digest get pipe`, `psabpf_digest_get_next` from psabpf C API or `bpf_map_lookup_and_delete_elem` from `libbpf` API. | |||
|
|||
### Meters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also please edit ../README to make it point to this new backend?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean to add info about PSA-eBPF backend as another architecture? I think this is for another PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps, but the implementation is so featured now that it should be mentioned in the toplevel README.
backends/ebpf/psa/README.md
Outdated
Meters implement Dual Token Bucket Algorithm with both "color aware" and "color blind" modes. The PSA-eBPF implementation uses a BPF hash map | ||
to store a Meter state. The current implementation in eBPF uses BPF spinlocks to make operations on Meters atomic. The `bpf_ktime_get_ns()` helper is used to get a packet arrival timestamp. | ||
|
||
The best way to configure a Meter is to use `psabpf-ctl meter` tool. Please see an example. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as in the following example
PORT2 = 2 | ||
ALL_PORTS = [PORT0, PORT1, PORT2] | ||
|
||
meter_value_mask = 0xff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_ff_00_00_00_00_00_00_00_00_00_00_00_00_00_00_00_00_00_00_00_00_00_00_00_00 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems to be used to check expected results
testutils.send_packet(self, PORT0, pkt) | ||
testutils.verify_packet(self, pkt, PORT1) | ||
# Expecting pbs_left, cbs_left 2500 B - 100 B = 2400 B -> 09 60 | ||
self.verify_map_entry(name="ingress_meter1", key="hex 00", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you want to write some utility functions to decode these values?
could be part of a future PR.
And perhaps the converse too, so you can specify the expected value in a more readable way.
isDirect = true; | ||
} else if (typeName.startsWith("Meter")) { | ||
isDirect = false; | ||
auto ts = di->type->to<IR::Type_Specialized>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't you make a utility function for this purpose?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cstring getSpecializedTypeName(...)
does something different. It returns Type_Specialized
name. Here we take an argument from Type_Specialized
.
IR::IndexedVector<IR::StructField> EBPFMeterPSA::getValueFields() { | ||
auto vec = IR::IndexedVector<IR::StructField>(); | ||
auto bits_64 = new IR::Type_Bits(64, false); | ||
vec.push_back(new IR::StructField(IR::ID("pir_period"), bits_64)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be more maintainable if you put these fields into a list initializer with strings and used a loop to create vec. All the types seem to be bits_64.
|
||
IR::IndexedVector<IR::StructField> EBPFMeterPSA::getValueFields() { | ||
auto vec = IR::IndexedVector<IR::StructField>(); | ||
auto bits_64 = new IR::Type_Bits(64, false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's also a static Type_Bits::get() method.
ControlBodyTranslatorPSA* translator) const { | ||
auto pipeline = dynamic_cast<const EBPFPipeline *>(program); | ||
if (pipeline == nullptr) { | ||
::error(ErrorType::ERR_INVALID, "Meter used outside of pipeline %1%", method->expr); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "pipeline" an ebpf concept? It is not a P4 concept.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mbudiu-vmw We have introduced a concept of "pipeline" in the PSA/eBPF backend to distinguish between EBPF program used by ebpf_model and a new PSA arch which has two pipelines. PSA defines ingress and egress pipelines so we should have two EBPFPipeline objects representing them.
This PR adds Meter extern.
Co-authored-by: Tomasz Osiński tomasz@opennetworking.org
Co-authored-by: Jan Palimąka jan.palimaka@orange.com