SAI proposal for icmp echo offload #2020

chikkaiah-work · 2024-05-28T20:39:29Z

This specification suggests:

The idea of an ICMP echo object or session that can be transferred to HW/ASIC or other modules for faster link detection.
The introduction of SAI APIs to outline ICMP echo session properties.
The introduction of a switch-level notification API to monitor changes in ICMP echo session states.
The switchover flow on link down detection.

Creating this PR to recommit #1943.

zjswhhh

Please update with:

Jai's questions in the offline emails
LAG solution

Signed-off-by: chikkaiah-work <[email protected]> fix sanity Signed-off-by: chikkaiah-work <[email protected]> add LAG info Signed-off-by: chikkaiah-work <[email protected]> sanity fix Signed-off-by: chikkaiah-work <[email protected]> fix sanity Signed-off-by: chikkaiah-work <[email protected]> sanity fix Signed-off-by: chikkaiah-work <[email protected]> sanity fix Signed-off-by: chikkaiah-work <[email protected]>

Signed-off-by: chikkaiah-work <[email protected]>

chikkaiah-work · 2024-06-07T01:31:41Z

Please update with:

1. Jai's questions in the offline emails

2. LAG solution

chikkaiah-work · 2024-06-07T01:32:30Z

closed by mistake

chikkaiah-work · 2024-06-07T01:34:41Z

Please update with:

1. Jai's questions in the offline emails

2. LAG solution

Added SAI_ICMP_ECHO_SESSION_ATTR_RX_PORT to specify RX port.
Added doc section "“ICMP echo session with LAG”

JaiOCP · 2024-06-18T21:00:54Z

inc/saiicmpecho.h

+     * @flags CREATE_ONLY
+     * @default true
+     */
+    SAI_ICMP_ECHO_SESSION_ATTR_HW_LOOKUP_VALID = SAI_ICMP_ECHO_SESSION_ATTR_START,


I believe hw lookup means that ICMP packet is forwarded using L3 lookup instead of forwarding to a port. Please rename the attribute and add comments to that intent.

You are correct. It's intended for L3 lookup for forwarding. I am willing to rename it, but the same naming convention is also used in saitwamp.h and saibfd.h, and I prefer to maintain consistency. Please let me know if still want me to rename it.

In that case please add details in comments clearly stating what happens when flag is set to true or false

Added a brief description for this attribute. Also, this attribute is mentioned explained in the examples.

JaiOCP · 2024-06-18T21:01:48Z

inc/saiicmpecho.h

+     * @objects SAI_OBJECT_TYPE_PORT
+     * @condition SAI_ICMP_ECHO_SESSION_ATTR_HW_LOOKUP_VALID == false
+     */
+    SAI_ICMP_ECHO_SESSION_ATTR_PORT,


This means that session is created between peering links. How do we specify more sessions i.e. monitoring between multiple peering links

This is for directly connected ports. only one session is allowed per port. same concept as in BFD.

This means that SONiC has to create multiple echo sessions; one per peering ports.
If thats the requirement then you may want to add the CRM attribute in switch. Something like
SAI_SWITCH_ATTR_AVAILABLE_ICMP_ECHO_SESSIONS read only attribute.
Initial read will give the max and as more sessions as added subsequent reads will give remaining.

Added SAI_SWITCH_ATTR_AVAILABLE_ICMP_ECHO_SESSION and SAI_SWITCH_ATTR_MAX_ICMP_ECHO_SESSION.

JaiOCP · 2024-06-18T21:03:58Z

inc/saiicmpecho.h

+     * @flags CREATE_AND_SET
+     * @default false
+     */
+    SAI_ICMP_ECHO_SESSION_ATTR_SET_NEXT_HOP_GROUP_SWITCHOVER,


This is very fuzzy. What happens if there are multiple such protection NHGs. How does this attribute indicates which one to of the protection NHG takes effect. We just can't trigger all the protection NHG to take over.

@chikkaiah-work Please look at this comment as well

You brought up a valid point. If it's possible for various NHGs to share the same monitor object, it seems reasonable to consider modifying this attribute to the NHG object ID. Should this attribute be left unset (which is null as default), automatic switchover will not occur. Instead, the switchover should be initiated based on the specified NHG. Please let me know if this sounds fine to you

