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

Routed subinterface enhancements #1821

Merged
merged 6 commits into from
Dec 3, 2021
Merged

Conversation

preetham-singh
Copy link
Contributor

@preetham-singh preetham-singh commented Sep 15, 2021

What I did

Routed subinterfae enhancements HLD #833
Added support for configuring routed subinterface in short name and long name format
Updated show command to display user configured subinterfaces in correct format.

Config commands:
config subinterface (add | del) <subinterface_name> [vlan <1-4094>]

Ex:
config subinterface add Ethernet0.100
config subinterface add Eth64.100 100

Show commands:
show subinterfaces status

How I did it

How to verify it

Configure routed subinterface on physical & port channel interfaces.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

Dependency

This PR is dependent on sonic-buildimage 8761

format
Update "show subniterface status" to reflect subinterface in user
configured long name and short name format.
@lgtm-com
Copy link

lgtm-com bot commented Sep 15, 2021

This pull request introduces 3 alerts when merging 88bae6d into f76f672 - view on LGTM.com

new alerts:

  • 1 for Unnecessary pass
  • 1 for Unused local variable
  • 1 for Variable defined multiple times

@prsunny
Copy link
Contributor

prsunny commented Sep 18, 2021

Can you add unit test?

@hasan-brcm
Copy link

/azpw run

@zhangyanzhao
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).


full_intf_table_name = "INTF_TABLE" + ":" + sub_intf_name

if status_type == "vlan":
vlan_id = appl_db.get(appl_db.APPL_DB, full_intf_table_name, status_type)
Copy link
Contributor

Choose a reason for hiding this comment

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

Where can we find DB migrator change to populate vlan_id field from sub-interface name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As per updated design, intfmgrd supports both short name and long name format subinterfaces.
In long name format, vlan_id is derived from subinterface index by intfmgrd.
In short name format, vlan_id is mandatory parameter to be configured by user(as part of click cli).
With this design we do not need db_migrator changes.

sub_intf_sep_idx = subintf.find(VLAN_SUB_INTERFACE_SEPARATOR)
if sub_intf_sep_idx == -1:
continue
if parent_intf == subintf[:sub_intf_sep_idx]:
Copy link
Contributor

@venkatmahalingam venkatmahalingam Nov 2, 2021

Choose a reason for hiding this comment

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

Can we please put these new Click commands in this PR description for easy reference? also, hope you are planning to update the Click config guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@prsunny
Copy link
Contributor

prsunny commented Nov 10, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Contributor

prsunny commented Nov 12, 2021

@preetham-singh , can you check if the failure is related to the PR?

@hasan-brcm
Copy link

@preetham-singh , can you check if the failure is related to the PR?

Hi @prsunny, this PR is dependent on PR8761.

@prsunny
Copy link
Contributor

prsunny commented Nov 29, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@preetham-singh
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 1821 in repo Azure/sonic-utilities

@preetham-singh
Copy link
Contributor Author

/azpw run

@mssonicbld
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Contributor

prsunny commented Dec 1, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Contributor

prsunny commented Dec 1, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny
Copy link
Contributor

prsunny commented Dec 2, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@prsunny prsunny merged commit 290ff5f into sonic-net:master Dec 3, 2021
abdosi pushed a commit that referenced this pull request Dec 8, 2021
*CLI based on Routed subinterface enhancements HLD #833
*Added support for configuring routed subinterface in short name and long name format
*Updated show command to display user configured subinterfaces in correct format.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants