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

Implement DirectMeter extern for eBPF backend #3243

Merged
merged 7 commits into from
Apr 26, 2022

Conversation

kmateuszssak
Copy link
Contributor

This PR adds DirectMeter extern.

Co-authored-by: Tomasz Osiński [email protected]
Co-authored-by: Jan Palimąka [email protected]

Co-authored-by: Tomasz Osiński <[email protected]>
Co-authored-by: Jan Palimąka <[email protected]>
@kmateuszssak
Copy link
Contributor Author

@mbudiu-vmw I will improve readability of meters PTF tests later. For now, pleas review as it is.

@@ -83,6 +82,16 @@ class EBPFTablePSA : public EBPFTable {
return nullptr;
}

EBPFMeterPSA* getMeter(cstring name) const {
auto result = std::find_if(meters.begin(), meters.end(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this suggests that you may want a std::map instead of a std::vector.

@@ -21,6 +21,7 @@
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
two_meters_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_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_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_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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is just meter_value_mask concatenated to itself that's perhaps what you should write.

# period 1ms -> 1250 B per period, 1ms -> 1e6 ns -> 0F 42 40, 1250 -> 04 E2
self.update_map(name="ingress_tbl_fwd", key="hex 04 00 00 00",
value="hex "
"01 00 00 00 05 00 00 00 " # action id | egress port
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made a comment in the previous PR about utility functions to build these values, it applies here.

class DirectTwoMetersPSATest(P4EbpfTest):
"""
Test two Direct Meters in one table. Type BYTES.
Send 100 B packet and verify if there is 100 tokens less left.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that the number of tokes decreases by 100

class DirectAndCounterMeterPSATest(P4EbpfTest):
"""
Test Direct Meter with Direct Counter.
Send 100 B packet and verify if there is 100 tokens less left.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

#include <core.p4>
#include <psa.p4>

typedef bit<48> EthernetAddress;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought you were using some common definitions of the headers using an #include.
Forgot to check this in a few previous PRs, perhaps you want to scan the recent test additions.

apply { }
}

control CommonDeparserImpl(packet_out packet,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am even wondering whether you couldn't also put all these controls into the common header file... would make the code much easier to read.

@mihaibudiu
Copy link
Contributor

Please rerequest a review when ready.

@kmateuszssak
Copy link
Contributor Author

@mbudiu-vmw please review.

# Expecting pbs_left, cbs_left 2500 B - 100 B = 2400 B -> 09 60
meter_value = build_meter_value(pir=250000, cir=250000, pbs=2500,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

much better, isn't it?

@mihaibudiu mihaibudiu merged commit 5b9e595 into p4lang:main Apr 26, 2022
osinstom pushed a commit to osinstom/p4c-1 that referenced this pull request Apr 27, 2022
* Implement DirectMeter extern for eBPF backend

Co-authored-by: Tomasz Osiński <[email protected]>
Co-authored-by: Jan Palimąka <[email protected]>
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.

2 participants