This attribute is acting as a knob to enable/disable the switchover of the protection groups that have monitor object as this ICMP echo session.
@chikkaiah-work, maybe you could provide protection group examples to justify this.

@lolyu, you are right. Also I have updated the FRR document by mentioning echo session could be included as monitor object. I have added an example under the section "ICMP echo session with protection group"

Thanks for updating the spec with an example workflow. Looks good to me.

lolyu · 2024-06-21T03:30:23Z

doc/SAI-Proposal-ICMP-ECHO.md

+#### sai_icmp_echo_session_attr_t ####
+This defines icmp echo session attributes. These attributes collectively define the configuration and behavior of ICMP echo sessions in the SAI framework.
+
+- SAI_ICMP_ECHO_SESSION_ATTR_VIRTUAL_ROUTER: Specifies whether hardware lookup is valid, allowing for virtual router configuration.


should be SAI_ICMP_ECHO_SESSION_ATTR_HW_LOOKUP_VALID

lolyu · 2024-06-21T03:37:58Z

doc/SAI-Proposal-ICMP-ECHO.md

+```
+***GUID:*** Global unique identifier for the ICMP echo session to correlate probe and reply packets with a session, especially when the session is offloaded to hardware, a unique identifier is required.
+
+***Session Cookie:*** The offloading engine interprets this value to decide whether the packet should undergo processing by the ICMP echo session or not. Packets not matching the cookie are punted to NOS. This can also be used to differentiate regular icmp response and icmp link monitoring response.


should we use the same cookie for sessions on the same device or we can use multiple cookies?

Each session can share the same cookie, but if a device needs to communicate with multiple devices, each session can have its own unique cookie. The decision to assign cookies to sessions is implementation-specific, and the device can implement a check to enforce the desired behavior.

lolyu · 2024-06-21T03:43:22Z

doc/SAI-Proposal-ICMP-ECHO.md

+
+### 4.0 Examples ###
+
+#### 4.0.1 Create session using egress interface: ####


Please provide examples with LAG

Added a section "ICMP echo session with LAG" and explained session creation process.

lolyu · 2024-06-21T03:47:44Z

inc/saiicmpecho.h

+     * @flags CREATE_AND_SET
+     * @default false
+     */
+    SAI_ICMP_ECHO_SESSION_ATTR_SET_NEXT_HOP_GROUP_SWITCHOVER,


This attribute is acting as a knob to enable/disable the switchover of the protection groups that have monitor object as this ICMP echo session.
@chikkaiah-work, maybe you could provide protection group examples to justify this.

lolyu · 2024-06-21T07:05:34Z

doc/SAI-Proposal-ICMP-ECHO.md

+
+### 4.0 Examples ###
+
+#### 4.0.1 Create session using egress interface: ####


also, please provide examples of sessions with protection group.

Added example with section tiltled " ICMP echo session with protection group"

Signed-off-by: chikkaiah-work <[email protected]>

lolyu · 2024-07-01T03:50:48Z

doc/SAI-Proposal-ICMP-ECHO.md

@@ -422,3 +469,12 @@ void myCallbackFunction(uint32_t count, sai_icmp_echo_session_state_notification
    }
 }
 ```
+### ICMP echo session with LAG ###
+
+- Sessions across a LAG bundle: Individual sessions are created on each of the member links. The device creates ICMP echo requests for each configured probing session, which are sent through the port specified by the Session attribute SAI_ICMP_ECHO_SESSION_ATT_PORT.


s/SAI_ICMP_ECHO_SESSION_ATT_PORT/SAI_ICMP_ECHO_SESSION_ATTR_PORT

Signed-off-by: chikkaiah-work <[email protected]>

zjswhhh

LGTM.

Please wait for Jai's approval before merging.

chikkaiah-work · 2024-07-08T23:49:20Z

@kcudnik - can you please help merge this.

chikkaiah-work · 2024-07-10T17:15:55Z

@kcudnik - can you please help merge this.

@rlhui - can we get this merged please ?

Signed-off-by: siqbal1986 <[email protected]>

zjswhhh reviewed May 31, 2024

View reviewed changes

chikkaiah-work added 2 commits June 6, 2024 21:13

fixes

df952c4

Signed-off-by: chikkaiah-work <[email protected]>

