Egress QoS support #5425
Merged
Egress QoS support #5425
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
GraysonWu
force-pushed
the
egress-qos
branch
12 times, most recently
from
36eb7ea to
e640a44
Compare
GraysonWu
force-pushed
the
egress-qos
branch
3 times, most recently
from
3495e11 to
638c0f4
Compare
GraysonWu
requested review from
Dyanngg and
wenyingd
and removed request for
wenyingd
Dyanngg
reviewed
Sep 14, 2023
|
@GraysonWu please help update the PR summary if this PR can resolve the issue #2766 |
|
Hi @GraysonWu please help to resolve the code conflicts and check if possible to improve the patch unit test coverage. Thanks. |
GraysonWu
force-pushed
the
egress-qos
branch
5 times, most recently
from
6f9c3c1 to
169b417
Compare
antoninbas
reviewed
Oct 20, 2023
tnqn
reviewed
Oct 23, 2023
GraysonWu
force-pushed
the
egress-qos
branch
2 times, most recently
from
24676c4 to
a2c64d5
Compare
tnqn
reviewed
Oct 23, 2023
antoninbas
reviewed
Oct 23, 2023
Comment on lines
+1104
to
+1105
| // Openflow bundle message doesn't support meter. Add meter individually instead of | ||
| // calling AddOFEntries function. |
There was a problem hiding this comment.
maybe you know too @GraysonWu ?
GraysonWu
force-pushed
the
egress-qos
branch
2 times, most recently
from
bca28da to
f8460b2
Compare
|
/test-all |
|
/test-conformance |
|
/test-e2e |
|
/test-e2e |
tnqn
previously approved these changes
Oct 24, 2023
|
@antoninbas feel free to merge once it looks good to you. |
jianjuns
reviewed
Oct 24, 2023
|
/test-all |
tnqn
previously approved these changes
Oct 25, 2023
|
@antoninbas @jianjuns it seems all of your comments have been addressed, could you confirm if the PR looks good to you? |
Add `bandwidth` to Egress specifying the rate limit of north-south egress traffic of this Egress. All backend workloads selected by a rate-limited Egress share the same bandwidth while sending egress traffic via this Egress. Signed-off-by: graysonwu <[email protected]>
Signed-off-by: graysonwu <[email protected]>
|
Fixed merge conflicts. |
|
/test-all |
tnqn
approved these changes
Oct 26, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #2766
Add rate-limit config to Egress specifying the rate limit of
north-south egress traffic of this Egress. All backend workloads
selected by a rate-limited Egress share the same bandwidth while
sending egress traffic via this Egress. An Egress with
rateLimitspecified cannot share EgressIP with any other Egresses.