Repository navigation
[lib]: Add shared interface name validator - #4844
Closed
ashutosh-agrawal wants to merge 1 commit into
Closed
ashutosh-agrawal wants to merge 1 commit into
ashutosh-agrawal wants to merge 1 commit into
Conversation
Collaborator
|
/azp run |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
This was referenced Aug 26, 2026
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
ashutosh-agrawal
marked this pull request as ready for review
August 26, 2026 05:18
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Provide a shared interface-name syntax check and focused unit coverage for accepted and rejected values. Signed-off-by: Ashutosh Agrawal <ashu@cisco.com>
ashutosh-agrawal
force-pushed
the
fix/shared-interface-name-validation
branch
from
August 26, 2026 05:36
cd8bfc8 to
2fbb87b
Compare
Collaborator
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
qiluo-msft
reviewed
Aug 26, 2026
| // This validates interface-name syntax only. Category semantics remain in the | ||
| // YANG models and component-specific parsers. IFNAMSIZ includes the | ||
| // terminating NUL. | ||
| inline bool isValidIfname(const std::string &name) |
Contributor
There was a problem hiding this comment.
Member
Author
There was a problem hiding this comment.
Thanks, agreed. I audited the current isInterfaceNameValid() callers in sonic-utilities (PortChannel, VRF, VXLAN, and subinterface validation) against the stricter rule; their supported names continue to pass. The implementation now strengthens the existing function in sonic-swss-common and adds focused C++ and Python regression coverage, including current caller categories and the length boundaries. Replacement PR: sonic-net/sonic-swss-common#1240
Member
Author
|
Replaced by sonic-net/sonic-swss-common#1240 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What I did
Added a header-only
swss::isValidIfnamehelper and focused unit tests for shared interface-name syntax validation.The exact accepted format is:
< IFNAMSIZ).[A-Za-z0-9_.-]..or...There is no letter-first rule and no SONiC prefix requirement. Names such as
1Ethernet,_eth0, and.eth0match the format. Interface category, parent existence, and naming conventions remain the responsibility of YANG and component-specific parsers.Implementation and coverage:
lib/ifname.handtests/ifname_ut.cpp.Why I did it
A shared helper gives SWSS components one interface-name syntax rule while leaving interface-category rules to the existing YANG models and component parsers.
Linux
dev_valid_nameand SONiC's YANGinterface_nametypedef provide the underlying length and naming constraints. Physical-interface and VLAN-subinterface semantics remain defined bysonic-interface.yangandsonic-vlan-sub-interface.yang.How I verified it
Built and ran the focused tests in the Bookworm build environment:
The tests cover names that match the exact format and each nonmatching length, character, whole-name, and first-character case.
Details if related
Dependent draft PRs #4845, #4846, and #4847 are intentionally paused. After this helper merges, each consumer branch will be rebuilt from current
masterso its review diff contains only the dependent integration.