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.

Sign up for GitHub

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

Jump to bottom

+ Modified buffer config template to include internal ASIC with 5m ca… #4959

Merged
merged 4 commits into from
Jul 27, 2020
Merged

+ Modified buffer config template to include internal ASIC with 5m ca… #4959

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
04734ba
+ Modified buffer config template to include internal ASIC with 5m ca…
smaheshm Jul 13, 2020
9dd6903
error in 'if' statement, fixed it
smaheshm Jul 14, 2020
383de5f
+ added unit test to verify cable length calculation for multi ASIC p…
smaheshm Jul 21, 2020
ed754d8
+ added comment for the change in 'sort_port_by_index' API
smaheshm Jul 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions files/build_templates/buffers_config.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ def
{%- else %}
{%- set ports2cable = {
'torrouter_server' : '5m',
'internal' : '5m',
'leafrouter_torrouter' : '40m',
'spinerouter_leafrouter' : '300m'
}
Expand All @@ -47,10 +48,14 @@ def
{%- if DEVICE_NEIGHBOR_METADATA is defined and DEVICE_NEIGHBOR_METADATA[DEVICE_NEIGHBOR[local_port].name] %}
{%- set neighbor = DEVICE_NEIGHBOR_METADATA[DEVICE_NEIGHBOR[local_port].name] %}
{%- set neighbor_role = neighbor.type %}
{%- set roles1 = switch_role + '_' + neighbor_role %}
{%- set roles2 = neighbor_role + '_' + switch_role %}
{%- set roles1 = roles1 | lower %}
{%- set roles2 = roles2 | lower %}
{%- if 'asic' == neighbor_role | lower %}
{%- set roles1 = 'internal' %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we have unit test to cover this change in sonic-config-engine?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added unit test.

{%- else %}
{%- set roles1 = switch_role + '_' + neighbor_role %}
{%- set roles2 = neighbor_role + '_' + switch_role %}
{%- set roles1 = roles1 | lower %}
{%- set roles2 = roles2 | lower %}
{%- endif %}
{%- if roles1 in ports2cable %}
{%- if cable_len.append(ports2cable[roles1]) %}{% endif %}
{%- elif roles2 in ports2cable %}
Expand Down
6 changes: 5 additions & 1 deletion src/sonic-config-engine/sonic-cfggen
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ def sort_by_port_index(value):
if not value:
return
if isinstance(value, list):
value.sort(key = lambda k: int(k[8:]))
# In multi-ASIC platforms backend ethernet ports are identified as
# 'Ethernet-BPxy'. Add 1024 to sort backend ports to the end.
value.sort(
key = lambda k: int(k[8:]) if "BP" not in k else int(k[11:]) + 1024
Copy link
Collaborator

@lguohan lguohan Jul 18, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

it is difficult to understand the logic here, for example, what does BP mean? what is 1024? why 1024?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Backend ethernet ports are identified as 'Ethernet-BPxy'. The change extracts the port number for the backend ports. '1024' is just to sort the back end ports after the front end ports, assuming number of front end ports will be < 1024.
'+ 1024' has no functional impact other than for cleaner look in config_db. All backend ports are grouped at the end. This can be removed if that is preferred.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment above this sort for future reference?

)

def is_ipv4(value):
if not value:
Expand Down
45 changes: 45 additions & 0 deletions src/sonic-config-engine/tests/test_multinpu_cfggen.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import os
import json
import yaml
import shutil

SKU = 'multi-npu-01'
ASIC_SKU = 'multi-npu-asic'
Expand Down Expand Up @@ -274,3 +275,47 @@ def test_loopback_intfs(self):
"Loopback0|FC00:1::32/128": {},
"Loopback4096|8.0.0.5/32": {},
"Loopback4096|FD00:4::32/128": {}})

def test_buffers_multi_asic_template(self):
build_root_dir = os.path.join(
self.test_dir, "..", "..", ".."
)
# using Trident2 buffer configuration
device_config_dir = os.path.join(
build_root_dir,
"device",
"arista",
"x86_64-arista_7050_qx32",
"Arista-7050-QX32"
)
device_buffer_template = os.path.join(
device_config_dir, "buffers.json.j2"
)
buffer_template = os.path.join(
build_root_dir, "files", "build_templates", "buffers_config.j2"
)
port_config_ini_asic0 = os.path.join(
self.test_data_dir, "sample_port_config-0.ini"
)
# asic0 - mix of front end and back end ports
shutil.copy2(buffer_template, device_config_dir)
argument = "-m {} -p {} -n asic0 -t {}".format(
self.sample_graph, port_config_ini_asic0, device_buffer_template
)
output = json.loads(self.run_script(argument))
os.remove(os.path.join(device_config_dir, "buffers_config.j2"))
self.assertDictEqual(
output['CABLE_LENGTH'],
{
'AZURE': {
'Ethernet8': '300m',
'Ethernet0': '300m',
'Ethernet4': '300m',
'Ethernet-BP4': '5m',
'Ethernet-BP0': '5m',
'Ethernet-BP12': '5m',
'Ethernet-BP8': '5m',
'Ethernet12': '300m'
}
}
)