Skip to content

Add DONOTDROP packet action. #1349

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.

Sign up for GitHub

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

Merged
rlhui merged 1 commit into from
Jun 2, 2022
Merged

Add DONOTDROP packet action. #1349

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 97 additions & 0 deletions doc/Packet-action-DONOTDROP.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# SAI_PACKET_ACTION_DONOTDROP packet action
-------------------------------------------------------------------------------
Title | SAI_PACKET_ACTION_DONOTDROP packet action
-------------|-----------------------------------------------------------------
Authors | Ashish Singh, Google LLC
Status | In review
Type | Standards track
Created | 01/25/2022

-------------------------------------------------------------------------------

This spec discusses the SAI_PACKET_ACTION_DONOTDROP packet action use-cases.

Goal of this packet action attribute (SAI_PACKET_ACTION_DONOTDROP) is to resolve the conflicts of ACL actions when a packet hits more than one ACLs in an ACL stage such that when this packet action is used with an ACL, it serves as an action that prevents the possible drop from another ACL that is in a lower priority table.

This new packet action attribute is in line with the current SAI architecture - actions in higher priority groups take precedence. If an ACL with DROP action is in a higher priority group than this ACL, DROP action is expected to be honored. Table priority group of a table can be set using SAI_ACL_TABLE_GROUP_MEMBER_ATTR_PRIORITY.

Scope of the SAI_PACKET_ACTION_DONOTDROP packet action is limited to the ACL stage where it is used in the ACL as packet action - it would not override the DROP action in subsequent ACL stage(s) tables.

Few cases are explained with examples in next section.

Copy link
Copy Markdown
Contributor

@rck-innovium rck-innovium Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the behavior if the highest priority entry in ACL stage 1 (say ingress) marks as DO_NOT_DROP, and another entry in ACL Stage 2 (say egress) marks the packet as drop?

In order to avoid ambiguities, we should spell out that DO NOT DROP overrides the DROP action set by:

  1. ACL tables with lower priority in this ACL Table Group, AND

  2. ACL tables in preceding stages

In other words, it would not override the drop action set in subsequent ACL Table stages.

Copy link
Copy Markdown
Contributor Author

@Ashish1805 Ashish1805 Apr 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As per my understanding, ACL action resolution is done per-stage and an ingress stage DONOTDROP action should not have have impact on VFP or EFP actions.

Copy link
Copy Markdown
Contributor Author

@Ashish1805 Ashish1805 May 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the doc with comment on DONOTDROP action in one stage will not override the drop action is subsequent ACL stages. Thank you.

# Usage Examples
### Case 1

| ACL in table T1 | ACL in table T2 |
|-------------------------------------|---------------------------------------|
| Table priority group: P1 | Table priority group: P2 |
Copy link
Copy Markdown
Contributor

@ashutosh-agrawal ashutosh-agrawal Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please elaborate on "Table priority group"? Currently, there is no priority attribute for ACL Group table in SAI.

Copy link
Copy Markdown
Contributor

@rck-innovium rck-innovium Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It is good to clarify if you meant SAI_ACL_TABLE_GROUP_MEMBER_ATTR_PRIORITY.

Copy link
Copy Markdown
Contributor

@ashutosh-agrawal ashutosh-agrawal Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But SAI_ACL_TABLE_GROUP_MEMBER_ATTR_PRIORITY is applicable only for the sequential ACL groups. For a sequential ACL group, only the highest priority table is applied so if the highest priority table has a redirect action, it will apply regardless of any conflicting actions in the lower priority tables. So, I'm bit unclear how this new packet action type will be used.

Copy link
Copy Markdown
Contributor Author

@Ashish1805 Ashish1805 Apr 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. Thank you.

Copy link
Copy Markdown
Contributor Author

@Ashish1805 Ashish1805 May 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ashutosh-agrawal would you take a look at whether the documentation addresses your question? Thank you.

|Action: SAI_PACKET_ACTION_DONOTDROP | Action: SAI_PACKET_ACTION_DROP |



If P1 > P2: action is “no DROP”


If P2 > P1: action is “DROP”



### Case 2

| ACL in table T1 | ACL in table T2 |
|-------------------------------------|---------------------------------------|
| Table priority group: P1 | Table priority group: P2 |
|Action: SAI_PACKET_ACTION_DONOTDROP | Action: neither SAI_PACKET_ACTION_DROP nor SAI_PACKET_ACTION_DONOTDROP |


If P1 > P2: action is “no DROP”


If P2 > P1: action is “no DROP”


### Case 3

| ACL in table T1 | ACL in table T2 |
|-------------------------------------|---------------------------------------|
| Table priority group: P1 | Table priority group: P2 |
|Action: neither SAI_PACKET_ACTION_DROP conflicting action nor SAI_PACKET_ACTION_DONOTDROP | Action: SAI_PACKET_ACTION_DROP |


