-
Notifications
You must be signed in to change notification settings - Fork 750
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
Support for minigraph generation for packet based chassis. #4479
Conversation
Signed-off-by: Abhishek Dosi <[email protected]>
2. Added Support Ipv6 Peering on Loopback4096 for voq also 3. Updated asic topology yml files to be offset of slot 4. Made slot_num to take string slot<number> instead of number 5. Consolidated template_dpg_voq_asic.j2 into dpg_asic.j2 6. Remove Loopback4096 from asic topology and parse as dut invertory for multi-asic 7. Updated topo_facts parsing for asic topology Signed-off-by: Abhishek Dosi <[email protected]>
cc @sanmalho-git please review. |
sonic_msft_sup: | ||
vars: | ||
HwSku: msft-RP-O | ||
slot_num: slot0 |
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.
There are some dependencies on slot_num that I believe are expecting this host variable to be int.
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.
Have fixed in latest commit to strip "slot" from the string.
portconfig = os.path.join(FILE_PATH, platform, self.hwsku, str(asic_id), PORTMAP_FILE) | ||
else: | ||
portconfig = os.path.join(FILE_PATH, platform, self.hwsku, str(slotid), str(asic_id), PORTMAP_FILE) |
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.
Is the port_config.ini file different for a card with the same hwsku being in a different slot?
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.
yes, Backend connectivity can be different based on slot the card is inserted.
<Multihop>1</Multihop> | ||
<HoldTime>0</HoldTime> | ||
<KeepAliveTime>0</KeepAliveTime> | ||
<ChassisInternal>{{ switch_type }}</ChassisInternal> |
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.
As mentioned in the review in chassis subgroup - we are currently not creating v6 iBGP neighbors across the asics - only v4. The IPv6 routes are carried over the v4 iBGP peer itself.
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.
thanks. for now we want to do Ipv6 peering.
Signed-off-by: Abhishek Dosi <[email protected]>
Signed-off-by: Abhishek Dosi <[email protected]>
@arlakshm can you review/approve this. |
What is the motivation for this PR? The minigraph template changes for chassis PR: #4479 removed Bandwidth element with the assumption that interface speed should be configured from port_config. But that will not be the case for multi-asic platform. Due to this change, internal interfaces in multi-asic were coming up without speed configured. How did you do it? Add default interface speed in minigraph png template. How did you verify/test it? Bring up mutli-asic VS and verify that internal interfaces are coming up with default speed and internal BGP sessions are coming up.
@abdosi - sorry, but am just getting to test this PR out on Nokia's VoQ chassis. I see two issues in the minigraph generated with this PR:
|
@sanmalho-git thanks for validating. we will check and fix this by this week. |
My approach to fixing this is to
I have prototyped with this solution, and works fine for both single and multi-asic linecards in a VoQ chassis. 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:
Please let me know if you are OK with the approach |
Thanks @sanmalho-git this looks fine to me . I assume this is for Problem#2. What about Problem#1 ? |
For problem#1, there are some minor fixes in minigraph_cpg.j2. Also, if I can get the command to generate minigraph of the packet-based chassis topology, that would help me test the solution out for packet based chassis as well. |
thanks @sanmalho-git Can you create PR for now ? I will take your change and verify. Not sure current public code will work to generate chassis-packet template. Will create seprate to make it work if not working. |
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:
@abdosi @SuvarnaMeenakshi - PR #4699 has been raised to address the issues with minigraph generation for linecards in a VoQ chasiss. Please review and also add the 'chassis' label to the PR. |
PR sonic-net#4479 introduced "slot" to hwsku topology definition. After this PR is merged, parsing topology of Nexus 3164 failed because its topology definition file in sonic-mgmt-int repo is not updated accordingly. This change added "slot to topo_Nexus-3164.yml to conform with the new "slot" requirement. Signed-off-by: Xin Wang <[email protected]>
Add slot to topology definition of Nexus 3164 PR sonic-net#4479 introduced "slot" to hwsku topology definition. After this PR is merged, parsing topology of Nexus 3164 failed because its topology definition file in sonic-mgmt-int repo is not updated accordingly. This change added "slot to topo_Nexus-3164.yml to conform with the new "slot" requirement. Signed-off-by: Xin Wang <[email protected]>
What I did:
Below changes goes with the sonic-buildimage PR: sonic-net/sonic-buildimage#8966
How I Verify