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

Add helper function to validate interface name length #889

Conversation

arfeigin
Copy link
Contributor

@arfeigin arfeigin commented Jul 3, 2024

This PR is accompanied by sonic-net/sonic-buildimage#20108 and sonic-net/sonic-utilities#3397

Add helper function to validate that interface name length does not exceed allowed length configured as IFNAMSIZ. This will be used both by sonic-utilities CLI (config) and by cfgmgr.

@arfeigin arfeigin requested a review from prsunny July 8, 2024 16:27
@arfeigin
Copy link
Contributor Author

arfeigin commented Jul 8, 2024

@prsunny Can you help please reviewing this and the attached PRs?

@prsunny prsunny requested a review from qiluo-msft August 6, 2024 00:47
@dprital
Copy link
Collaborator

dprital commented Aug 14, 2024

@prsunny , @qiluo-msft , Can you please approve this PR ?


size_t ifaceNameMaxLen()
{
return IFNAMSIZ;
Copy link
Contributor

Choose a reason for hiding this comment

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

@liuh-80 The purpose of this PR is to share a const to multiple applications in C++/python/go. What is the best practice? any sample?

Copy link
Contributor

@liuh-80 liuh-80 Sep 24, 2024

Choose a reason for hiding this comment

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

The best practice is to define a const variable, SWIG will generate a global read-only wrapper:
https://www.swig.org/Doc4.0/SWIGDocumentation.html#SWIG_nn13

I think the issue here is IFNAMESIZ is not defined in sonic-swss-common, it's defined in the libc.

So, the SWIG can't generate wrapper for it.

@qiluo-msft
Copy link
Contributor

Do you still need this PR? if not, you can 'Close' it.

@arfeigin
Copy link
Contributor Author

Do you still need this PR? if not, you can 'Close' it.

I think it is good to have this PR in order to export IFNAMSIZ value for sonic-utilities config scripts, not?

@liuh-80
Copy link
Contributor

liuh-80 commented Sep 24, 2024

@arfeigin , a workaround could be define a new variable:
const size_t IFACE_NAME_MAX_LEN = IFNAMESIZ;

@stepanblyschak
Copy link
Contributor

@qiluo-msft @liuh-80 Please review #931

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.

6 participants