-
Notifications
You must be signed in to change notification settings - Fork 15
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
[Donut Merge] Support for off-switch buffering in SPGW pipeline with reordering prevention #153
Open
ccascone
wants to merge
17
commits into
main
Choose a base branch
from
spgwBuffering
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
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
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.
This pull request was originally opened by @robertmacdavid in #54. Carmelo re-created this to be based on the new
main
branch. This PR describes a potential solution to the packet re-ordering problem in downlink buffering. We leave it open for reference, it is not intended to be merged as-is. Some comments can be found in the old PR.Below is the original description from Robert.
This branch makes drastic changes to the SPGW pipeline with several goals in mind.
The forwarding model of distinct "PDRs" and "FARs" is removed from the pipeline, to break away from the associated restrictions and permit tofino-specific optimizations. Now there are simply UE flows, which are amalgamates of PDRs and FARs.
The entirety of the standard fabric.p4 pipeline cannot execute until the last table of the spgw pipeline has finished. The deeper the SPGW pipeline, the less resources available for all other modules. Operations have been reshuffled to reduce the pipeline depth to fewer stages. In some cases, the reshuffling is ugly. An attempt was made to document the reason behind the reshuffling in such cases.
The new pipline supports redirecting downlink UE flows to an off-switch buffering device, assumed to be dbuf. A custom GTPU extension header was added for preserving some pipeline metadata for when the packet returns from dbuf. Registers were added to avoid packet reordering.
There is also one minor change to the INT pipeline, which fixes a bug introduced by fixing an inconsistent behavior in the
SPGW pipeline. When GTPU encapsulation was performed, fabric_md.bridged.l4_sport and fabric_md.bridged.l4_dport were erroneously not updated to refer to the new outer headers (compare to fabric_md.ipv4_src and fabric_md.ipv4_dst), meaning these fields would refer to the innermost L4 ports for the downlink case, but the outermost for the uplink case. INT relied upon this (what I believe to be an) incorrect assumption. New metadata fields fabric_md.bridged.innermost_l4_sport and fabric_md.bridged.innermost_l4_dport were added as a fix.
The primary drawback of this approach is that a large amount of changes will need to be made to the UP4 app. There is currently a one-to-one and almost-stateless mapping between up4.p4 table entries and fabric.p4 entries. That will no longer be the case if this branch is merged as-is.