Support for gen-mg for multi-asic linecards in VoQ chassis #3746
Conversation
| {% if num_asics == 1 %} | ||
| {% set start_rtr = inventory_hostname %} | ||
| {% else %} | ||
| {% set start_rtr = "ASIC" + asic_id|string %} |
There was a problem hiding this comment.
start_rtr also requires the linecard/hostname prefix before the "ASIC"
There was a problem hiding this comment.
No, because is BGPRouterDeclaration for each asic (lines 221-226 in the file, we are defining the hostname as 'ASIC<#>'. This has to match with the hostname defined for our asics in the DPG/PNG sections
There was a problem hiding this comment.
is the ASIC<#> unique for the whole chassis ?
There was a problem hiding this comment.
I think this section will have the hostname of the device(linecard).
There was a problem hiding this comment.
I think this section will have the hostname of the device(linecard).
Sample multi asic pizza box https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-config-engine/tests/multi_npu_data/sample-minigraph.xml\
There was a problem hiding this comment.
ASIC<#> is not unique for the whole chassis, but within a linecard. In this section, we are creating iBGP peers for a linecard, so for a multi-asic linecard, the starting router includes only the ASIC<#>. The end_rtr includes the other linecards hostname and ASIC<#> which is unique for the whole chassis.
This is similar to iBGP defined at L44 in the sample multi-asic xml file.
| {% if switch_type is defined and switch_type == 'voq' %} | ||
| {% for all_idx in range(all_inbands|length) %} | ||
| {% if voq_inband_ip != all_inbands[all_idx] %} | ||
| {% if num_asics == 1 and switch_type is defined and switch_type == 'voq' %} |
There was a problem hiding this comment.
why do we need num_asics == 1 and handle >1 conditions separately ?
There was a problem hiding this comment.
Because we are in BgpRouterDeclaration of the whole linecard (inventory_hostname). If I am single asic linecard, then I need to add all my voq peers to the other linecards. For multiasic, the voq peers go in the asisc BgpRouterDeclaration
| @@ -197,6 +204,52 @@ | |||
| <a:RouteMaps/> | |||
| </a:BGPRouterDeclaration> | |||
| {% endfor %} | |||
| {% if switch_type is defined and switch_type == 'voq' %} | |||
| {% for a_linecard in all_inbands %} | |||
| {% if a_linecard != inventory_hostname %} | |||
There was a problem hiding this comment.
how about iBGP peers within a linecard that has multiple asics ?
There was a problem hiding this comment.
all_inbands now is a dictionary with key being each linecard and value being a list of inband ip's. For 2 linecards (board3 and board4) with 2 asics, it would look like:
"all_inbands": {
"board3": [
"3.3.3.4/32",
"3.3.3.5/32"
],
"board4": [
"3.3.3.10/32",
"3.3.3.11/32"
]
}
And the voq_peers generated above has key for all the inbands across all linecards and its correspoding router name. So, for the above example when running through board3, it would be:
{
'3.3.3.4' : 'ASIC0',
'3.3.3.5': 'ASIC1',
'3.3.3.10': 'board4-ASIC0'
'3.3.3.11': 'board4-ASIC1'
}
There was a problem hiding this comment.
a_linecard != inventory_hostname check will not add the iBGP between 3.3.3.4 and 3.3.3.5 when running through board3 right ?
There was a problem hiding this comment.
Here we are defining BgpRouterDeclaration for all other asics in all other linecards - not iBGP sessions. That is taken care of above in lines 82 - 115.
|
@arlakshm could you please review this before every other chassis PR? In case we decide to make any changes to mg, the in-fligt PRs need changes. So could we please fix this soon? Thanks. |
| {% if num_asics == 1 %} | ||
| {% set start_rtr = inventory_hostname %} | ||
| {% else %} | ||
| {% set start_rtr = "ASIC" + asic_id|string %} |
There was a problem hiding this comment.
is the ASIC<#> unique for the whole chassis ?
| {% if num_asics == 1 %} | ||
| {% set start_rtr = inventory_hostname %} | ||
| {% else %} | ||
| {% set start_rtr = "ASIC" + asic_id|string %} |
There was a problem hiding this comment.
I think this section will have the hostname of the device(linecard).
| {% if num_asics == 1 %} | ||
| {% set start_rtr = inventory_hostname %} | ||
| {% else %} | ||
| {% set start_rtr = "ASIC" + asic_id|string %} |
There was a problem hiding this comment.
I think this section will have the hostname of the device(linecard).
Sample multi asic pizza box https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-config-engine/tests/multi_npu_data/sample-minigraph.xml\
|
@abdosi, can you take a look as well |
For multi-asic linecards the following fields in the inventory (from TestbedProcessing.py) are converted to lists instead of a single value: - switchids - instead of start_switchid - voq_inband_ip - voq_inband_ipv6 - voq_inband_intf We also added the following fields: - loopback4096_ip - loopback4096_ipv6 The minigraph templates were modified to use generate asic level metadata for all asics in a linecard. We added minigraph_dpg_voq_asic.j2 template to generate asic DPG definitions for multi-asic linecards.
- Incorporated review comments in TestbedProcessing.py:
- Added num_cores_per_asic which defaults to 2. And use that in calculating the switchids per asic.
It is assumed that all linecards have the same number of core per asic
- port_alias.py:
- Take care when switch_ids are not passed for backward compatability.
port_alias was being called from iface_namingmode tests, and that was not passing switchids and thus failing.
This also addresses the review comment
- fabric_asic:
- fabric_info.py:
- Needed to use unicode when creating ipaddress objects
- Add asic_id into the dictionary to be used later in generating in minigraph
- minigraph_dpq_voq_asic.j2:
- Add fabric info to the minigraph only if switch_type is 'fabric'
…gle asic linecard It was recommended in the sonic chassis subgroup to follow the same methodology of using Loopback4096 addresses as router-id for iBGP connections from a single asic linecard to other asics, similar to that for a multi-asic linecard.
… asic ports Had not done this before as we would otherwise enable all front end ports, but in our linecards not all ports are connected. Modified to add DeviceInterfaceLink only if ports are defined in the connection graph.
sanmalho-git
force-pushed
the
gen-mg
branch
from
006a84b
to
d2f5ca9
Compare
…chassis (sonic-net#3746)" This reverts commit 9b9fe9b.
…onic-net#3746) What is the motivation for this PR? Support for generating minigraph for multi-asic linecards in a VoQ chassis is missing. Also, for multi-asic linecards we needed to add support for Loopback4096 interfaces, such that they can be used as the router-id for the iBGP connectivity. How did you do it? Support for minigraph generation for single asic linecards and supervisor card was done as part of PR sonic-net#3245. This introduced several fields into the ansible inventory required for VoQ chassis linecard. However, for multi-asic linecards these fields in the inventory (from TestbedProcessing.py) are converted to lists instead of a single value. The fields changed are: switchids - instead of start_switchid voq_inband_ip voq_inband_ipv6 voq_inband_intf We also added the following fields to support Loopback4096 interfaces on the asics of a multi-asic linecard. loopback4096_ip loopback4096_ipv6 The minigraph templates were modified to use generate asic level metadata for all asics in a linecard. We added minigraph_dpg_voq_asic.j2 template to generate asic DPG definitions for multi-asic linecards. Also modified test_bgp_facts to include BGP_VOQ_CHASSIS_NEIGHBOR's in its checking of all bgp neighbors. How did you verify/test it? Generated minigraphs for single asic and multi-asic linecards and supervisor cards and validated configs generated using 'config load_minigraph' command in SONiC.
… in VoQ chassis (sonic-net#3746)" (sonic-net#4351)" This reverts commit aa9c059.
… in VoQ chassis (sonic-net#3746)" (sonic-net#4351)" This reverts commit aa9c059.
* Revert "Revert "[chassis] Support for gen-mg for multi-asic linecards in VoQ chassis (#3746)" (#4351)" This reverts commit aa9c059. * Fix for issue #4343 when running gen-mg for multi-asic kvm - Use empty list as the default for switchids in the call to port_alias. - In port_alias ansible library, check the length of switchids before using it. - In minigraph_png, - For supporting not all ports connected to fanout, in minigraph_png.j2, we added DeviceInterfaceLink for only ports that are defined in the device_conn (from connection graph). But, for masic KVM, device_conn is not defined, and thus we were not adding any port. The fix is that if the inventory_hostname is not present in the device_conn, then add DeviceInterfaceLink for all front_panel_asic_ifnames - 'Bandwidth' for each DeviceInterfaceLink was hard-coded to 40G, we added code to get it from port_speed which also gets populated from the connection graph. However, port_speed is empty for KVM, and thus generation was failing. Fix is to check if the port is present in port_speed, and if so, use the speed defined in port_speed. Else, use default 40G.
* Revert "Revert "[chassis] Support for gen-mg for multi-asic linecards in VoQ chassis (sonic-net#3746)" (sonic-net#4351)" This reverts commit aa9c059. * Fix for issue sonic-net#4343 when running gen-mg for multi-asic kvm - Use empty list as the default for switchids in the call to port_alias. - In port_alias ansible library, check the length of switchids before using it. - In minigraph_png, - For supporting not all ports connected to fanout, in minigraph_png.j2, we added DeviceInterfaceLink for only ports that are defined in the device_conn (from connection graph). But, for masic KVM, device_conn is not defined, and thus we were not adding any port. The fix is that if the inventory_hostname is not present in the device_conn, then add DeviceInterfaceLink for all front_panel_asic_ifnames - 'Bandwidth' for each DeviceInterfaceLink was hard-coded to 40G, we added code to get it from port_speed which also gets populated from the connection graph. However, port_speed is empty for KVM, and thus generation was failing. Fix is to check if the port is present in port_speed, and if so, use the speed defined in port_speed. Else, use default 40G.
…o-git:gen-mg) For multi-asic linecards the following fields in the inventory (from TestbedProcessing.py) are converted to lists instead of a single value: - switchids - instead of start_switchid - voq_inband_ip - voq_inband_ipv6 - voq_inband_intf We also added the following fields: - loopback4096_ip - loopback4096_ipv6 The minigraph templates were modified to use generate asic level metadata for all asics in a linecard. We added minigraph_dpg_voq_asic.j2 template to generate asic DPG definitions for multi-asic linecards.
Description of PR
Summary: Support for gen-mg for multi-asic linecards in VoQ chassis
Fixes # (issue)
Type of change
Back port request
Approach
What is the motivation for this PR?
Support for generating minigraph for multi-asic linecards in a VoQ chassis is missing. Also, for multi-asic linecards we needed to add support for Loopback4096 interfaces, such that they can be used as the router-id for the iBGP connectivity.
How did you do it?
Support for minigraph generation for single asic linecards and supervisor card was done as part of PR #3245. This introduced several fields into the ansible inventory required for VoQ chassis linecard. However, for multi-asic linecards these fields in the inventory (from TestbedProcessing.py) are converted to lists instead of a single value. The fields changed are:
We also added the following fields to support Loopback4096 interfaces on the asics of a multi-asic linecard.
The minigraph templates were modified to use generate asic level metadata for all asics in a linecard.
We added minigraph_dpg_voq_asic.j2 template to generate asic DPG definitions for multi-asic linecards.
Also modified test_bgp_facts to include BGP_VOQ_CHASSIS_NEIGHBOR's in its checking of all bgp neighbors.
How did you verify/test it?
Generated minigraphs for single asic and multi-asic linecards and supervisor cards and validated configs generated using 'config load_minigraph' command in SONiC.
Sample gen-mg command
Any platform specific information?
Supported testbed topology if it's a new test case?
Documentation