Add Dash flow design #462

zhixiongniu · 2023-11-13T07:23:51Z

No description provided.

KrisNey-MSFT · 2023-12-11T21:56:20Z

hi @zhixiongniu - do we need to get the spellcheck to pass?

zhixiongniu · 2023-12-16T01:00:06Z

hi @zhixiongniu - do we need to get the spellcheck to pass?

Hi I will update this PR soon.

documentation/dataplane/dash-flow/DASH_FLOW_SAI.md

chrispsommers

See minor spelling errors

ashutosh-agrawal · 2024-01-26T18:50:46Z

documentation/dataplane/dash-flow/DASH_FLOW_SAI.md

+
+    /* @brief Destination port */
+    sai_uint16_t dst_port;
+} sai_dash_ha_flow_tcp_udp_info_t;


Any particular reason to include "ha" in the name of this structure?

ashutosh-agrawal · 2024-01-26T19:19:49Z

documentation/dataplane/dash-flow/DASH_FLOW_SAI.md

+#### Remove flow table
+
+```c
+/* Finally, remove the flow table */


Do you expect this API to actively remove all the entries from the HW before returning the status? The alternative is to do perform a lazy delete where the ENI will be marked with a drop status and we can let HW age-out the entries over time.

@zhixiongniu , this is a very point. I believe an attribute to control the flow expiry will be awesome to add.

documentation/dataplane/dash-flow/DASH_FLOW_SAI.md

r12f · 2024-02-18T18:14:05Z

just found my comments from months back are not published..... orz.

documentation/dataplane/dash-flow/DASH_FLOW_SAI.md

r12f · 2024-02-18T18:40:07Z

documentation/dataplane/dash-flow/DASH_FLOW_SAI.md

+#### Remove flow table
+
+```c
+/* Finally, remove the flow table */


@zhixiongniu , this is a very point. I believe an attribute to control the flow expiry will be awesome to add.

r12f · 2024-02-18T18:41:19Z

since the sides are actually covered in the HA design, so we can either update that slides if anything new, or remove it and reference the HA doc. @zhixiongniu

KrisNey-MSFT · 2024-02-20T19:55:04Z

@r12f Looks like this needs a spellcheck correction before merge

zhixiongniu · 2024-02-21T03:44:14Z

thanks @r12f and @KrisNey-MSFT @ashutosh-agrawal @chrispsommers, I will provided a updated version when the P4 generated version API is ready.

Signed-off-by: Zhixiong Niu <[email protected]>

documentation/dataplane/dash-flow/DASH_FLOW_API_HLD.md

documentation/dataplane/dash-flow-api.md

Signed-off-by: Zhixiong Niu <[email protected]>

ashutosh-agrawal · 2024-05-01T18:57:18Z

documentation/dataplane/dash-flow-api.md

+  IpAddress src_ip = 2;  // Source IP address
+  IpAddress dst_ip = 3;  // Destination IP address
+  uint32 src_port = 4;  // Source port
+  uint32 dst_port = 5;  // Destination port


Don't we need IP protocol here as well?

Yes, we should have IP protocol.

documentation/dataplane/dash-flow-api.md

Signed-off-by: Zhixiong Niu <[email protected]>

ashutosh-agrawal · 2024-05-08T00:32:53Z

documentation/dataplane/dash-flow-api.md

+  uint32 meter_class = 3;  // SAI_FLOW_ENTRY_ATTR_METER_CLASS
+  bool is_unidirectional_flow = 4;  // SAI_FLOW_ENTRY_ATTR_IS_UNIDIRECTIONAL_FLOW
+  uint32 underlay_vni = 5;  // SAI_FLOW_ENTRY_ATTR_UNDERLAY_VNI
+  IpAddress underlay_sip = 6;  // Underlay source IP address


Should we use underlay0 and underlay1 to be consistent with the SAI_FLOW_ENTRY_ATTRs listed earlier in this document?

