Fixes to minigraph generation for linecards in a VoQ Chassis#4699
Merged
arlakshm merged 3 commits intosonic-net:masterfrom Dec 8, 2021
Merged
Fixes to minigraph generation for linecards in a VoQ Chassis#4699arlakshm merged 3 commits intosonic-net:masterfrom
arlakshm merged 3 commits intosonic-net:masterfrom
Conversation
PR# 4479 added support for minigraph generation for packet based chassis. However, this broke the generated minigraph for linecards in a VoQ chassis. There were two issues in the minigraph generated with PR sonic-net#4479: - In Cpg - Missing BGPRouterDeclaration section for remote asics - <remote_linecard>-ASIC<#> - For multi-asic linecard, missing BGPRouterDeclaraion section for my asics - ASIC<#> - In Dpg, we are missing the DeviceDataPlaneInfo for my asics in a multi-asic linecard - This is generated in template_dpg_asic, which is dependent on asic_topo_config. For voq_chassis, asic_topo_config is empty dictionary - as the iBGP connections depend on the what linecards are present in the chassis, and also what asics are on each linecard. To fix the above issues: - create an almost empty asic topology file for our multi-asic linecard with slot0. - In config_sonic_basedon_testbed.yml, when dealing with voq switch_type, before running through the Jinja2 templates, change slot0 in the asic_topo_config to the slot_num that is defined in the inventory. Below is the asic topology for our multi-asic linecard Nokia_IXR7250e_36x400G card (in vars/topo_Nokia_IXR7250e_36x400G.yml) that I prototyped with: slot0: ASIC0: topology: NEIGH_ASIC: configuration_properties: common: asic_type: FrontEnd configuration: ASIC1: topology: NEIGH_ASIC: configuration_properties: common: asic_type: FrontEnd configuration:
In minigraph_dpg_asic.j2 we are making port channel names be zero padded with upto 2 zeros with 'zfill(2)' - example: PortChannel02 However, in minigraph_dpg.j2, the padded is upto 4 zeros with 'zfill(4)' - example: PortChannel0002 This causes mismatch in the minigraph for the port channels in the host DeviceDataPlaneInfo vs asic DeviceDataPlaneInfo. Fix is to use 'zfill(2)' in minigraph_dpg.j2 as well, instead of 'zfill(4)'.
abdosi
reviewed
Nov 18, 2021
| {% set asic_name = "ASIC" + asic_id|string %} | ||
| <a:BGPRouterDeclaration> | ||
| <a:ASN>{{ vm_topo_config['dut_asn'] }}</a:ASN> | ||
| <a:Hostname>{{ asic_name }}</a:Hostname> |
Contributor
Author
There was a problem hiding this comment.
Now that we asic_topo_config defined for multi-asic line cards, this is covered by iterating over asic_topo_config at line 203.
abdosi
reviewed
Nov 18, 2021
ansible/templates/minigraph_dpg.j2
Outdated
| <Name i:Name="true"/> | ||
| {% if 'port-channel' in vm_topo_config['vm'][vms[index]]['ip_intf'][dut_index|int]|lower %} | ||
| <AttachTo>PortChannel{{ ((index+1) |string).zfill(4) }}</AttachTo> | ||
| <AttachTo>PortChannel{{ ((index+1) |string).zfill(2) }}</AttachTo> |
Contributor
There was a problem hiding this comment.
what is need of this change ?
Contributor
Author
There was a problem hiding this comment.
Two reasons:
- For multi-asic, the asic DeviceDataPlanInfo defines port channels with 1 leading 0 (like PortChannel02), while the host level DeviceDataPlanInfo defines the port channels with 3 leading 0's (like PortChannel0002). So, this mismatch causes some tests to fail. Example:
- iface_namingmode/test_iface_namingmode.py::TestShowInterfaces::test_show_interfaces_portchannel. In this test we get minigraph_facts for duthost with would be padded with 3 0's and check against 'show interfaces portchannel' which from the asic config_db.json generated from minigraph would be padded to only 1 0.
- If we have a combination of single asic and multi-asic linecards, then in show commands on single asic we would have port channels padded with 3 0's, while on multi-asic they would be padded with only 1 0. So, did the change to have consistency across single and multi-asic line cards
Contributor
There was a problem hiding this comment.
@sanmalho-git i think you might not have this change #4576.
After above PR External Portchannel will always have 4 digit. Can you revert this particular change and then we can merge the PR. I have verified other change is good.
Contributor
Author
There was a problem hiding this comment.
Yes - I missed that PR in my testing. I have reverted the commit that changed the PortChannels to have 2 digits instead of 4.
This reverts commit e27fc23.
abdosi
approved these changes
Nov 24, 2021
arlakshm
approved these changes
Dec 7, 2021
AntonHryshchuk
pushed a commit
to AntonHryshchuk/sonic-mgmt
that referenced
this pull request
Jan 4, 2022
sonic-net#4699) What is the motivation for this PR? PR# 4479 added support for minigraph generation for packet based chassis. However, this broke the generated minigraph for linecards in a VoQ chassis. There were two issues in the minigraph generated with this PR: In Cpg Missing BGPRouterDeclaration section for remote asics - <remote_linecard>-ASIC<#> For multi-asic linecard, missing BGPRouterDeclaraion section for my asics - ASIC<#> In Dpg, we are missing the DeviceDataPlaneInfo for my asics in a multi-asic linecard This is generated in template_dpg_asic, which is dependent on asic_topo_config. For voq_chassis, asic_topo_config is empty dictionary - as the iBGP connections depend on the what linecards are present in the chassis, and also what asics are on each linecard. How did you do it? To fix the above issues: create an almost empty asic topology file for our multi-asic linecard with slot0. In config_sonic_basedon_testbed.yml, when dealing with voq switch_type, before running through the Jinja2 templates, change slot0 in the asic_topo_config to the slot_num that is defined in the inventory.
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
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.
Description of PR
Summary:
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
PR# 4479 added support for minigraph generation for packet based chassis.
However, this broke the generated minigraph for linecards in a VoQ chassis.
There were two issues in the minigraph generated with this PR:
For voq_chassis, asic_topo_config is empty dictionary - as the iBGP connections depend on the what linecards are present in the chassis, and also what asics are on each linecard.
How did you do it?
To fix the above issues:
change slot0 in the asic_topo_config to the slot_num that is defined in the inventory.
Below is the asic topology for a multi-asic linecard Nokia_IXR7250e_36x400G card (in vars/topo_Nokia_IXR7250e_36x400G.yml) that is added:
How did you verify/test it?
Generated minigraph for single and multi-asic linecards in a VoQ chassis and validated with:
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation