Skip to content
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.

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

HLD for PNAC feature introduction in SONiC #1292

Closed
wants to merge 69 commits into from
Closed

HLD for PNAC feature introduction in SONiC #1292

wants to merge 69 commits into from

Conversation

kishorgovind
Copy link
Collaborator

This document describes the high-level design of PNAC (Port Network Access Control) feature in SONiC.

doc/NAC/PnacSonicHLD.md Show resolved Hide resolved
doc/NAC/PnacSonicHLD.md Show resolved Hide resolved
@venkatmahalingam
Copy link
Collaborator

venkatmahalingam commented Apr 11, 2023

Looks like there is an HLD from BRCM to support the same PNAC features + MAB
#1315

IMO, we should compare and converge on what's best for the community.

doc/NAC/PnacSonicHLD.md Show resolved Hide resolved
“SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DROP”
“SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW”

When port is in unauthorized state after enabling NAC feature then port learn mode is configured as “SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DROP”.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please provide config flow for both un-authorized and authorized states.

doc/NAC/PnacSonicHLD.md Show resolved Hide resolved
doc/NAC/PnacSonicHLD.md Show resolved Hide resolved
- Update learn_mode for the specific interface in config_db to “***SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW***”
This action will trigger Orchagent invoking SAI to update the learn_mode.
After receiving interface state change notification from Authorised to Unauthorised hostapd_wrapper performs following two operations,
- Update “***nac_status***” in NAC_SESSION table which is part of config_DB to "***unauthorised***".
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this table a part of CONFIG_DB? This seems like an operational change again w.r.t PNAC.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Acknowledged. We will update this topic.

Copy link
Collaborator

@zhangyanzhao zhangyanzhao left a comment

Choose a reason for hiding this comment

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

@kishorgovind can you please help to provide the code PRs? Thanks.

@zhangyanzhao
Copy link
Collaborator

Please add the code PRs if you have. Thanks

@thovikeerthi
Copy link
Contributor

Please add the code PRs if you have. Thanks

Code Pull Request IDs:
sonic-buildimage repository : sonic-net/sonic-buildimage#15332
sonic-swss repository : sonic-net/sonic-swss#2809
sonic-utilities repository : sonic-net/sonic-utilities#2859

@msosyak
Copy link

msosyak commented Aug 8, 2023

@kishorgovind Did I understand correctly that community desided to go with #1315 with farther extention with local autentification? If yes, is this PR and related Code PRs still relevant or they should be closed?

@kishorgovind kishorgovind closed this by deleting the head repository Sep 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants