minigraph: Add the ability to set a per-port speed in the minigraph #3527
Conversation
This file contains 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
In the case of devices with mixed port speeds, the speed for each port has to be individually specified in the minigraph, and a single speed for the entire device cannot be used. Instead, read the connection graph, which contains the speed for each individual link, and use the speed specified there for each port. If a port/link isn't specified in the connection graph, then the speed for each port in port_config.ini is still used. If no speed is specified there, then the speed in the inventory yaml file is used. Signed-off-by: Saikrishna Arcot <saiarcot895@gmail.com>
neethajohn
approved these changes
May 25, 2021
|
This pull request introduces 1 alert when merging 3c13ea7 into 1a28408 - view on LGTM.com new alerts:
|
|
@saiarcot895 please address the LGTM warning |
yxieca
reviewed
May 25, 2021
yxieca
requested changes
May 25, 2021
bingwang-ms
reviewed
May 25, 2021
Instead of getting and using the connection graph data in port_alias.py, use it instead in the template file, and add a new ansible task to load in the connection graph data. This keeps port_alias.py strictly for loading in the port_config.ini data, and instead moves the speed selection logic into the template file. Signed-off-by: Saikrishna Arcot <saiarcot895@gmail.com>
yxieca
approved these changes
May 26, 2021
neethajohn
approved these changes
May 26, 2021
bingwang-ms
approved these changes
May 27, 2021
saiarcot895
added a commit
to saiarcot895/sonic-mgmt
that referenced
this pull request
Jun 1, 2021
…onic-net#3527) * minigraph: Add the ability to set a per-port speed in the minigraph In the case of devices with mixed port speeds, the speed for each port has to be individually specified in the minigraph, and a single speed for the entire device cannot be used. Instead, read the connection graph, which contains the speed for each individual link, and use the speed specified there for each port. The speed specified in the connection graph is used only if port_config.ini doesn't have a speed specified there. If there is a speed specified there, then that is used. If no speed is specified in port_config.ini, and there's no speed specified for the port in the connection graph, then the speed in the inventory yaml file is used. Signed-off-by: Saikrishna Arcot <saiarcot895@gmail.com> (cherry picked from commit cef1f77)
saiarcot895
added a commit
that referenced
this pull request
Jun 2, 2021
* minigraph: Add the ability to set a per-port speed in the minigraph (#3527) * minigraph: Add the ability to set a per-port speed in the minigraph (cherry picked from commit cef1f77) * minigraph: Fix with_dict syntax in the playbook That entry needs to be specified as referring to a variable. Signed-off-by: Saikrishna Arcot <saiarcot895@gmail.com> * [topo] Add test topology for 7050QX-32S-S4Q31 (#3568) (cherry picked from commit 6d1720b)
vmittal-msft
pushed a commit
to vmittal-msft/sonic-mgmt
that referenced
this pull request
Sep 28, 2021
…onic-net#3527) * minigraph: Add the ability to set a per-port speed in the minigraph In the case of devices with mixed port speeds, the speed for each port has to be individually specified in the minigraph, and a single speed for the entire device cannot be used. Instead, read the connection graph, which contains the speed for each individual link, and use the speed specified there for each port. The speed specified in the connection graph is used only if port_config.ini doesn't have a speed specified there. If there is a speed specified there, then that is used. If no speed is specified in port_config.ini, and there's no speed specified for the port in the connection graph, then the speed in the inventory yaml file is used. Signed-off-by: Saikrishna Arcot <saiarcot895@gmail.com>
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.
Signed-off-by: Saikrishna Arcot saiarcot895@gmail.com
Description of PR
In the case of devices with mixed port speeds, the speed for each port
has to be individually specified in the minigraph, and a single speed
for the entire device cannot be used. Instead, read the connection
graph, which contains the speed for each individual link, and use the
speed specified there for each port.
The speed specified in the connection graph is used only if
port_config.ini doesn't have a speed specified there. If there is a speed
specified there, then that is used.
If no speed is specified in port_config.ini, and there's no speed specified
for the port in the connection graph, then the speed in the inventory
yaml file is used.
Type of change
Bug fix
Testbed and Framework(new/improvement)
Test case(new/improvement)
Testing done
Generated minigraph with and without this change for most of our internal testbeds (str and str2), as well as
vms-s6000-t0. Verified that all of the generated minigraphs remained the same with this change, and that there are no minigraphs that are not getting successfully generated that were getting generated before this change.Also verified that with a non-default speed in the connection graph data (i.e. a speed for one port that was different than the speed for the other ports), that speed correctly got populated in the minigraph.