Copy link
Copy Markdown
Contributor

@rck-innovium rck-innovium Feb 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to define the behavior for enums that are "Combination of Packet Actions" like SAI_PACKET_ACTION_TRAP. For example, can you explain the below:

ACL in table T1 ACL in table T2
Table priority group: P1 Table priority group: P2
Action: SAI_PACKET_ACTION_DONOTDROP Action: TRAP

If P1 > P2: action is “Copy to CPU?”

If P2 > P1: action is “Trap?”

Copy link
Copy Markdown
Contributor Author

@Ashish1805 Ashish1805 Apr 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added behavior of DONOTDROP with TRAP and DENY. Thank you.

Copy link
Copy Markdown
Contributor Author

@Ashish1805 Ashish1805 May 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rck-innovium would you take a look at whether the documentation addresses your question? Thank you.

If P1 > P2: action is “DROP”


If P2 > P1: action is “DROP”

### Case 4

For SAI actions that are implemented as a copy of other actions including a DROP, the SAI_PACKET_ACTION_DONOTDROP action acts on only the DROP portion of the action.

TRAP action may be implemented as COPY + DROP. A higher-priority DONOTDROP action will cancel the DROP action only, resulting in the packet being copied to CPU but not dropped.

| ACL in table T1 | ACL in table T2 |
|-------------------------------------|---------------------------------------|
| Table priority group: P1 | Table priority group: P2 |
|Action: SAI_PACKET_ACTION_DONOTDROP | Action: SAI_PACKET_ACTION_TRAP |


If P1 > P2: action is "copy to CPU"


If P2 > P1: action is "TRAP"

### Case 5

DENY action may be implemented as COPY_CANCEL + DROP. A higher-priority DONOTDROP action will cancel the DROP action only, resulting action as COPY_CANCEL.

| ACL in table T1 | ACL in table T2 |
|-------------------------------------|---------------------------------------|
| Table priority group: P1 | Table priority group: P2 |
|Action: SAI_PACKET_ACTION_DONOTDROP | Action: SAI_PACKET_ACTION_DENY |


If P1 > P2: action is "COPY_CANCEL"


If P2 > P1: action is "DENY"

5 changes: 4 additions & 1 deletion inc/saiswitch.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,10 @@ typedef enum _sai_packet_action_t
SAI_PACKET_ACTION_DENY,

/** This is a combination of SAI packet action COPY_CANCEL and FORWARD */
SAI_PACKET_ACTION_TRANSIT
SAI_PACKET_ACTION_TRANSIT,

/** Do not drop the packet. */
SAI_PACKET_ACTION_DONOTDROP
Copy link
Copy Markdown
Collaborator

@kcudnik kcudnik Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats the difference with SAI_PACKET_ACTION_FORWARD?

Copy link
Copy Markdown
Contributor Author

@Ashish1805 Ashish1805 Dec 2, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our intention here is to get an acl packet action which can help override ACL DROP action of another ACL based on acl priority groups of these ACLs.

For example:
Lets say we have an ACL in a table T1 (with ACL group priority = X) that has ACL action = DROP and we have another ACL in table Table T2 (With ACL group priority = Y) with ACL action = REDIRECT and a packet is hitting both the ACLs.

Since, ACL actions REDIRECT and DROP are conflicting actions on a packet, action resolution will happen. Irrespective of acl group priority of Table T1 and T2, DROP action will take precedence and packet will be dropped.

We want packet to not get dropped rather be redirected (which means we want to override the ACL DROP action of ACL in Table T1 by ACL REDIRECT action of ACL in Table T2) when Table T2's ACL group priority is higher than Table T1's ACL group priority, i.e. Y > X. To achieve this we want to add packet action SAI_PACKET_ACTION_DONOTDROP.

Using this (ACL action = REDIRECT + packet action = SAI_PACKET_ACTION_DONOTDROP), Acl in Table2 can override the ACL DROP action of ACL action in Table 1 given acl group priority of Table2 > acl group priority of Table1.

As per my understanding, SAI_PACKET_ACTION_FORWARD packet action helps forward the packets using forwarding info from L2/L3 table if packets are not being dropped - but it cannot help override ACL DROP action itself irrespective of acl group priorities of ACL tables.

Copy link
Copy Markdown
Contributor

@mikeberesford mikeberesford Jan 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kcudnik would you take a look at whether the documentation addresses your question?

Copy link
Copy Markdown
Collaborator

@kcudnik kcudnik Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

explanation is very informative, thanks


} sai_packet_action_t;

Expand Down