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

[config qos] QoS and Buffer config genration for multi ASIC platforms #978

Merged
merged 8 commits into from
Aug 8, 2020
Merged

[config qos] QoS and Buffer config genration for multi ASIC platforms #978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5d7b03d
QoS and Buffer config genration for multi ASIC platforms
smaheshm Jul 8, 2020
9df039f
* use get_npu_id_from_name(namespace) API
smaheshm Jul 13, 2020
13d235c
Updated print statements
smaheshm Jul 24, 2020
6b8591d
removed the option of providing namespace for Qos commands
smaheshm Jul 27, 2020
a9e7347
Merge branch 'master' into masic
smaheshm Aug 4, 2020
ffd5d1a
removed extra space when not running under a namespace
smaheshm Aug 5, 2020
d7a7ffe
Merge remote-tracking branch 'upstream/master' into masic
smaheshm Aug 5, 2020
59148e8
remove extra spaces in load minigraph output in unit test
smaheshm Aug 5, 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
117 changes: 92 additions & 25 deletions config/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -663,10 +663,21 @@ def _clear_qos():
'BUFFER_PROFILE',
'BUFFER_PG',
'BUFFER_QUEUE']
config_db = ConfigDBConnector()
config_db.connect()
for qos_table in QOS_TABLE_NAMES:
config_db.delete_table(qos_table)

namespace_list = [DEFAULT_NAMESPACE]
if sonic_device_util.get_num_npus() > 1:
namespace_list = sonic_device_util.get_namespaces()

for ns in namespace_list:
if ns is DEFAULT_NAMESPACE:
smaheshm marked this conversation as resolved.
Show resolved Hide resolved
config_db = ConfigDBConnector()
else:
config_db = ConfigDBConnector(
use_unix_socket_path=True, namespace=ns
)
config_db.connect()
for qos_table in QOS_TABLE_NAMES:
config_db.delete_table(qos_table)

def _get_sonic_generated_services(num_asic):
if not os.path.isfile(SONIC_GENERATED_SERVICE_PATH):
Expand Down Expand Up @@ -1229,11 +1240,11 @@ def load_minigraph(no_service_restart):
if namespace is DEFAULT_NAMESPACE:
config_db = ConfigDBConnector()
cfggen_namespace_option = " "
ns_cmd_prefix = " "
ns_cmd_prefix = ""
else:
config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace)
cfggen_namespace_option = " -n {}".format(namespace)
ns_cmd_prefix = "sudo ip netns exec {}".format(namespace)
ns_cmd_prefix = "sudo ip netns exec {} ".format(namespace)
config_db.connect()
client = config_db.get_redis_client(config_db.CONFIG_DB)
client.flushdb()
Expand All @@ -1250,12 +1261,14 @@ def load_minigraph(no_service_restart):
# These commands are not run for host on multi asic platform
if num_npus == 1 or namespace is not DEFAULT_NAMESPACE:
if device_type != 'MgmtToRRouter':
run_command('{} pfcwd start_default'.format(ns_cmd_prefix), display_cmd=True)
run_command("{} config qos reload".format(ns_cmd_prefix), display_cmd=True)
run_command('{}pfcwd start_default'.format(ns_cmd_prefix), display_cmd=True)
smaheshm marked this conversation as resolved.
Show resolved Hide resolved

if os.path.isfile('/etc/sonic/acl.json'):
run_command("acl-loader update full /etc/sonic/acl.json", display_cmd=True)

# generate QoS and Buffer configs
run_command("config qos reload", display_cmd=True)

# Write latest db version string into db
db_migrator='/usr/bin/db_migrator.py'
if os.path.isfile(db_migrator) and os.access(db_migrator, os.X_OK):
Expand Down Expand Up @@ -1640,26 +1653,80 @@ def reload():
_clear_qos()
platform = sonic_device_util.get_platform()
hwsku = sonic_device_util.get_hwsku()
buffer_template_file = os.path.join('/usr/share/sonic/device/', platform, hwsku, 'buffers.json.j2')
if os.path.isfile(buffer_template_file):
command = "{} -d -t {} >/tmp/buffers.json".format(SONIC_CFGGEN_PATH, buffer_template_file)
run_command(command, display_cmd=True)

qos_template_file = os.path.join('/usr/share/sonic/device/', platform, hwsku, 'qos.json.j2')
sonic_version_file = os.path.join('/etc/sonic/', 'sonic_version.yml')
if os.path.isfile(qos_template_file):
command = "{} -d -t {} -y {} >/tmp/qos.json".format(SONIC_CFGGEN_PATH, qos_template_file, sonic_version_file)
run_command(command, display_cmd=True)
namespace_list = [DEFAULT_NAMESPACE]
if sonic_device_util.get_num_npus() > 1:
namespace_list = sonic_device_util.get_namespaces()