Sure, I have changed these.

ashutosh-agrawal · 2024-05-08T00:33:16Z

documentation/dataplane/dash-flow-api.md

+  uint32 underlay_vni = 5;  // SAI_FLOW_ENTRY_ATTR_UNDERLAY_VNI
+  IpAddress underlay_sip = 6;  // Underlay source IP address
+  IpAddress underlay_dip = 7;  // Underlay destination IP address
+  MacAddress underlay_smac = 8;  // Underlay source MAC address


Remove underlay_smac and dmac as discussed earlier.

Thanks. I have changed these.

ashutosh-agrawal · 2024-05-08T00:35:45Z

documentation/dataplane/dash-flow-api.md

+uint32_t attr_count = 3; 
+sai_attribute_t attr_list[3];
+attr_list[0].id = SAI_FLOW_TABLE_ATTR_DASH_FLOW_ENABLED_KEY;
+attr_list[0].value = SAI_DASH_FLOW_ENABLED_KEY_PROTOCOL | 


Is eni_addr skipped here intentionally?

No, I added SAI_DASH_FLOW_ENABLED_KEY_ENI_ADDR in the value.
Thanks!

Signed-off-by: Zhixiong Niu <[email protected]>

zhixiongniu · 2024-05-11T13:02:11Z

See minor spelling errors

Thanks, revised all typos.

zhixiongniu · 2024-05-11T13:03:16Z

See minor spelling errors

Thanks. I have solved all types.

clarklee-guizhao · 2024-05-13T03:36:41Z

documentation/dataplane/dash-flow-api.md

+    SAI_DASH_FLOW_ENABLED_KEY_SRC_PORT = 1 << 5,
+
+    SAI_DASH_FLOW_ENABLED_KEY_DST_PORT = 1 << 6,
+


In NAT scenario, SAI_DASH_FLOW_ENABLED_KEY_VNI is needed, would you please help add this field support? Thanks.

Thanks, added.

clarklee-guizhao · 2024-05-13T03:38:02Z

documentation/dataplane/dash-flow-api.md

+     * @brief Exact matched key dst_port
+     */
+    sai_uint16_t dst_port;
+


Would you please add sai_uint32_t vni; so that we can support VNI as a key in flow_entry? Thanks.

Thanks, added.

clarklee-guizhao · 2024-05-13T03:39:37Z

documentation/dataplane/dash-flow-api.md

+    SAI_DASH_FLOW_ENTRY_BULK_GET_SESSION_FILTER_KEY_FLOW_VERSION,
+
+    SAI_DASH_FLOW_ENTRY_BULK_GET_SESSION_FILTER_KEY_AGED,
+


Would you please add SAI_DASH_FLOW_ENTRY_BULK_GET_SESSION_FILTER_KEY_VNI so that we can support VNI as a key in filter? Thanks.

Thanks, added.

clarklee-guizhao · 2024-05-13T03:52:28Z

documentation/dataplane/dash-flow-api.md

+When a service intends to use DASH Flow SAI APIs, it should first establish a flow table via the `create_flow_table()` function. After the table creation, the programmer can add, delete, modify, or retrieve flow entries to/from the table. For instance, when DASH HA needs to perform bulk sync from the active DPU to the standby DPU, it should initially fetch the entry from the active DPU using `get_flow_entries`, then transmit the flow entries to the standby DPU via the control plane. The standby DPU subsequently calls `create_flow_entries()`to add entries to the corresponding flow table.
+
+These examples describe how to create a flow state table, and how to operate flow entries.
+


Would a simple programing flow chart helps before example code? For example, take HA as a use case.

Hi Clark,

Thanks for your comments! I have added a figure for this and also updated the words.

Signed-off-by: Zhixiong Niu <[email protected]>

r12f · 2024-05-08T20:46:18Z

documentation/dataplane/dash-flow-api.md