chikkaiah-work closed this Jun 7, 2024

chikkaiah-work reopened this Jun 7, 2024

chikkaiah-work requested a review from zjswhhh June 7, 2024 01:39

JaiOCP reviewed Jun 18, 2024

View reviewed changes

lolyu reviewed Jun 21, 2024

View reviewed changes

address review comments

35a91d1

Signed-off-by: chikkaiah-work <[email protected]>

lolyu reviewed Jul 1, 2024

View reviewed changes

doc fix

0c11fa0

Signed-off-by: chikkaiah-work <[email protected]>

zjswhhh approved these changes Jul 5, 2024

View reviewed changes

JaiOCP approved these changes Jul 8, 2024

View reviewed changes

chikkaiah-work changed the title ~~Recommit - SAI proposal for icmp echo offload #1943~~ SAI proposal for icmp echo offload Jul 8, 2024

lolyu approved these changes Jul 9, 2024

View reviewed changes

kcudnik approved these changes Jul 9, 2024

View reviewed changes

tjchadaga merged commit 19b30c1 into opencomputeproject:master Jul 10, 2024
3 checks passed

siqbal1986 pushed a commit to siqbal1986/SAI that referenced this pull request Sep 30, 2024

SAI proposal for icmp echo offload (opencomputeproject#2020)

01c4729

Signed-off-by: siqbal1986 <[email protected]>

erohsik pushed a commit to erohsik/SAI that referenced this pull request Nov 7, 2024

SAI proposal for icmp echo offload (opencomputeproject#2020)

95accbc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SAI proposal for icmp echo offload #2020

SAI proposal for icmp echo offload #2020

chikkaiah-work commented May 28, 2024 •

edited

Loading

zjswhhh left a comment

chikkaiah-work commented Jun 7, 2024

chikkaiah-work commented Jun 7, 2024

chikkaiah-work commented Jun 7, 2024

JaiOCP Jun 18, 2024

chikkaiah-work Jun 20, 2024

JaiOCP Jun 20, 2024

chikkaiah-work Jun 30, 2024

JaiOCP Jul 3, 2024

JaiOCP Jun 18, 2024

chikkaiah-work Jun 20, 2024 •

edited

Loading

JaiOCP Jun 20, 2024

chikkaiah-work Jun 30, 2024

JaiOCP Jul 3, 2024

JaiOCP Jun 18, 2024

JaiOCP Jun 20, 2024

chikkaiah-work Jun 20, 2024

lolyu Jun 21, 2024 •

edited

Loading

chikkaiah-work Jun 30, 2024

JaiOCP Jul 8, 2024

lolyu Jun 21, 2024

chikkaiah-work Jun 30, 2024

lolyu Jun 21, 2024

chikkaiah-work Jun 30, 2024

lolyu Jun 21, 2024

chikkaiah-work Jun 30, 2024

lolyu Jun 21, 2024 •

edited

Loading

lolyu Jun 21, 2024

chikkaiah-work Jun 30, 2024

lolyu Jul 1, 2024

chikkaiah-work Jul 1, 2024

zjswhhh left a comment

chikkaiah-work commented Jul 8, 2024

chikkaiah-work commented Jul 10, 2024


		### 4.0 Examples ###

		#### 4.0.1 Create session using egress interface: ####

SAI proposal for icmp echo offload #2020

SAI proposal for icmp echo offload #2020

Conversation

chikkaiah-work commented May 28, 2024 • edited Loading

zjswhhh left a comment

Choose a reason for hiding this comment

chikkaiah-work commented Jun 7, 2024

chikkaiah-work commented Jun 7, 2024

chikkaiah-work commented Jun 7, 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

chikkaiah-work Jun 20, 2024 • edited Loading

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

lolyu Jun 21, 2024 • edited Loading

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

lolyu Jun 21, 2024 • edited Loading

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

zjswhhh left a comment

Choose a reason for hiding this comment

chikkaiah-work commented Jul 8, 2024

chikkaiah-work commented Jul 10, 2024

chikkaiah-work commented May 28, 2024 •

edited

Loading

chikkaiah-work Jun 20, 2024 •

edited

Loading

lolyu Jun 21, 2024 •

edited

Loading

lolyu Jun 21, 2024 •

edited

Loading