-
Notifications
You must be signed in to change notification settings - Fork 93
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
[NAT HLD] Nat hld #348
[NAT HLD] Nat hld #348
Conversation
Signed-off-by: 桂棹 <[email protected]>
Signed-off-by: Clark Lee <[email protected]>
We strongly prefer using editable .svg for diagram files instead of bitmaps (.png) because the source may be edited. Is it possible your diagrams can be provided that way? A great drawing tool is https://www.diagrams.net/ |
|
||
* **first packet** processing flow of inbound packet | ||
|
||
![dnat-first-packet-processing-inbound-packet-flow](images/Figure9-dnat-first-packet-processing-inbound-packet-flow.png) |
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.
need clarification on how to match dst ip to vnet
`Destination: IP 8.8.8.8, port 53, protocol UDP` | ||
|
||
* **first packet** processing flow of outbound packet | ||
|
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.
What is the mechanism to pick a SIP (111.1.1.237 in this diagram) from a list of SIPs specified in the SNAT_RULE_TABLE?
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.
Prince asked this question in the meeting - Where did the translated UDP source port value of 45678 come from?
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.
For the populating of inbound flow entry, can dst_vtep and inner_mac_address be learned from the data packets outer_sip and inner_smac respectively?
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.
What is the mechanism to pick a SIP (111.1.1.237 in this diagram) from a list of SIPs specified in the SNAT_RULE_TABLE?
The lookup takes 2 steps: 1) Use SIP + VNID to lookup into SNAT_RULE_TABLE( the SIP lookup should be LPM lookup); 2) Use DIP(also LPM) to lookup for a list of available SIP. After we get the list of SIP, use a load balancing algorithm to select SIP from the list.
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.
Prince asked this question in the meeting - Where did the translated UDP source port value of 45678 come from?
Source port is allocated by the slowpath logic, we don't show it in the pipe for simplicity.
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.
For the populating of inbound flow entry, can dst_vtep and inner_mac_address be learned from the data packets outer_sip and inner_smac respectively?
In my understanding, for DST_VTEP lookup, we allow to configure a list of VTEP instead of learn it from the packet, for load balance purpose.
For Inner Dst MAC, learn from the packet is fine.
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.
For the populating of inbound flow entry, can dst_vtep and inner_mac_address be learned from the data packets outer_sip and inner_smac respectively?
In my understanding, for DST_VTEP lookup, we allow to configure a list of VTEP instead of learn it from the packet, for load balance purpose.
For Inner Dst MAC, learn from the packet is fine.
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.
Prince asked this question in the meeting - Where did the translated UDP source port value of 45678 come from?
Source port is allocated by the slowpath logic, we don't show it in the pipe for simplicity.
Are you planning to implement "slow path" in the P4 behavioral pipeline? If yes, we will need mechanisms for load-balancing to select a SIP and also SPORT in the P4 program.
…ic view of NAT inbound/outbound processing pipeline. Signed-off-by: Clark Lee <[email protected]>
Signed-off-by: Clark Lee <[email protected]>
Signed-off-by: Clark Lee <[email protected]>
* Routing service for underlay forwarding. | ||
* Billing service(by traffic volumn and by commited traffic rate). | ||
|
||
The goal is to test the following properties: |
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 seems a little bit out of place, describing an experiment. If this document is an HLD, it should simply be a description and specification of something and not talk about what-if scenarios you wish to test.
* **first packet** processing flow of inbound packet | ||
|
||
![dnat-first-packet-processing-inbound-packet-flow](images/Figure9-dnat-first-packet-processing-inbound-packet-flow.png) | ||
*Figure 9 - DNAT first packet processing inbound packet flow* |
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.
DNAT inbound table i think SIP and SPORT may be redundant since our Key is DIP and DPORT, this way we can optimize the Table size.
Reviewed this week 3/15/2023. Look to unify flow table, create as ENI-based in order to be standardized. Cover updates at another later time when ready. |
|
||
![snat-inbound-packet-processing-pipeline](images/Figure3-snat-inbound-pipeline.png) | ||
*Figure 3 - SNAT inbound packet processing pipeline* | ||
|
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.
Shouldn't this figure be titled DNAT processing (instead of SNAT)?
|
||
* Both an outbound flow table entry and an inbound flow table entry are added to their respective flow tables (H/W tables). | ||
|
||
* A search of the outbound flow table is triggered once the packet is re-inserted. The packet will match the outbound flow table entry created (with previous **slow path** processing), and the flow action will be applied to the packet, including the replacement of the source IP address/port number and the decapsulation of the VXLAN header. |
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.
Why re-insert this packet? Why not continue processing this packet in forward direction? Subsequent packets hitting the modified outbound flow table will go through the fast path.
|
||
* Both an outbound flow table entry and an inbound flow table entry are added to corresponding flow tables. | ||
|
||
* A search of the inbound flow table is triggered once the packet is re-injected. The packet will match the inbound flow table entry created previously and the flow action will be applied, which includes replacing the destination IP address/port number and adding VXLAN encapsulation. |
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.
Why re-inject this packet? Wouldn't this eat away extra pipeline bandwidth? Why not continue processing this packet in forward direction? Subsequent packets hitting the modified inbound flow table will go through the fast path.
### 2.2 Outbound packet processing pipeline | ||
|
||
![dnat-outbound-packet-processing-pipeline](images/Figure5-dnat-outbound-pipeline.png) | ||
*Figure 5 - DNAT outbound packet processing pipeline* |
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.
Shouldn't this figure be titled SNAT processing (instead of DNAT)?
* **first packet** processing flow of outbound packet | ||
|
||
![snat-first-packet-processing-outbnand-packet-flow](images/Figure6-snat-first-packet-processing-outbound-packet-flow.png) | ||
*Figure 6 - SNAT first packet processing outbound packet flow* |
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.
Where does it get the sport=45678 to rewrite in the packet? This sport is not part of the SNAT_RULE_TABLE so not sure where it comes from?
Hi @clarklee-guizhao - did we want to keep going on this PR? |
Yes, we're now working on the P4 code for this HLD, I think we may update this HLD when P4 code PR is added to Behavioral Mode. Estimated in late July. |
Hi @clarklee-guizhao - just checking in on this PR, would we want to present it at an upcoming call? |
Hi KrisNey, I think we need to change a bit and update in 3 weeks, as I have a conversation with @r12f on #449 yesterday, the changes can make NAT processing flow fit in current community design. Should be ready on Dec 1st weekly meeting. |
Oh great, thank you! Kristina
From: Clark Lee ***@***.***>
Sent: Wednesday, November 8, 2023 7:11 PM
To: sonic-net/DASH ***@***.***>
Cc: Kristina Moore ***@***.***>; Mention ***@***.***>
Subject: Re: [sonic-net/DASH] [NAT HLD] Nat hld (PR #348)
Hi @clarklee-guizhao<https://github.com/clarklee-guizhao> - did we want to keep going on this PR? Thank you, @KrisNey-MSFT<https://github.com/KrisNey-MSFT>
Yes, we're now working on the P4 code for this HLD, I think we may update this HLD when P4 code PR is added to Behavioral Mode. Estimated in late July.
Hi @clarklee-guizhao<https://github.com/clarklee-guizhao> - just checking in on this PR, would we want to present it at an upcoming call? TY, @KrisNey-MSFT<https://github.com/KrisNey-MSFT>
Hi KrisNey, I think we need to change a bit and update in 3 weeks, as I have a conversation with @r12f<https://github.com/r12f> on #449<#449> yesterday, the changes can make NAT processing flow fit in current community design. Should be ready on Dec 1st weekly meeting.
-
Reply to this email directly, view it on GitHub<#348 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AFJSI6F6J2MWLMSVSX467MLYDQ3R7AVCNFSM6AAAAAAVLUYAPKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBTGA2TGMZZHA>.
You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>
|
Hi @clarklee-guizhao - checking back to see if you wanted to review/show this at the Community call meeting next week on Dec 6th? Or do you feel @r12f covered it for you in the call yesterday? Let me know...thanks! |
* main: (75 commits) [dash-SAI] Enable warnings as errors (sonic-net#466) [SAI] wrong code generated in libsai sonic-net#415 (sonic-net#463) Fix incorrect IP in SONiC-DASH HLD VNET to VNET example. (sonic-net#459) DASH pipeline packet flow update proposal. (sonic-net#449) [libsai] Add attr name logging when doing get api (sonic-net#451) Build libsai deb packages in github workflow (sonic-net#450) Add Private Link mapping (sonic-net#327) [SAI] Update SAI submodule to the latest origin/master (sonic-net#446) [dash] Add libsai-debs target to create libsai debian packages (sonic-net#444) update p4 compile dependancy to avoid parallel docker runs (sonic-net#443) [dash] Refactor libsai (sonic-net#438) [dash] Update SAI to latest v1.13 (sonic-net#435) [dash-pipeline] Refactor Makefiles (sonic-net#432) Remove ACL tags from BM (sonic-net#425) [submodule] Update SAI submodule to origin/master (sonic-net#431) [sai-api-gen] Write files only when changes are detected (sonic-net#429) Adds SAI_ROUTE_ENTRY_ATTR_PACKET_ACTION api to dash_underlay_routing (sonic-net#422) [SAI] Add missing check for api initialized [SAI] Print oids in hex form [SAI] Change asserts to return error codes and add missing switch api ...
Hi @KrisNey-MSFT Thanks for the reminder. I've updated NAT HLD, may get back after some feedback, I think Dec 13 may be a better time frame incase I need some modification after several feedback. @r12f Please help review the updated NAT HLD, to check if it is compliant to get a generic enough pipeline, thanks a lot. |
@clarklee-guizhao and @r12f - I'll put you on deck for Dec 13th :) |
Per feedback, PR464 should cover what the NAT HLD intended at 90%. Alibaba can also leverage the dash-sai-pipeline-packe-flow.md framework (after talking w/Riff). As such, closing this PR. |
Add NAT scenario HLD