-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Routed subinterface enhancements #8761
Conversation
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
HLD is not merged yet, we should merge the HLD before merging this code PR. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
if intf is None: | ||
return None | ||
|
||
if VLAN_SUB_INTERFACE_SEPARATOR in intf: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to check Eth or Po prefix as well instead of blindly checking for "." in the intf string and returning long interface name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In certain case we need to fetch full name for the given subinterface parent interface which could be in short name format. Just checking for "." will not suffice here.
This pull request introduces 1 alert when merging 60500f345691633fd466ca40592c57c5680adf8d into 07038a0 - view on LGTM.com new alerts:
|
/azp run |
Commenter does not have sufficient privileges for PR 8761 in repo Azure/sonic-buildimage |
Please add some tests here - https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-py-common/tests/interface_test.py |
This pull request introduces 1 alert when merging 2e7b38e50f7a7dfd467e12ba32d8f82f506d9fb3 into bb798a3 - view on LGTM.com new alerts:
|
else: | ||
return str(intf) | ||
|
||
def intf_get_longname(intf): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please follow a similar naming convention in the file.. suggest rename to get_subintf_name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is being used to get longname for both subinterface and parent of subinterface, I have updated to use get_intf_longname as part of next commit.
/AzurePipelines run Azure.sonic-buildimage, |
Azure Pipelines successfully started running 1 pipeline(s). |
@preetham-singh , could you please rebase? |
parent interface for the routed subinterface
This pull request introduces 1 alert when merging 7b2237e9385c6ce3f76caf0f6bfb3c692c8fa7cf into 5f235a9 - view on LGTM.com new alerts:
|
looks like incorrect rebase. Could you please fix? |
This pull request introduces 1 alert when merging 0626347bc424e9dbde6f87484a293023e4c4821d into 11a93d2 - view on LGTM.com new alerts:
|
0626347
to
3fbf12f
Compare
This pull request introduces 1 alert when merging 56e8236 into b3ccef9 - view on LGTM.com new alerts:
|
Why I did it
Routed subinterfae enhancements HLD #833
Adding python API support to get routed subinterface long name to get correct parent interface for the routed subinterface.
How I did it
Adding python API.
How to verify it
Configuring routed subinterface on Physical & port channel subinterfaces.
Which release branch to backport (provide reason below if selected)
Description for the changelog
A picture of a cute animal (not mandatory but encouraged)