Combine v4 and v6 L3 ACL rules on optimized platforms #1267

rck-innovium · 2023-02-18T10:24:30Z

This document describes the SONiC ACL optimization for using IPv4 ACL rules in L3V6 ACL tables on platforms optimized for this.

Repo	PR title	State
sonic-swss	Combine v4 and v6 L3 ACL rules on optimized platforms	Merged
sonic-utilities	[acl-loader] Support for ACL table type L3V4V6	Merged
sonic-buildimage	Support a new ACL table type called L3V4V6	Merged
sonic-mgmt	Test plan for L3V4V6 type ACL table	Merged
sonic-mgmt	PTF Test for L3V4V6 type ACL table	Open

zhangyanzhao · 2023-02-21T16:05:01Z

reviewed in community meeting.

doc/acl/Extend-L3V6ACLs.md

bingwang-ms · 2023-02-21T16:57:28Z

The proposal looks god to me. Actually, I was doing similar investigation as well. I tried to implement same feature with the custom ACL type introduced by this PR sonic-net/sonic-swss#1982. We may define the new ACL table type by configuration without code change,
I also added a test case to verify that sonic-net/sonic-mgmt#6905 .

venkatmahalingam · 2023-02-21T17:03:15Z

@zhangyanzhao Please add me as the reviewer.

doc/acl/Extend-L3V6ACLs.md

ghost · 2023-02-21T19:56:58Z

Hi, There might be a mistake, I’m not involved at all in this. Please stop sending me notification emails about that. Thank you Best regards / Cordialement, Antonino NAPOLI Manager Advanced Services O: +33 972 601 900 | M: +33 766 036 737 A: 19 rue des châteaux, 59290 Wasquehal, FRANCE [A close up of a logo Description automatically generated]<https://www.expereo.com/> From: svshah-intel ***@***.***> Sent: Tuesday, February 21, 2023 8:55 PM To: sonic-net/SONiC ***@***.***> Cc: Subscribed ***@***.***> Subject: Re: [sonic-net/SONiC] Support v4 ACL rules in V6 ACL tables on optimized platforms (PR #1267) You don't often get email from ***@***.******@***.***>. Learn why this is important<https://aka.ms/LearnAboutSenderIdentification> @svshah-intel commented on this pull request.

________________________________ In doc/acl/Extend-L3V6ACLs.md<https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsonic-net%2FSONiC%2Fpull%2F1267%23discussion_r1113508338&data=05%7C01%7C%7C70f5ac7b7ac04859c83508db14458b93%7Cd8b0675bbea4400292891da6ecc1ab6c%7C1%7C0%7C638126061161778453%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=HzlxhLcdfoVzrROY5Dqr4tziJgf0cxxk8YsXazBzwVw%3D&reserved=0>:

+ * | MATCH_L4_SRC_PORT | √ |

+ * | MATCH_L4_DST_PORT | √ | + * | MATCH_TCP_FLAGS | √ | + * |-------------------------------------| + */ + + +##### Pros and Cons +All the pros and cons of option-B (extending L3V6 ACL table) applies here as well. +Additionally, option-C has the below cons: +- Need platform checks to determine which platforms can support combined v4 and v6 ACL table. +- Even on platforms that support combined v4 and v6 ACL tables, we need additional checks to identify which platforms are optimized to have v4 and v6 in the same ACL table. + +#### Implementation + +Based on the above design considerations, option-B is implemented. @rck-innovium<https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Frck-innovium&data=05%7C01%7C%7C70f5ac7b7ac04859c83508db14458b93%7Cd8b0675bbea4400292891da6ecc1ab6c%7C1%7C0%7C638126061161778453%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=oazoD%2BMKCSVQhEyEGmXTGyKYAzvpfcUmjPnjdVmd3oo%3D&reserved=0> as discussed in the meeting today, it is worth to take a look at how openconfig ACL yang model supports mixed mode, and if we can follow similar option. — Reply to this email directly, view it on GitHub<https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fsonic-net%2FSONiC%2Fpull%2F1267%23pullrequestreview-1308078057&data=05%7C01%7C%7C70f5ac7b7ac04859c83508db14458b93%7Cd8b0675bbea4400292891da6ecc1ab6c%7C1%7C0%7C638126061161778453%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=fZ02l3RQZDrrui%2BcpFOl8VRTFZbO3Q%2FyMZbKYUhrzlk%3D&reserved=0>, or unsubscribe<https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FACZOGFYK2QEUQNIZ63UFNOLWYUMRZANCNFSM6AAAAAAVAISUMM&data=05%7C01%7C%7C70f5ac7b7ac04859c83508db14458b93%7Cd8b0675bbea4400292891da6ecc1ab6c%7C1%7C0%7C638126061161778453%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=YV1XUCR1rxGloH13bIFNm%2FDmfPBOVaUXdi%2Ft8AHgv9M%3D&reserved=0>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.******@***.***>>