+```c
+sai_flow_entry_t flow_entry;
+flow_entry.flow_table_id = 0x112233;
+flow_entry.ip_proto = 6;


eni id seems to be missing here.

ali-partner

LGTM

marian-pritsak · 2024-06-03T18:13:22Z

documentation/dataplane/dash-flow-api.md

+The sai_dash_flow_enabled_key_t is defined as follows:
+
+```c
+typedef enum _sai_dash_flow_enabled_key_t


The word "enabled" is redundant.

this is actually intentional, so it will differ from the flow key, which usually refers to the struct that holds all the fields. using "enabled key" makes it much more clear to be a flag that indicates which key field should be enabled during flow matching.

marian-pritsak · 2024-06-03T18:16:43Z

documentation/dataplane/dash-flow-api.md

+| Attribute name                      | Type            | Description                                                  |
+| ----------------------------------- | --------------- | ------------------------------------------------------------ |
+| SAI_FLOW_ENTRY_ATTR_VENDOR_METADATA | `sai_u8_list_t` | Vendor-specific metadata that can be attached to the flow entry for custom processing. |
+| SAI_FLOW_ENTRY_ATTR_FLOW_DATA_PB    | `sai_u8_list_t` | The flow data protocol buffer enables high-efficiency creation, retrieval, and communication for a flow entry. |


I don't think it's a good idea to tie the transport to the SAI struct. Vendor should provide a raw data that can be consumed as is by the other side.

i think this attribute "VENDOR_METADAT" didn't break the assumption of "Vendor should provide a raw data that can be consumed as is by the other side.", as this is basically a raw buffer, and we will pass it as it is without the internal knowledge. Do you mind to elaborate this concern a bit more?

Signed-off-by: Zhixiong Niu <[email protected]>

zhixiongniu added 3 commits November 13, 2023 15:22

Add dash flow design doc

fb3378b

Add vni in the flow key and metering class 2

3864ff5

Move doc to dataplane

6d05bc4

KrisNey-MSFT assigned zhixiongniu Dec 11, 2023

zhixiongniu added 3 commits December 18, 2023 13:02

Merge branch 'sonic-net:main' into main

e686d73

Merge branch 'sonic-net:main' into main

b1ff342

Merge branch 'sonic-net:main' into main

6abe821

chrispsommers reviewed Jan 24, 2024

View reviewed changes

documentation/dataplane/dash-flow/DASH_FLOW_SAI.md Outdated Show resolved Hide resolved

chrispsommers reviewed Jan 24, 2024

View reviewed changes

documentation/dataplane/dash-flow/DASH_FLOW_SAI.md Outdated Show resolved Hide resolved

chrispsommers requested changes Jan 24, 2024

View reviewed changes

ashutosh-agrawal reviewed Jan 26, 2024

View reviewed changes

r12f reviewed Feb 18, 2024

View reviewed changes

documentation/dataplane/dash-flow/DASH_FLOW_SAI.md Outdated Show resolved Hide resolved

documentation/dataplane/dash-flow/DASH_FLOW_SAI.md Outdated Show resolved Hide resolved

r12f reviewed Feb 18, 2024

View reviewed changes

zhixiongniu and others added 7 commits February 22, 2024 14:11

Merge branch 'sonic-net:main' into main

4053750

Update the doc

8eede46

Update the doc

54747fd

Update to lastest doc

9a58b18

Signed-off-by: Zhixiong Niu <[email protected]>

Merge branch 'sonic-net:main' into main

26206c6

Update with latest P4-gen API

7553ec8

Signed-off-by: Zhixiong Niu <[email protected]>

Updated

cf6504a

Signed-off-by: Zhixiong Niu <[email protected]>

r12f reviewed Mar 12, 2024

View reviewed changes

documentation/dataplane/dash-flow/DASH_FLOW_API_HLD.md Outdated Show resolved Hide resolved

