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

[multi-asic][minigraph]: Add changes to support minigraph generation for multi-asic #3024

Merged

Conversation

SuvarnaMeenakshi
Copy link
Contributor

@SuvarnaMeenakshi SuvarnaMeenakshi commented Feb 23, 2021

  • Add new topo files to define internal asic topology
    for Virtual Switch hwsku msft_multi_asic_vs and
    msft_four_asic_vs.
  • Add changes in topo_facts to parse the new ASIC topo files.
  • Add changes in port_alias to generate list of asic names for
    front-end interfaces.
  • Add additional dictionary in port_alias to generate
    interfaces list for all ASICs.

Signed-off-by: SuvarnaMeenakshi [email protected]

Description of PR

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

What is the motivation for this PR?

To support minigraph generation for multi-asic platforms.

How did you do it?

Multi-asic minigraph will include internal asic topology and asic metadata.

  • To provide the internal asic information, a new topo file is added. The new topo file will hwsku specific, and will contain asic topology similar to the other topo files. A single topo file will be used to provide topology of all asics for that hwsku.
    In the pull request, topo files for 2 Virtual switch hwsku's is added: - topo_msft_multi_asic_vs.yml and topo_msft_four_asic_vs.yml.
  • Made changes to topo_facts.py to parse the newly added topo files similar to the parsing logic of existing topo files.
  • Made changes to port_alias.py to generate a list of front-end ASIC interface names and list of interfaces of each ASIC which will be used in minigraph templates.
  • Made changes to config config_sonic_basedon_testbed to pass hwsku to topo_facts.py. Based on the hwsku, topo_facts will look for hwsku specific topo file.

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

generation for multi-asic VS.
- Add new topo files to define internal asic topology
for Virtual Switch hwsku msft_multi_asic_vs and
msft_four_asic_vs.
- Add changes in topo_facts to parse the new ASIC topo files.
- Add changes in port_alias to generate list of asic names for
front-end interfaces.
- Add additional dictionary in port_alias to generate
interfaces list for all ASICs.

Signed-off-by: SuvarnaMeenakshi <[email protected]>
@lgtm-com
Copy link

lgtm-com bot commented Feb 23, 2021

This pull request introduces 2 alerts when merging b0c202b into b0890db - view on LGTM.com

new alerts:

  • 2 for Unused local variable

@yxieca
Copy link
Collaborator

yxieca commented Feb 26, 2021

@SuvarnaMeenakshi please address LGTM issues.

Signed-off-by: SuvarnaMeenakshi <[email protected]>
@SuvarnaMeenakshi
Copy link
Contributor Author

@SuvarnaMeenakshi please address LGTM issues.

Fixed LGTM errors.

@@ -94,6 +94,11 @@ def get_portmap(self, asic_id=None):
portmap = {}
aliasmap = {}
portspeed = {}
# Front end interface asic names
asicifnames = []
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to asic_ifnames

Copy link
Contributor Author

@SuvarnaMeenakshi SuvarnaMeenakshi Feb 27, 2021

Choose a reason for hiding this comment

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

Renamed to front_panel_asic_ifnames. This will be the list of asic names of front panel interfaces.

# Front end interface asic names
asicifnames = []
# All asic names
asicnames = []
Copy link
Contributor

Choose a reason for hiding this comment

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

rename to asic_naames ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to asic_if_names. This will be a map of ASIC: [ list of all interface of that asic ]

asicifnames.append(asicifname)
if (asic_name_index != -1) and (len(mapping) > asic_name_index):
asicname = mapping[asic_name_index]
asicnames.append(asicname)

Copy link
Contributor

Choose a reason for hiding this comment

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

the asicnames list will have both internal and external interfaces ?
Can you elaborate what is the difference between asicnames and asicifnames ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to front_panel_asic_ifnames and asic_if_names to make it clear, updated comment in port_alias.py

ASIC0:
topology:
ASICs:
ASIC2:
Copy link
Contributor

Choose a reason for hiding this comment

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

rename this to NEIGH_ASIC or PEER_ASIC? it is clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified to NEIGH_ASIC

- Change variable name of list of front-panel asic interface names.
- Change variable name of dict of asic to list of all asic ifnames.
- Change ASICs key word to NEIGH_ASIC in topo file for multi-asic
hwsku.

Signed-off-by: SuvarnaMeenakshi <[email protected]>
of current asic instead of interface index.

Signed-off-by: SuvarnaMeenakshi <[email protected]>
@SuvarnaMeenakshi SuvarnaMeenakshi requested a review from wangxin March 3, 2021 19:25
Copy link
Contributor

@arlakshm arlakshm left a comment

Choose a reason for hiding this comment

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

lgtm

@SuvarnaMeenakshi SuvarnaMeenakshi merged commit 2715ded into sonic-net:master Mar 9, 2021
SuvarnaMeenakshi added a commit that referenced this pull request Mar 10, 2021
…atform (#3025)

What is the motivation for this PR?
Minigraph template changes to support minigraph generation for multi-asic platform.

How did you do it?
Pre-requisite: #3024
Add changes to minigraph templates to use the new data structure asic_topo_config and include asic topology.

How did you verify/test it?
With the changes in PR#3024:
Bring up four-asic VS testbed using the changes in: #2858
testbed-cli.sh -t vtestbed.csv -m veos_vtb -k ceos add-topo vms-kvm-four-asic-t1-lag password.txt
Deploy minigraph using:
./testbed-cli.sh -t vtestbed.csv -m veos_vtb deploy-mg vms-kvm-four-asic-t1-lag lab password.txt
With this, minigraph should be generated and deployed on the multi-asic VS DUT.
Check all interfaces status and BGP status.
wangxin pushed a commit that referenced this pull request Mar 11, 2021
What is the motivation for this PR?
#3024 introduced a new function with an argument called 'type'.
This will override existing python 'type' function. To avoid this, modify the argument name to 'neigh_type'
which can be 'VMs' or 'NEIGH_ASIC' based on the topo file being parsed.

How did you do it?
Modify the argument name to 'neigh_type'.

How did you verify/test it?
Bring up single-asic and multi-asic VS testbed with this change.

Signed-off-by: Suvarna Meenakshi <[email protected]>
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.

3 participants