doc/acl/Extend-L3V6ACLs.md

zhangyanzhao · 2023-02-28T00:07:48Z

community review recording https://zoom.us/rec/share/4-yYo-RXcFAKMxptEFExMMFYYGJnHfizv9G9I3ySVzdAfg-JN21vJdg5MOTR4HKz.OqXPjTeeWNi64ocW

rck-innovium · 2023-03-21T18:30:47Z

The proposal looks god to me. Actually, I was doing similar investigation as well. I tried to implement same feature with the custom ACL type introduced by this PR sonic-net/sonic-swss#1982. We may define the new ACL table type by configuration without code change, I also added a test case to verify that sonic-net/sonic-mgmt#6905 .

Please look at the updated proposal.
On platforms NOT supporting v4 and v6 in a single SAI ACL table, this new L3V4V6 ACL table will be supported by orchagent's special handling. Orchagent would internally create and manage two SAI ACL tables as shown in the right picture below. The operator is agnostic about the underlying ASIC behavior and only sees a consolidated SONiC ACL table.

This needs ACL orchagent changes.

doc/acl/Extend-L3V6ACLs.md

Support a new ACL table type called L3V4V6. This table supports both v4 and v6 Match types. Add unit tests for this new ACL table type. HLD: sonic-net/SONiC#1267 Signed-off-by: Ravi(Marvell) rck@innovium.com

This table supports both v4 and v6 Match types. HLD: sonic-net/SONiC#1267 Signed-off-by: Ravi(Marvell) rck@innovium.com

Support a new ACL table type called L3V4V6. This table supports both v4 and v6 Match types. Add unit tests for this new ACL table type. HLD: sonic-net/SONiC#1267 Signed-off-by: Ravi(Marvell) rck@innovium.com

This table supports both v4 and v6 Match types. HLD: sonic-net/SONiC#1267 Signed-off-by: Ravi(Marvell) rck@innovium.com

Support a new ACL table type called L3V4V6. This table supports both v4 and v6 Match types. Add unit tests for this new ACL table type. HLD: sonic-net/SONiC#1267 Signed-off-by: Ravi(Marvell) rck@innovium.com

doc/acl/Extend-L3V6ACLs.md

rules in L3V6 ACL tables on platforms optimized for this.

comments.

Ether-type and IP type is not enforced in SWSS. The underlying SAI platform handles the case when both of these fields are not provided.

This table supports both v4 and v6 Match types. HLD: sonic-net/SONiC#1267 Signed-off-by: Ravi(Marvell) rck@innovium.com Signed-off-by: Ravi(Marvell) rck@innovium.com

Support a new ACL table type called L3V4V6. This table supports both v4 and v6 Match types. Add unit tests for this new ACL table type. HLD: sonic-net/SONiC#1267 Signed-off-by: Ravi(Marvell) rck@innovium.com

doc/acl/Extend-L3V6ACLs.md

Support a new ACL table type called L3V4V6. This table supports both v4 and v6 Match types. Add unit tests for this new ACL table type. HLD: sonic-net/SONiC#1267

rck-innovium force-pushed the marvell_l3_acl branch from 6a7a0d0 to b94b023 Compare February 18, 2023 18:09

bingwang-ms reviewed Feb 21, 2023

View reviewed changes

doc/acl/Extend-L3V6ACLs.md Show resolved Hide resolved

bingwang-ms reviewed Feb 21, 2023

View reviewed changes

doc/acl/Extend-L3V6ACLs.md Show resolved Hide resolved

svshah-intel reviewed Feb 21, 2023

View reviewed changes

doc/acl/Extend-L3V6ACLs.md Outdated Show resolved Hide resolved

venkatmahalingam reviewed Feb 22, 2023

View reviewed changes

doc/acl/Extend-L3V6ACLs.md Show resolved Hide resolved

rck-innovium requested review from venkatmahalingam, svshah-intel and bingwang-ms March 21, 2023 18:33

rck-innovium changed the title ~~Support v4 ACL rules in V6 ACL tables on optimized platforms~~ Combine v4 and v6 L3 ACL rules on optimized platforms Apr 3, 2023

bingwang-ms reviewed Apr 3, 2023

View reviewed changes

doc/acl/Extend-L3V6ACLs.md Show resolved Hide resolved

bingwang-ms reviewed Apr 3, 2023

View reviewed changes

doc/acl/Extend-L3V6ACLs.md Outdated Show resolved Hide resolved

bingwang-ms reviewed Apr 3, 2023

View reviewed changes

doc/acl/Extend-L3V6ACLs.md Show resolved Hide resolved

bingwang-ms reviewed Apr 3, 2023

View reviewed changes

doc/acl/Extend-L3V6ACLs.md Show resolved Hide resolved

rck-innovium mentioned this pull request Apr 14, 2023

Combine v4 and v6 L3 ACL rules on optimized platforms #1267 sonic-net/sonic-swss#2735

Merged

rck-innovium added a commit to rck-innovium/sonic-mgmt-common that referenced this pull request Apr 14, 2023

Support a new ACL table type called L3V4V6.

3340781

This table supports both v4 and v6 Match types. HLD: sonic-net/SONiC#1267 Signed-off-by: Ravi(Marvell) rck@innovium.com

rck-innovium added a commit to rck-innovium/sonic-mgmt-common that referenced this pull request Apr 14, 2023

Support a new ACL table type called L3V4V6.

542e043

This table supports both v4 and v6 Match types. HLD: sonic-net/SONiC#1267 Signed-off-by: Ravi(Marvell) rck@innovium.com

rck-innovium mentioned this pull request Apr 14, 2023

Support a new ACL table type called L3V4V6. sonic-net/sonic-mgmt-common#71

Closed

rck-innovium mentioned this pull request Apr 16, 2023

[acl-loader] Support for ACL table type L3V4V6 sonic-net/sonic-utilities#2794

Merged

rck-innovium requested a review from bingwang-ms April 16, 2023 18:43

bingwang-ms previously approved these changes Apr 18, 2023

View reviewed changes

rck-innovium added a commit to rck-innovium/sonic-buildimage that referenced this pull request Apr 22, 2023

Support a new ACL table type called L3V4V6.

e542a67

This table supports both v4 and v6 Match types. HLD: sonic-net/SONiC#1267 Signed-off-by: Ravi(Marvell) rck@innovium.com

rck-innovium mentioned this pull request Apr 22, 2023

Support a new ACL table type called L3V4V6. sonic-net/sonic-buildimage#14803

Merged

1 task

rck-innovium requested review from svshah-intel and removed request for svshah-intel April 22, 2023 18:45

svshah-intel reviewed Apr 23, 2023

View reviewed changes

doc/acl/Extend-L3V6ACLs.md Outdated Show resolved Hide resolved

rck-innovium added 3 commits April 30, 2023 14:40

This document describes the SONiC ACL optimization for using IPv4 ACL

5298e3b

rules in L3V6 ACL tables on platforms optimized for this.

Address comments received during the community discussion and offline

384d843

comments.

Address review comments on Ether-type and IP-type

f744bab

Ether-type and IP type is not enforced in SWSS. The underlying SAI platform handles the case when both of these fields are not provided.

rck-innovium dismissed bingwang-ms’s stale review via f744bab April 30, 2023 09:14

rck-innovium force-pushed the marvell_l3_acl branch from aac0039 to f744bab Compare April 30, 2023 09:14

rck-innovium requested review from bingwang-ms and svshah-intel April 30, 2023 09:16

svshah-intel approved these changes May 1, 2023

View reviewed changes

rck-innovium assigned bingwang-ms May 5, 2023

bingwang-ms approved these changes May 5, 2023

View reviewed changes

Merge branch 'master' into marvell_l3_acl

808e0c8

prsunny reviewed May 12, 2023

View reviewed changes

doc/acl/Extend-L3V6ACLs.md Show resolved Hide resolved

Merge branch 'master' into marvell_l3_acl

f527931

Merge branch 'master' into marvell_l3_acl

6744b7d

rck-innovium requested a review from prsunny May 26, 2023 03:21

Merge branch 'master' into marvell_l3_acl

38caea1

prsunny merged commit 58bb6b9 into sonic-net:master May 27, 2023
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Combine v4 and v6 L3 ACL rules on optimized platforms #1267

Combine v4 and v6 L3 ACL rules on optimized platforms #1267

rck-innovium commented Feb 18, 2023 •

edited

zhangyanzhao commented Feb 21, 2023

bingwang-ms commented Feb 21, 2023

venkatmahalingam commented Feb 21, 2023

ghost commented Feb 21, 2023 via email

zhangyanzhao commented Feb 28, 2023

rck-innovium commented Mar 21, 2023

Combine v4 and v6 L3 ACL rules on optimized platforms #1267

Combine v4 and v6 L3 ACL rules on optimized platforms #1267

Conversation

rck-innovium commented Feb 18, 2023 • edited

zhangyanzhao commented Feb 21, 2023

bingwang-ms commented Feb 21, 2023

venkatmahalingam commented Feb 21, 2023

ghost commented Feb 21, 2023 via email

zhangyanzhao commented Feb 28, 2023

rck-innovium commented Mar 21, 2023

rck-innovium commented Feb 18, 2023 •

edited