# Apply the configurations only when both buffer and qos configuration files are presented
command = "{} -j /tmp/buffers.json --write-to-db".format(SONIC_CFGGEN_PATH)
run_command(command, display_cmd=True)
command = "{} -j /tmp/qos.json --write-to-db".format(SONIC_CFGGEN_PATH)
for ns in namespace_list:
if ns is DEFAULT_NAMESPACE:
asic_id_suffix = ""
else:
asic_id = sonic_device_util.get_npu_id_from_name(ns)
if asic_id is None:
click.secho(
"Command 'qos reload' failed with invalid namespace '{}'".
format(ns),
fg='yellow'
)
raise click.Abort()
Copy link
Contributor

Choose a reason for hiding this comment

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

we have sonic_device_util.get_all_namespaces() that provide the dictionary of front and back namespace with string as namespace value. Then no need to convert from id to string. We can see if this API can be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need the ASIC ID. That API doesn't give the ASIC ID.

Copy link
Contributor

Choose a reason for hiding this comment

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

why can't we have directory name as "asic0" ? i feel that is better and correlate to namespace directory created by linux.
We can change port_config.ini to also be in similar folder

@judyjoseph @SuvarnaMeenakshi what do you think here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a temporary directory, it can be anything. I still need the ASIC ID to read, and matches that format.

/usr/share/sonic/device/0
/usr/share/sonic/device/1
...

Copy link
Contributor Author

@smaheshm smaheshm Jul 27, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

ASIC IDs and ASIC namespaces are different. The configuration is for each ASIC ID rather than namespace. I think it's OK to specify ASIC ID for configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

A simple approach would be to get the num of asics present in the platform and do a for range loop. We are using the asic_index as asic_id's internally.
If we derive the asic_id from namespace name, could cause problems if we create a namespace for a different purpose in the switch ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using 'get_namespaces()' API, this should return the valid namespaces for ASICs. It's better to use common APIs so there's one place that calculates the ASIC ID and namespace. Using a range loop can be fragile if the ASIC ID allocation scheme changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a temporary directory, it can be anything. I still need the ASIC ID to read, and matches that format.

/usr/share/sonic/device/0
/usr/share/sonic/device/1
...

@smaheshm why can't asic_id_suffix be namespace name directly ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because the directories use 'asic ID'.

/usr/share/sonic/device/0
/usr/share/sonic/device/1
...

asic_id_suffix = str(asic_id)

buffer_template_file = os.path.join(
'/usr/share/sonic/device/',
platform,
hwsku,
asic_id_suffix,
'buffers.json.j2'
)
buffer_output_file = "/tmp/buffers{}.json".format(asic_id_suffix)
qos_output_file = "/tmp/qos{}.json".format(asic_id_suffix)

cmd_ns = "" if ns is DEFAULT_NAMESPACE else "-n {}".format(ns)
if os.path.isfile(buffer_template_file):
command = "{} {} -d -t {} > {}".format(
SONIC_CFGGEN_PATH,
cmd_ns,
buffer_template_file,
buffer_output_file
)
run_command(command, display_cmd=True)
qos_template_file = os.path.join(
'/usr/share/sonic/device/',
platform,
hwsku,
asic_id_suffix,
'qos.json.j2'
)
sonic_version_file = os.path.join(
'/etc/sonic/', 'sonic_version.yml'
)
if os.path.isfile(qos_template_file):
command = "{} {} -d -t {} -y {} > {}".format(
SONIC_CFGGEN_PATH,
cmd_ns,
qos_template_file,
sonic_version_file,
qos_output_file
)
run_command(command, display_cmd=True)
# Apply the configurations only when both buffer and qos
# configuration files are presented
command = "{} {} -j {} --write-to-db".format(
SONIC_CFGGEN_PATH, cmd_ns, buffer_output_file
)
run_command(command, display_cmd=True)
command = "{} {} -j {} --write-to-db".format(
SONIC_CFGGEN_PATH, cmd_ns, qos_output_file
)
run_command(command, display_cmd=True)
else:
click.secho('QoS definition template not found at {}'.format(
qos_template_file
), fg='yellow')
else:
click.secho('QoS definition template not found at {}'.format(qos_template_file), fg='yellow')
else:
click.secho('Buffer definition template not found at {}'.format(buffer_template_file), fg='yellow')
click.secho('Buffer definition template not found at {}'.format(
buffer_template_file
), fg='yellow')

#
# 'warm_restart' group ('config warm_restart ...')
Expand Down
4 changes: 2 additions & 2 deletions tests/config_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
Executing stop of service hostcfgd...
Executing stop of service nat...
Running command: /usr/local/bin/sonic-cfggen -H -m --write-to-db
Running command: pfcwd start_default
Running command: config qos reload
Running command: pfcwd start_default
Running command: config qos reload
Executing reset-failed of service bgp...
Executing reset-failed of service dhcp_relay...
Executing reset-failed of service hostcfgd...
Expand Down