documentation/dataplane/dash-flow/DASH_FLOW_API_HLD.md Outdated Show resolved Hide resolved

documentation/dataplane/dash-flow/DASH_FLOW_API_HLD.md Outdated Show resolved Hide resolved

r12f reviewed Mar 12, 2024

View reviewed changes

documentation/dataplane/dash-flow/DASH_FLOW_API_HLD.md Outdated Show resolved Hide resolved

documentation/dataplane/dash-flow/DASH_FLOW_API_HLD.md Outdated Show resolved Hide resolved

Merge branch 'sonic-net:main' into main

2e1a4a9

ashutosh-agrawal reviewed Apr 24, 2024

View reviewed changes

documentation/dataplane/dash-flow-api.md Outdated Show resolved Hide resolved

r12f requested a review from marian-pritsak April 24, 2024 22:49

Add slides

e7e5e9e

Signed-off-by: Zhixiong Niu <[email protected]>

ashutosh-agrawal reviewed May 1, 2024

View reviewed changes

documentation/dataplane/dash-flow-api.md Show resolved Hide resolved

Update

746ac47

Signed-off-by: Zhixiong Niu <[email protected]>

zhixiongniu requested review from chrispsommers, ashutosh-agrawal and r12f May 7, 2024 14:57

ashutosh-agrawal reviewed May 8, 2024

View reviewed changes

Updated

3ad3a02

Signed-off-by: Zhixiong Niu <[email protected]>

zhixiongniu requested a review from ashutosh-agrawal May 9, 2024 09:12

zhixiongniu closed this May 11, 2024

zhixiongniu reopened this May 11, 2024

clarklee-guizhao reviewed May 13, 2024

View reviewed changes

zhixiongniu added 2 commits May 13, 2024 12:57

add vni support

ae30517

Signed-off-by: Zhixiong Niu <[email protected]>

Update the flow api example

7b13342

Signed-off-by: Zhixiong Niu <[email protected]>

r12f approved these changes May 18, 2024

View reviewed changes

ali-partner approved these changes May 24, 2024

View reviewed changes

marian-pritsak suggested changes Jun 3, 2024

View reviewed changes

Updated

fcc6180

Signed-off-by: Zhixiong Niu <[email protected]>

marian-pritsak approved these changes Jun 10, 2024

View reviewed changes

r12f merged commit fafe00f into sonic-net:main Jun 10, 2024
3 checks passed

		SAI_DASH_FLOW_ENABLED_KEY_SRC_PORT = 1 << 5,

		SAI_DASH_FLOW_ENABLED_KEY_DST_PORT = 1 << 6,

		SAI_DASH_FLOW_ENTRY_BULK_GET_SESSION_FILTER_KEY_FLOW_VERSION,

		SAI_DASH_FLOW_ENTRY_BULK_GET_SESSION_FILTER_KEY_AGED,

		When a service intends to use DASH Flow SAI APIs, it should first establish a flow table via the `create_flow_table()` function. After the table creation, the programmer can add, delete, modify, or retrieve flow entries to/from the table. For instance, when DASH HA needs to perform bulk sync from the active DPU to the standby DPU, it should initially fetch the entry from the active DPU using `get_flow_entries`, then transmit the flow entries to the standby DPU via the control plane. The standby DPU subsequently calls `create_flow_entries()`to add entries to the corresponding flow table.

		These examples describe how to create a flow state table, and how to operate flow entries.

Add Dash flow design #462

Add Dash flow design #462

Conversation

zhixiongniu commented Nov 13, 2023

KrisNey-MSFT commented Dec 11, 2023

zhixiongniu commented Dec 16, 2023

chrispsommers left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

r12f commented Feb 18, 2024

Choose a reason for hiding this comment

r12f commented Feb 18, 2024

KrisNey-MSFT commented Feb 20, 2024

zhixiongniu commented Feb 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zhixiongniu commented May 11, 2024

zhixiongniu commented May 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ali-partner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment