search

sonic-net / DASH

Disaggregated APIs for SONiC Hosts
Apache License 2.0
77 stars 88 forks source link

dash-sonic-hld and dash_pipeline.p4 Inbound Routing question #518

Open KrisNey-MSFT opened 4 months ago

KrisNey-MSFT commented 4 months ago

Question from @ashutosh-agrawal - adding @prsunny and @sharmasushant and @r12f

In section 2.2 of dash-sonic-hld, @ashutosh-agrawal could you clarify your question re: Inbound Routing (VNI vs ENI?) and Priority - and line 229 table in the dash_pipeline.p4 @SAITable.

2.2 of dash-sonic-hld

Inbound Routing 2_2 Priority

Inbound Routing

ashutosh-agrawal commented 4 months ago

My questions: 1) Why is the eni_id used in the match key for inbound_routing table? From the description in section 2.2 of SONiC HLD, just the vni and SRC PA prefix should be enough. 2) HLD states that two VNETs in different region can have the same VNI key - Is that assumption still valid? 3) Assuming that VNI value is shared and we do need to qualify VNI with SRC PA Prefix to derive the VNET ID. What is the purpose of "Priority" in the inbound routing table? Is the priority only used as a proxy for prefix length here? If yes, can we safely perform a LPM match on all the entries for a given VNI and ignore the priority field? 4) The 2nd paragraph in section 2.2 states that CA-PA mapping table should be used for both encap and decap process. I don't see CA-PA table getting used in the inbound flow in the behavioral model. The outer destination IP for inbound packet from the DASH appliance to VM is derived based on ENI. Can you please clarify how the mapping table is used in the inbound flow?

KrisNey-MSFT commented 4 months ago

@prsunny @sharmasushant @r12f - would we be able to have this sorted by the Community call tomorrow? TY, Kristina

KrisNey-MSFT commented 4 months ago

PA check SDN Features original doc

Routing is used for validating; must have a prefix (not used, not populated in API call)

Maybe create 'PA Validation Object' stage with fields relevant for PA validation? Will close offline.

ashutosh-agrawal commented 4 months ago

In the last community call, we agreed that outer src IP prefix is not required in the inbound routing lookup. But this document listed in the previous comment has both source and destination IP in the lookup key. Perhaps, we should cleanup both the document and P4 code and remove src/dst IP addresses.

https://github.com/sonic-net/DASH/blob/main/documentation/general/sdn-features-packet-transforms.md#inbound-priority-route-rules-processing:~:text=will%20be%20matched.-,Inbound%20(priority)%20route%20rules%20processing,-All%20inbound%20rules

KrisNey-MSFT commented 4 months ago

In the last community call, we agreed that outer src IP prefix is not required in the inbound routing lookup. But this document listed in the previous comment has both source and destination IP in the lookup key. Perhaps, we should cleanup both the document and P4 code and remove src/dst IP addresses.

https://github.com/sonic-net/DASH/blob/main/documentation/general/sdn-features-packet-transforms.md#inbound-priority-route-rules-processing:~:text=will%20be%20matched.-,Inbound%20(priority)%20route%20rules%20processing,-All%20inbound%20rules

@r12f or @prsunny - would you mind editing and I'll merge?

KrisNey-MSFT commented 3 months ago

gentle ping @r12f

KrisNey-MSFT commented 3 weeks ago

Created PR 591 to edit doc per Issue 518 PR591 Remove Outer SRC IP Prefix (not req's for Inbound routing lookup)