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

Changes from 2 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
179 changes: 137 additions & 42 deletions config/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ def _is_neighbor_ipaddress(config_db, ipaddress):

def _get_all_neighbor_ipaddresses(config_db, ignore_local_hosts=False):
"""Returns list of strings containing IP addresses of all BGP neighbors
if the flag ignore_local_hosts is set to True, additional check to see if
if the flag ignore_local_hosts is set to True, additional check to see if
if the BGP neighbor AS number is same as local BGP AS number, if so ignore that neigbor.
"""
addrs = []
Expand Down Expand Up @@ -454,7 +454,7 @@ def _change_hostname(hostname):
run_command('sed -i "/\s{}$/d" /etc/hosts'.format(current_hostname), display_cmd=True)
run_command('echo "127.0.0.1 {}" >> /etc/hosts'.format(hostname), display_cmd=True)

def _clear_qos():
def _clear_qos(namespace):
smaheshm marked this conversation as resolved.
Show resolved Hide resolved
QOS_TABLE_NAMES = [
'TC_TO_PRIORITY_GROUP_MAP',
'MAP_PFC_PRIORITY_TO_QUEUE',
Expand All @@ -470,10 +470,32 @@ 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()

if namespace:
if namespace not in namespace_list:
click.secho(
"Command {} failed with invalid namespace {}".format(
"qos reload", namespace
neethajohn marked this conversation as resolved.
Show resolved Hide resolved
),
fg='yellow'
)
raise click.Abort()
namespace_list = [namespace]

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 @@ -819,7 +841,7 @@ def load(filename, yes):
# if any of the config files in linux host OR namespace is not present, return
if not os.path.isfile(file):
click.echo("The config_db file {} doesn't exist".format(file))
return
return

if namespace is None:
command = "{} -j {} --write-to-db".format(SONIC_CFGGEN_PATH, file)
Expand Down Expand Up @@ -1033,11 +1055,12 @@ def load_minigraph(no_service_restart):
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)

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 All @@ -1047,7 +1070,7 @@ def load_minigraph(no_service_restart):
else:
cfggen_namespace_option = " -n {}".format(namespace)
run_command(db_migrator + ' -o set_version' + cfggen_namespace_option)

# We first run "systemctl reset-failed" to remove the "failed"
# status from all services before we attempt to restart them
if not no_service_restart:
Expand Down Expand Up @@ -1410,38 +1433,110 @@ def qos(ctx):
pass

@qos.command('clear')
def clear():
@click.option(
'-n', '--namespace', default=None, type=str, help="Network namespace"
)
def clear(namespace):
"""Clear QoS configuration"""
log_info("'qos clear' executing...")
_clear_qos()
_clear_qos(namespace)

@qos.command('reload')
def reload():
@click.option(
'-n', '--namespace', default=None, type=str, help="Network namespace"
)
def reload(namespace):
"""Reload QoS configuration"""
log_info("'qos reload' executing...")
_clear_qos()
_clear_qos(namespace)
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()

if namespace:
if namespace not in namespace_list:
click.secho(
"Command {} failed with invalid namespace {}".format(
"qos reload", namespace
neethajohn marked this conversation as resolved.
Show resolved Hide resolved
),
fg='yellow'
)
raise click.Abort()
namespace_list = [namespace]

# 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 {} failed with invalid namespace {}".format(
"qos reload", 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 Expand Up @@ -1703,7 +1798,7 @@ def add_snmp_agent_address(ctx, agentip, port, vrf):
#Construct SNMP_AGENT_ADDRESS_CONFIG table key in the format ip|<port>|<vrf>
key = agentip+'|'
if port:
key = key+port
key = key+port
key = key+'|'
if vrf:
key = key+vrf
Expand All @@ -1724,7 +1819,7 @@ def del_snmp_agent_address(ctx, agentip, port, vrf):

key = agentip+'|'
if port:
key = key+port
key = key+port
key = key+'|'
if vrf:
key = key+vrf
Expand Down Expand Up @@ -1986,7 +2081,7 @@ def all(verbose):
config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace)
config_db.connect()
bgp_neighbor_ip_list = _get_all_neighbor_ipaddresses(config_db, ignore_local_hosts)
for ipaddress in bgp_neighbor_ip_list:
for ipaddress in bgp_neighbor_ip_list:
_change_bgp_session_status_by_addr(config_db, ipaddress, 'up', verbose)

# 'neighbor' subcommand
Expand Down Expand Up @@ -2157,7 +2252,7 @@ def speed(ctx, interface_name, interface_speed, verbose):
run_command(command, display_cmd=verbose)

def _get_all_mgmtinterface_keys():
"""Returns list of strings containing mgmt interface keys
"""Returns list of strings containing mgmt interface keys
"""
config_db = ConfigDBConnector()
config_db.connect()
Expand Down Expand Up @@ -2885,9 +2980,9 @@ def priority(ctx, interface_name, priority, status):
interface_name = interface_alias_to_name(interface_name)
if interface_name is None:
ctx.fail("'interface_name' is None!")

run_command("pfc config priority {0} {1} {2}".format(status, interface_name, priority))

#
# 'platform' group ('config platform ...')
#
Expand Down Expand Up @@ -2986,10 +3081,10 @@ def naming_mode_alias():
def is_loopback_name_valid(loopback_name):
"""Loopback name validation
"""

if loopback_name[:CFG_LOOPBACK_PREFIX_LEN] != CFG_LOOPBACK_PREFIX :
return False
if (loopback_name[CFG_LOOPBACK_PREFIX_LEN:].isdigit() is False or
if (loopback_name[CFG_LOOPBACK_PREFIX_LEN:].isdigit() is False or
int(loopback_name[CFG_LOOPBACK_PREFIX_LEN:]) > CFG_LOOPBACK_ID_MAX_VAL) :
return False
if len(loopback_name) > CFG_LOOPBACK_NAME_TOTAL_LEN_MAX:
Expand Down Expand Up @@ -3156,7 +3251,7 @@ def add_ntp_server(ctx, ntp_ip_address):
if ntp_ip_address in ntp_servers:
click.echo("NTP server {} is already configured".format(ntp_ip_address))
return
else:
else:
db.set_entry('NTP_SERVER', ntp_ip_address, {'NULL': 'NULL'})
click.echo("NTP server {} added to configuration".format(ntp_ip_address))
try:
Expand All @@ -3177,7 +3272,7 @@ def del_ntp_server(ctx, ntp_ip_address):
if ntp_ip_address in ntp_servers:
db.set_entry('NTP_SERVER', '{}'.format(ntp_ip_address), None)
click.echo("NTP server {} removed from configuration".format(ntp_ip_address))
else:
else:
ctx.fail("NTP server {} is not configured.".format(ntp_ip_address))
try:
click.echo("Restarting ntp-config service...")
Expand Down Expand Up @@ -3465,7 +3560,7 @@ def delete(ctx):

#
# 'feature' command ('config feature name state')
#
#
@config.command('feature')
@click.argument('name', metavar='<feature-name>', required=True)
@click.argument('state', metavar='<feature-state>', required=True, type=click.Choice(["enabled", "disabled"]))
Expand Down