From 32da2bc12d846dcc4427e3ef80a194e464ab57f2 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Fri, 11 Sep 2020 19:58:22 +0800 Subject: [PATCH 1/5] Support dynamic buffer calculation 1. commands added(see below) and testcases for new commands 2. db_migrator: - migrate CONFIG_DB from old approach to the new approach - when system warm starts from old image to the new one, copies related tables from CONFIG_DB to APPL_DB for the purpose that buffermgrd can start smoothly 3. warm-reboot script: don't clear BUFFER_MAX_PARAM table across warm reboot CLI list - config interface buffer priority-group lossless - config interface buffer priority-group lossless add [headroom-override-profile] for adding the PG for the first time providing option headroom-override-profile means to configure the PG as headroom override otherwise as dynamically calculated headroom - config interface buffer priority-group lossless set [headroom-override-profile] for modifying an existing PG, the option headroom-override-profile has the same meaning as "add" - config interface buffer priority-group lossless remove [PG] for removing the PG specified by option PG. if the option isn't provided, all lossless PGs on the port will be removed - config buffer-profile To add, modify or remove buffer profiles - show buffer Testcase covered for global config commands. show command unconvered due to subprocess call isn't supported by test infra Signed-off-by: Stephen Sun --- config/main.py | 368 ++++++++++++++++++++++++++++ doc/Command-Reference.md | 332 ++++++++++++++++++++++++- scripts/buffershow | 1 + scripts/db_migrator.py | 184 +++++++++++++- scripts/fast-reboot | 3 +- scripts/mellanox_buffer_migrator.py | 54 +++- scripts/mmuconfig | 105 +++++--- setup.py | 1 + show/main.py | 30 +++ tests/buffer_test.py | 70 ++++++ tests/mock_tables/config_db.json | 44 ++++ 11 files changed, 1150 insertions(+), 42 deletions(-) mode change 100755 => 100644 config/main.py create mode 120000 scripts/buffershow mode change 100644 => 100755 scripts/mmuconfig mode change 100755 => 100644 show/main.py create mode 100644 tests/buffer_test.py diff --git a/config/main.py b/config/main.py old mode 100755 new mode 100644 index b832c45cb9..8ad5d0b8d3 --- a/config/main.py +++ b/config/main.py @@ -2517,6 +2517,239 @@ def remove(ctx, interface_name, ip_addr): except ValueError: ctx.fail("'ip_addr' is not valid.") + +# +# buffer commands and utilities +# +def pgmaps_check_legality(ctx, interface_name, input_pg, is_new_pg): + """ + Tool function to check whether input_pg is legal. + Three checking performed: + 1. Whether the input_pg is legal: pgs are in range [0-7] + 2. Whether the input_pg overlaps an existing pg in the port + """ + config_db = ctx.obj["config_db"] + + try: + lower = int(input_pg[0]) + upper = int(input_pg[-1]) + + if upper < lower or lower < 0 or upper > 7: + ctx.fail("PG {} is not valid.".format(input_pg)) + except Exception: + ctx.fail("PG {} is not valid.".format(input_pg)) + + # Check overlapping. + # To configure a new PG which is overlapping an existing one is not allowed + # For example, to add '5-6' while '3-5' existing is illegal + existing_pgs = config_db.get_table("BUFFER_PG") + if not is_new_pg: + if not (interface_name, input_pg) in existing_pgs.keys(): + ctx.fail("PG {} doesn't exist".format(input_pg)) + return + + for k, v in existing_pgs.iteritems(): + port, existing_pg = k + if port == interface_name: + existing_lower = int(existing_pg[0]) + existing_upper = int(existing_pg[-1]) + if existing_upper < lower or existing_lower > upper: + # new and existing pgs disjoint, legal + pass + else: + ctx.fail("PG {} overlaps with existing PG {}".format(input_pg, existing_pg)) + + +def update_pg(ctx, interface_name, pg_map, override_profile, add = True): + config_db = ctx.obj["config_db"] + + # Check whether port is legal + ports = config_db.get_entry("PORT", interface_name) + if not ports: + ctx.fail("Port {} doesn't exist".format(interface_name)) + + # Check whether pg_map is legal + # Check whether there is other lossless profiles configured on the interface + pgmaps_check_legality(ctx, interface_name, pg_map, add) + + # All checking passed + if override_profile: + profile_dict = config_db.get_entry("BUFFER_PROFILE", override_profile) + if not profile_dict: + ctx.fail("Profile {} doesn't exist".format(override_profile)) + if not 'xoff' in profile_dict.keys() and 'size' in profile_dict.keys(): + ctx.fail("Profile {} doesn't exist or isn't a lossless profile".format(override_profile)) + profile_full_name = "[BUFFER_PROFILE|{}]".format(override_profile) + config_db.set_entry("BUFFER_PG", (interface_name, pg_map), {"profile": profile_full_name}) + else: + config_db.set_entry("BUFFER_PG", (interface_name, pg_map), {"profile": "NULL"}) + adjust_pfc_enable(ctx, interface_name, pg_map, True) + + +def remove_pg_on_port(ctx, interface_name, pg_map): + config_db = ctx.obj["config_db"] + + # Check whether port is legal + ports = config_db.get_entry("PORT", interface_name) + if not ports: + ctx.fail("Port {} doesn't exist".format(interface_name)) + + # Remvoe all dynamic lossless PGs on the port + existing_pgs = config_db.get_table("BUFFER_PG") + removed = False + for k, v in existing_pgs.iteritems(): + port, existing_pg = k + if port == interface_name and (not pg_map or pg_map == existing_pg): + need_to_remove = False + referenced_profile = v.get('profile') + if referenced_profile and referenced_profile == '[BUFFER_PROFILE|ingress_lossy_profile]': + if pg_map: + ctx.fail("Lossy PG {} can't be removed".format(pg_map)) + else: + continue + config_db.set_entry("BUFFER_PG", (interface_name, existing_pg), None) + adjust_pfc_enable(ctx, interface_name, pg_map, False) + removed = True + if not removed: + if pg_map: + ctx.fail("No specified PG {} found on port {}".format(pg_map, interface_name)) + else: + ctx.fail("No lossless PG found on port {}".format(interface_name)) + + +def adjust_pfc_enable(ctx, interface_name, pg_map, add): + config_db = ctx.obj["config_db"] + + # Fetch the original pfc_enable + qosmap = config_db.get_entry("PORT_QOS_MAP", interface_name) + pfc_enable = qosmap.get("pfc_enable") + + pfc_set = set() + if pfc_enable: + for priority in pfc_enable.split(","): + pfc_set.add(int(priority)) + + if pg_map: + lower_bound = int(pg_map[0]) + upper_bound = int(pg_map[-1]) + + for priority in range(lower_bound, upper_bound + 1): + if add: + pfc_set.add(priority) + elif priority in pfc_set: + pfc_set.remove(priority) + + empty_set = set() + pfc_enable = "" + if not pfc_set.issubset(empty_set): + for priority in pfc_set: + pfc_enable += str(priority) + "," + elif not add: + # Remove all + pfc_enable = "" + else: + ctx.fail("Try to add empty priorities") + + qosmap["pfc_enable"] = pfc_enable[:-1] + config_db.set_entry("PORT_QOS_MAP", interface_name, qosmap) + + +# +# 'buffer' subgroup ('config interface buffer ...') +# +@interface.group(cls=clicommon.AbbreviationGroup) +@click.pass_context +def buffer(ctx): + """Set or clear buffer configuration""" + pass + + +# +# 'priority_group' subgroup ('config interface buffer priority_group ...') +# +@buffer.group(cls=clicommon.AbbreviationGroup) +@click.pass_context +def priority_group(ctx): + """Set or clear buffer configuration""" + pass + + +# +# 'lossless' subgroup ('config interface buffer priority_group lossless ...') +# +@priority_group.group(cls=clicommon.AbbreviationGroup) +@click.pass_context +def lossless(ctx): + """Set or clear lossless PGs""" + pass + + +# +# 'add' subcommand +# +@lossless.command('add') +@click.argument('interface_name', metavar='', required=True) +@click.argument('pg_map', metavar='', required=True) +@click.argument('override_profile', metavar='', required=False) +@click.pass_context +def add_pg(ctx, interface_name, pg_map, override_profile): + """Set lossless PGs for the interface""" + update_pg(ctx, interface_name, pg_map, override_profile) + + +# +# 'set' subcommand +# +@lossless.command('set') +@click.argument('interface_name', metavar='', required=True) +@click.argument('pg_map', metavar='', required=True) +@click.argument('override_profile', metavar='', required=False) +@click.pass_context +def set_pg(ctx, interface_name, pg_map, override_profile): + """Set lossless PGs for the interface""" + update_pg(ctx, interface_name, pg_map, override_profile, False) + + +# +# 'remove' subcommand +# +@lossless.command('remove') +@click.argument('interface_name', metavar='', required=True) +@click.argument('pg_map', metavar='m, like 300m".format(cable_length)) + + keys = config_db.get_keys("CABLE_LENGTH") + + cable_length_set = {} + cable_length_set[interface_name] = length + config_db.mod_entry("CABLE_LENGTH", keys[0], cable_length_set) + # # 'transceiver' subgroup ('config interface transceiver ...') # @@ -3097,6 +3330,141 @@ def priority(ctx, interface_name, priority, status): clicommon.run_command("pfc config priority {0} {1} {2}".format(status, interface_name, priority)) +# +# 'buffer_profile' group ('config buffer_profile ...') +# + +@config.group(cls=clicommon.AbbreviationGroup) +@click.pass_context +def buffer_profile(ctx): + """Configure buffer_profile""" + +@buffer_profile.command('add') +#@click.option('-profile', metavar='', type=str, help="Profile name", required=True) +@click.argument('profile', metavar='', required=True) +@click.option('-xon', metavar='', type=int, help="Set xon threshold") +@click.option('-xoff', metavar='', type=int, help="Set xoff threshold") +@click.option('-headroom', metavar='', type=int, help="Set reserved headroom size") +@click.option('-dynamic_th', metavar='', type=str, help="Set dynamic threshold") +@click.option('-pool', metavar='', type=str, help="Buffer pool") +@click.pass_context +def add_profile(ctx, profile, xon, xoff, headroom, dynamic_th, pool): + """Add or modify a buffer profile""" + config_db = ConfigDBConnector() + config_db.connect() + + profile_entry = config_db.get_entry('BUFFER_PROFILE', profile) + if profile_entry: + ctx.fail("Profile {} already exist".format(profile)) + + update_profile(ctx, config_db, profile, xon, xoff, headroom, dynamic_th, pool) + + +@buffer_profile.command('set') +#@click.option('-profile', metavar='', type=str, help="Profile name", required=True) +@click.argument('profile', metavar='', required=True) +@click.option('-xon', metavar='', type=int, help="Set xon threshold") +@click.option('-xoff', metavar='', type=int, help="Set xoff threshold") +@click.option('-headroom', metavar='', type=int, help="Set reserved headroom size") +@click.option('-dynamic_th', metavar='', type=str, help="Set dynamic threshold") +@click.option('-pool', metavar='', type=str, help="Buffer pool") +@click.pass_context +def set_profile(ctx, profile, xon, xoff, headroom, dynamic_th, pool): + """Add or modify a buffer profile""" + config_db = ConfigDBConnector() + config_db.connect() + + profile_entry = config_db.get_entry('BUFFER_PROFILE', profile) + if not profile_entry: + ctx.fail("Profile {} doesn't exist".format(profile)) + + if not 'xoff' in profile_entry.keys() and xoff: + ctx.fail("Can't change profile {} from dynamically calculating headroom to non-dynamically one".format(profile)) + + update_profile(ctx, config_db, profile, xon, xoff, headroom, dynamic_th, pool, profile_entry) + + +def update_profile(ctx, config_db, profile_name, xon, xoff, headroom, dynamic_th, pool, profile_entry = None): + params = {} + if profile_entry: + params = profile_entry + dynamic_calculate = True + + if not pool: + pool = 'ingress_lossless_pool' + params['pool'] = '[BUFFER_POOL|' + pool + ']' + if not config_db.get_entry('BUFFER_POOL', pool): + ctx.fail("Pool {} doesn't exist".format(pool)) + + if xon: + params['xon'] = xon + dynamic_calculate = False + else: + xon = params.get('xon') + + if xoff: + params['xoff'] = xoff + dynamic_calculate = False + else: + xoff = params.get('xoff') + + if headroom: + params['size'] = headroom + dynamic_calculate = False + if xon and not xoff: + xoff = int(headroom) - int (xon) + params['xoff'] = xoff + elif not dynamic_calculate: + if xon and xoff: + headroom = int(xon) + int(xoff) + params['size'] = headroom + else: + ctx.fail("Either both xon and xoff or headroom should be provided") + + if dynamic_calculate: + params['headroom_type'] = 'dynamic' + if not dynamic_th: + ctx.fail("Either headroom information (xon, xoff, headroom) or dynamic_th needs to be provided") + + if dynamic_th: + params['dynamic_th'] = dynamic_th + else: + # Fetch all the keys of default_lossless_buffer_parameter table + # and then get the default_dynamic_th from that entry (should be only one) + keys = config_db.get_keys('DEFAULT_LOSSLESS_BUFFER_PARAMETER') + if len(keys) > 1 or len(keys) == 0: + ctx.fail("Multiple or no entry in DEFAULT_LOSSLESS_BUFFER_PARAMETER found while no dynamic_th specified") + + default_lossless_param = config_db.get_entry('DEFAULT_LOSSLESS_BUFFER_PARAMETER', keys[0]) + if 'default_dynamic_th' in default_lossless_param.keys(): + params['dynamic_th'] = default_lossless_param['default_dynamic_th'] + else: + ctx.fail("No dynamic_th defined in DEFAULT_LOSSLESS_BUFFER_PARAMETER") + + config_db.set_entry("BUFFER_PROFILE", (profile_name), params) + +@buffer_profile.command('remove') +@click.argument('profile', metavar='', required=True) +@click.pass_context +def remove_profile(ctx, profile): + """Delete a buffer profile""" + config_db = ConfigDBConnector() + config_db.connect() + + full_profile_name = '[BUFFER_PROFILE|{}]'.format(profile) + existing_pgs = config_db.get_table("BUFFER_PG") + for k, v in existing_pgs.iteritems(): + port, pg = k + referenced_profile = v.get('profile') + if referenced_profile and referenced_profile == full_profile_name: + ctx.fail("Profile {} is referenced by {}|{} and can't be removed".format(profile, port, pg)) + + entry = config_db.get_entry("BUFFER_PROFILE", profile) + if entry: + config_db.set_entry("BUFFER_PROFILE", profile, None) + else: + ctx.fail("Profile {} doesn't exist".format(profile)) + # # 'platform' group ('config platform ...') # diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 1887d29118..bf6366fc2a 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -42,6 +42,9 @@ * [Drop Counter show commands](#drop-counters-show-commands) * [Drop Counter config commands](#drop-counters-config-commands) * [Drop Counter clear commands](#drop-counters-clear-commands) +* [Dynamic Buffer Management](#dynamic-buffer-management) + * [Configuration commands](#configuration-commands) + * [Show commands](#show-commands) * [ECN](#ecn) * [ECN show commands](#ecn-show-commands) * [ECN config commands](#ecn-config-commands) @@ -2163,7 +2166,7 @@ This command is used to delete a configured DHCP Relay Destination IP address fr Go Back To [Beginning of the document](#) or [Beginning of this section](#dhcp-relay) -# Drop Counters +## Drop Counters This section explains all the Configurable Drop Counters show commands and configuration options that are supported in SONiC. @@ -2345,8 +2348,310 @@ This comnmand is used to clear drop counters. This is done on a per-user basis. Cleared drop counters ``` -Go Back To [Beginning of the document](#) or [Beginning of this section](#drop-counters) +Go Back To [Beginning of the document](#) or [Beginning of this section](##drop-counters) +## Dynamic Buffer Management + +This section explains all the show and configuration commands regarding the dynamic buffer management. + +Dynamic buffer management is responsible for calculating buffer size according to the ports' configured speed and administrative state. In order to enable dynamic buffer management feature, the ports' speed must be configured. For this please refer [Interface naming mode config commands](#interface-naming-mode-config-commands) + +### Configuration commands + +**configure a lossless buffer profile** + +This command is used to configure a lossless buffer profile. + +- Usage: + + ``` + config buffer_profile add -xon -xoff [-headroom ] [-dynamic_th ] [-pool ] + config buffer_profile set -xon -xoff [-headroom ] [-dynamic_th ] [-pool ] + config buffer_profile remove + ``` + + All the parameters are devided to two groups, one for headroom and one for dynamic_th. For any command at lease one group of parameters should be provided. + For headroom parameters: + + - At lease one of `xoff` and `headroom` should be provided and the other will be optional and conducted via the formula `xon + xoff = headroom`. + All other parameters are optional. + - `xon` is madantory. + - `xon` + `xoff` <= `headroom`; For Mellanox platform xon + xoff == headroom + + If only headroom parameters are provided, the `dynamic_th` will be taken from `CONFIG_DB.DEFAULT_LOSSLESS_BUFFER_PARAMETER.default_dynamic_th`. + + If only dynamic_th parameter is provided, the `headroom_type` will be set as `dynamic` and `xon`, `xoff` and `size` won't be set. This is only used for non default dynamic_th. In this case, the profile won't be deployed to ASIC directly. It can be configured to a lossless PG and then a dynamic profile will be generated based on the port's speed, cable length, and MTU and deployed to the ASIC. + + The subcommand `add` is designed for adding a new buffer profile to the system. + + The subcommand `set` is designed for modifying an existing buffer profile in the system. + For a profile with dynamically calculated headroom information, only `dynamic_th` can be modified. + + The subcommand `remove` is designed for removing an existing buffer profile from the system. When removing a profile, it shouldn't be referenced by any entry in `CONFIG_DB.BUFFER_PG`. + +- Example: + + ``` + admin@sonic:~$ sudo config buffer_profile add profile1 -xon 18432 -xoff 18432 + admin@sonic:~$ sudo config buffer_profile remove profile1 + ``` + +**config interface cable_length** + +This command is used to configure the length of the cable connected to a port. The cable_length is in unit of meters and must be suffixed with "m". + +- Usage: + + ``` + config interface cable_length + ``` + +- Example: + + ``` + admin@sonic:~$ sudo config interface cable_length Ethernet0 40m + ``` + +Go Back To [Beginning of the document](#) or [Beginning of this section](#dynamic-buffer-management) + +**config interface buffer priority-group lossless** + +This command is used to configure the priority groups on which lossless traffic runs. + +- Usage: + + ``` + config interface buffer priority-group lossless add [profile] + config interface buffer priority-group lossless set [profile] + config interface buffer priority-group lossless remove [] + ``` + + The can be in one of the following two forms: + + - For a range of priorities, the lower bound and upper bound connected by a dash, like `3-4` + - For a single priority, the number, like `6` + + The `pg-map` represents the map of priorities for lossless traffic. It should be a string and in form of a bit map like `3-4`. The `-` connects the lower bound and upper bound of a range of priorities. + + The subcommand `add` is designed for adding a new lossless PG on top of current PGs. The new PG range must be disjoint with all existing PGs. + + For example, currently the PG range 3-4 exist on port Ethernet4, to add PG range 4-5 will fail because it isn't disjoint with 3-4. To add PG range 5-6 will succeed. After that both range 3-4 and 5-6 will work as lossless PG. + + The `override-profile` parameter is optional. When provided, it represents the predefined buffer profile for headroom override. + + The subcommand `set` is designed for modifying an existing PG from dynamic calculation to headroom override or vice versa. The `pg-map` must be an existing PG. + + The subcommand `remove` is designed for removing an existing PG. The option `pg-map` must be an existing PG. All lossless PGs will be removed in case no `pg-map` provided. + +- Example: + + To configure lossless_pg on a port: + + ``` + admin@sonic:~$ sudo config interface buffer priority-group lossless add Ethernet0 3-4 + ``` + + To change the profile used for lossless_pg on a port: + + ``` + admin@sonic:~$ sudo config interface buffer priority-group lossless set Ethernet0 3-4 new-profile + ``` + + To remove one lossless priority from a port: + + ``` + admin@sonic:~$ sudo config interface buffer priority-group lossless remove Ethernet0 6 + ``` + + To remove all lossless priorities from a port: + + ``` + admin@sonic:~$ sudo config interface buffer priority-group lossless remove Ethernet0 + ``` + +Go Back To [Beginning of the document](#) or [Beginning of this section](#dynamic-buffer-management) + +### Show commands + +**show buffer information** + +This command is used to display the status of buffer pools and profiles currently deployed to the ASIC. + +- Usage: + + ``` + show buffer information + ``` + +- Example: + + ``` + admin@sonic:~$ buffershow -l + Pool: ingress_lossless_pool + ---- -------- + type ingress + mode dynamic + size 17170432 + ---- -------- + + Pool: egress_lossless_pool + ---- -------- + type egress + mode dynamic + size 34340822 + ---- -------- + + Pool: ingress_lossy_pool + ---- -------- + type ingress + mode dynamic + size 17170432 + ---- -------- + + Pool: egress_lossy_pool + ---- -------- + type egress + mode dynamic + size 17170432 + ---- -------- + + Profile: pg_lossless_100000_5m_profile + ---------- ----------------------------------- + xon 18432 + dynamic_th 0 + xoff 18432 + pool [BUFFER_POOL:ingress_lossless_pool] + size 36864 + ---------- ----------------------------------- + + Profile: q_lossy_profile + ---------- ------------------------------- + dynamic_th 3 + pool [BUFFER_POOL:egress_lossy_pool] + size 0 + ---------- ------------------------------- + + Profile: egress_lossy_profile + ---------- ------------------------------- + dynamic_th 3 + pool [BUFFER_POOL:egress_lossy_pool] + size 4096 + ---------- ------------------------------- + + Profile: egress_lossless_profile + ---------- ---------------------------------- + dynamic_th 7 + pool [BUFFER_POOL:egress_lossless_pool] + size 0 + ---------- ---------------------------------- + + Profile: ingress_lossless_profile + ---------- ----------------------------------- + dynamic_th 0 + pool [BUFFER_POOL:ingress_lossless_pool] + size 0 + ---------- ----------------------------------- + + Profile: pg_lossless_100000_79m_profile + ---------- ----------------------------------- + xon 18432 + dynamic_th 0 + xoff 60416 + pool [BUFFER_POOL:ingress_lossless_pool] + size 78848 + ---------- ----------------------------------- + + Profile: pg_lossless_100000_40m_profile + ---------- ----------------------------------- + xon 18432 + dynamic_th 0 + xoff 38912 + pool [BUFFER_POOL:ingress_lossless_pool] + size 57344 + ---------- ----------------------------------- + + Profile: ingress_lossy_profile + ---------- -------------------------------- + dynamic_th 3 + pool [BUFFER_POOL:ingress_lossy_pool] + size 0 + ---------- -------------------------------- + ``` + +**show buffer configure** + +This command is used to display the status of buffer pools and profiles currently configured. + +- Usage: + + ``` + show buffer configure + ``` + +- Example: + + ``` + admin@sonic:~$ show buffer configure + Pool: ingress_lossless_pool + ---- -------- + type ingress + mode dynamic + ---- -------- + + Pool: egress_lossless_pool + ---- -------- + type egress + mode dynamic + size 34340822 + ---- -------- + + Pool: ingress_lossy_pool + ---- -------- + type ingress + mode dynamic + ---- -------- + + Pool: egress_lossy_pool + ---- -------- + type egress + mode dynamic + ---- -------- + + Profile: q_lossy_profile + ---------- ------------------------------- + dynamic_th 3 + pool [BUFFER_POOL:egress_lossy_pool] + size 0 + ---------- ------------------------------- + + Profile: egress_lossy_profile + ---------- ------------------------------- + dynamic_th 3 + pool [BUFFER_POOL:egress_lossy_pool] + size 4096 + ---------- ------------------------------- + + Profile: egress_lossless_profile + ---------- ---------------------------------- + dynamic_th 7 + pool [BUFFER_POOL:egress_lossless_pool] + size 0 + ---------- ---------------------------------- + + Profile: ingress_lossless_profile + ---------- ----------------------------------- + dynamic_th 0 + pool [BUFFER_POOL:ingress_lossless_pool] + size 0 + ---------- ----------------------------------- + + Profile: ingress_lossy_profile + ---------- -------------------------------- + dynamic_th 3 + pool [BUFFER_POOL:ingress_lossy_pool] + size 0 + ---------- -------------------------------- + ``` ## ECN @@ -3259,6 +3564,29 @@ kindly use, double tab i.e. to see the available breakout option cust Go Back To [Beginning of the document](#) or [Beginning of this section](#interfaces) +**config interface cable_length (Versions >= 202006)** + +This command is used to configure the length of the cable connected to a port. The cable_length is in unit of meters and must be suffixed with "m". + +For details please refer [dynamic buffer management](#dynamic-buffer-management) + +Go Back To [Beginning of the document](#) or [Beginning of this section](#interfaces) + +**config interface lossless_pg (Versions >= 202006)** + +This command is used to configure the priority groups on which lossless traffic runs. + +For details please refer [dynamic buffer management](#dynamic-buffer-management) + +Go Back To [Beginning of the document](#) or [Beginning of this section](#interfaces) + +**config interface headroom_override (Versions >= 202006)** + +This command is used to configure a static buffer profile on a port's lossless priorities. There shouldn't be any `lossless_pg` configured on the port when configuring `headroom_override`. The port's headroom won't be updated after `headroom_override` has been configured on the port. + +For details please refer [dynamic buffer management](#dynamic-buffer-management) + +Go Back To [Beginning of the document](#) or [Beginning of this section](#interfaces) ## Interface Naming Mode diff --git a/scripts/buffershow b/scripts/buffershow new file mode 120000 index 0000000000..e223bf79ce --- /dev/null +++ b/scripts/buffershow @@ -0,0 +1 @@ +mmuconfig \ No newline at end of file diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py index c040551ac7..b67851b795 100755 --- a/scripts/db_migrator.py +++ b/scripts/db_migrator.py @@ -4,6 +4,7 @@ import json import sys import traceback +import re from sonic_py_common import device_info, logger from swsssdk import ConfigDBConnector, SonicDBConfig @@ -30,7 +31,7 @@ def __init__(self, namespace, socket=None): none-zero values. build: sequentially increase within a minor version domain. """ - self.CURRENT_VERSION = 'version_1_0_4' + self.CURRENT_VERSION = 'version_1_0_5' self.TABLE_NAME = 'VERSIONS' self.TABLE_KEY = 'DATABASE' @@ -50,6 +51,10 @@ def __init__(self, namespace, socket=None): if self.appDB is not None: self.appDB.connect(self.appDB.APPL_DB) + self.stateDB = SonicV2Connector(host='127.0.0.1') + if self.stateDB is not None: + self.stateDB.connect(self.stateDB.STATE_DB) + version_info = device_info.get_sonic_version_info() asic_type = version_info.get('asic_type') self.asic_type = asic_type @@ -112,7 +117,6 @@ def migrate_intf_table(self): "Vlan1000:192.168.0.1/21": {}", this function shall add an entry without IP prefix as ""Vlan1000": {}". This also migrates 'lo' to 'Loopback0' interface ''' - if self.appDB is None: return @@ -157,6 +161,158 @@ def migrate_copp_table(self): for copp_key in keys: self.appDB.delete(self.appDB.APPL_DB, copp_key) + def migrate_config_db_buffer_tables_for_dynamic_calculation(self, speed_list, cable_len_list, default_dynamic_th): + ''' + Migrate buffer tables to dynamic calculation mode + parameters + @speed_list - list of speed supported + @cable_len_list - list of cable length supported + @default_dynamic_th - default dynamic th + + 1. Buffer profiles for lossless PGs in BUFFER_PROFILE table will be removed + if their names have the convention of pg_lossless___profile + where the speed and cable_length belongs speed_list and cable_len_list respectively + and the dynamic_th is equal to default_dynamic_th + 2. Insert tables required for dynamic buffer calculation + - DEFAULT_LOSSLESS_BUFFER_PARAMETER|AZURE: {'default_dynamic_th': default_dynamic_th} + - LOSSLESS_TRAFFIC_PATTERN|AZURE: {'mtu': '1500', 'small_packet_percentage': '100'} + 3. For lossless dynamic PGs, remove the explicit referencing buffer profiles + Before: BUFFER_PG||3-4: {'profile': 'BUFFER_PROFILE|pg_lossless___profile'} + After: BUFFER_PG||3-4: {'profile': 'NULL'} + ''' + # Migrate BUFFER_PROFILEs, removing dynamically generated profiles + dynamic_profile = self.configDB.get_table('BUFFER_PROFILE') + profile_pattern = 'pg_lossless_([0-9]*000)_([0-9]*m)_profile' + for name, info in dynamic_profile.iteritems(): + m = re.search(profile_pattern, name) + if not m: + continue + speed = m.group(1) + cable_length = m.group(2) + if speed in speed_list and cable_length in cable_len_list and info['dynamic_th'] == default_dynamic_th: + self.configDB.set_entry('BUFFER_PROFILE', name, None) + log.log_info("Lossless profile {} has been removed".format(name)) + + # Insert other tables required for dynamic buffer calculation + self.configDB.set_entry('DEFAULT_LOSSLESS_BUFFER_PARAMETER', 'AZURE', {'default_dynamic_th': default_dynamic_th}) + self.configDB.set_entry('LOSSLESS_TRAFFIC_PATTERN', 'AZURE', { + 'mtu': '1500', 'small_packet_percentage': '100'}) + + # Migrate BUFFER_PGs, removing the explicit designated profiles + buffer_pgs = self.configDB.get_table('BUFFER_PG') + ports = self.configDB.get_table('PORT') + all_cable_lengths = self.configDB.get_table('CABLE_LENGTH') + if not buffer_pgs or not ports or not all_cable_lengths: + log.log_notice("At lease one of tables BUFFER_PG, PORT and CABLE_LENGTH hasn't been defined, skip following mitration") + return True + + cable_lengths = all_cable_lengths[all_cable_lengths.keys()[0]] + for name, profile in buffer_pgs.iteritems(): + # do the db migration + port, pg = name + if pg != '3-4': + continue + try: + m = re.search(profile_pattern, profile['profile'][1:-1].split('|')[1]) + except Exception: + continue + if not m: + continue + speed = m.group(1) + cable_length = m.group(2) + try: + if speed == ports[port]['speed'] and cable_length == cable_lengths[port]: + self.configDB.set_entry('BUFFER_PG', name, {'profile': 'NULL'}) + except Exception: + continue + + return True + + def prepare_dynamic_buffer_for_warm_reboot(self, buffer_pools = None, buffer_profiles = None, buffer_pgs = None): + ''' + This is the very first warm reboot of buffermgrd (dynamic) if the system reboot from old image by warm-reboot + In this case steps need to be taken to get buffermgrd prepared (for warm reboot) + + During warm reboot, buffer tables should be installed in the first place. + However, it isn't able to achieve that when system is warm-rebooted from an old image + without dynamic buffer supported, because the buffer info wasn't in the APPL_DB in the old image. + The solution is to copy that info from CONFIG_DB into APPL_DB in db_migrator. + During warm-reboot, db_migrator adjusts buffer info in CONFIG_DB by removing some fields + according to requirement from dynamic buffer calculation. + The buffer info before that adjustment needs to be copied to APPL_DB. + + 1. set WARM_RESTART_TABLE|buffermgrd as {restore_count: 0} + 2. Copy the following tables from CONFIG_DB into APPL_DB in case of warm reboot + The separator in fields that reference objects in other table needs to be updated from '|' to ':' + - BUFFER_POOL + - BUFFER_PROFILE, separator updated for field 'pool' + - BUFFER_PG, separator updated for field 'profile' + - BUFFER_QUEUE, separator updated for field 'profile + - BUFFER_PORT_INGRESS_PROFILE_LIST, separator updated for field 'profile_list' + - BUFFER_PORT_EGRESS_PROFILE_LIST, separator updated for field 'profile_list' + + ''' + warmreboot_state = self.stateDB.get(self.stateDB.STATE_DB, 'WARM_RESTART_ENABLE_TABLE|system', 'enable') + if warmreboot_state == 'true': + self.stateDB.set(self.stateDB.STATE_DB, 'WARM_RESTART_TABLE|buffermgrd', 'restore_count', '0') + log.log_notice("This is the very first run of buffermgrd (dynamc), prepare info requred from warm reboot") + else: + return + + buffer_table_list = [ + ('BUFFER_POOL', buffer_pools, None), + ('BUFFER_PROFILE', buffer_profiles, 'pool'), + ('BUFFER_PG', buffer_pgs, 'profile'), + ('BUFFER_QUEUE', None, 'profile'), + ('BUFFER_PORT_INGRESS_PROFILE_LIST', None, 'profile_list'), + ('BUFFER_PORT_EGRESS_PROFILE_LIST', None, 'profile_list') + ] + + for pair in buffer_table_list: + keys_copied = [] + keys_ignored = [] + table_name, entries, reference_field_name = pair + app_table_name = table_name + "_TABLE" + if not entries: + entries = self.configDB.get_table(table_name) + for key, items in entries.iteritems(): + # copy items to appl db + if reference_field_name: + confdb_ref = items.get(reference_field_name) + if not confdb_ref: + keys_ignored.append(key) + continue + items_referenced = confdb_ref.split(',') + appdb_ref = "" + first_item = True + for item in items_referenced: + if first_item: + first_item = False + else: + appdb_ref += ',' + subitems = item.split('|') + first_key = True + for subitem in subitems: + if first_key: + appdb_ref += subitem + '_TABLE' + first_key = False + else: + appdb_ref += ':' + subitem + + items[reference_field_name] = appdb_ref + keys_copied.append(key) + if type(key) is tuple: + appl_db_key = app_table_name + ':' + ':'.join(key) + else: + appl_db_key = app_table_name + ':' + key + for field, data in items.iteritems(): + self.appDB.set(self.appDB.APPL_DB, appl_db_key, field, data) + + if keys_copied: + log.log_info("The following items in table {} in CONFIG_DB have been copied to APPL_DB: {}".format(table_name, keys_copied)) + if keys_ignored: + log.log_info("The following items in table {} in CONFIG_DB have been ignored: {}".format(table_name, keys_copied)) + def version_unknown(self): """ version_unknown tracks all SONiC versions that doesn't have a version @@ -225,6 +381,30 @@ def version_1_0_4(self): """ log.log_info('Handling version_1_0_4') + # Check ASIC type, if Mellanox platform then need DB migration + if self.asic_type == "mellanox": + speed_list = ['1000', '10000', '25000', '40000', '50000', '100000', '200000', '400000'] + cable_len_list = ['5m', '40m', '300m'] + buffer_pools = self.configDB.get_table('BUFFER_POOL') + buffer_profiles = self.configDB.get_table('BUFFER_PROFILE') + buffer_pgs = self.configDB.get_table('BUFFER_PG') + if self.mellanox_buffer_migrator.mlnx_migrate_buffer_pool_size('version_1_0_4', 'version_1_0_5') \ + and self.mellanox_buffer_migrator.mlnx_migrate_buffer_profile('version_1_0_4', 'version_1_0_5') \ + and self.migrate_config_db_buffer_tables_for_dynamic_calculation(speed_list, cable_len_list, '0') \ + and self.prepare_dynamic_buffer_for_warm_reboot(buffer_pools, buffer_profiles, buffer_pgs): + self.set_version('version_1_0_5') + else: + self.prepare_dynamic_buffer_for_warm_reboot() + self.set_version('version_1_0_5') + + return 'version_1_0_5' + + def version_1_0_5(self): + """ + Current latest version. Nothing to do here. + """ + log.log_info('Handling version_1_0_5') + return None def get_version(self): diff --git a/scripts/fast-reboot b/scripts/fast-reboot index 7d7b8b832f..287239daed 100755 --- a/scripts/fast-reboot +++ b/scripts/fast-reboot @@ -242,7 +242,8 @@ function backup_database() for _, k in ipairs(redis.call('keys', '*')) do if not string.match(k, 'FDB_TABLE|') and not string.match(k, 'WARM_RESTART_TABLE|') \ and not string.match(k, 'MIRROR_SESSION_TABLE|') \ - and not string.match(k, 'WARM_RESTART_ENABLE_TABLE|') then + and not string.match(k, 'WARM_RESTART_ENABLE_TABLE|') \ + and not string.match(k, 'BUFFER_MAX_PARAM_TABLE|') then redis.call('del', k) end end diff --git a/scripts/mellanox_buffer_migrator.py b/scripts/mellanox_buffer_migrator.py index 90abf0d821..6a8726dee5 100644 --- a/scripts/mellanox_buffer_migrator.py +++ b/scripts/mellanox_buffer_migrator.py @@ -138,7 +138,7 @@ def __init__(self, configDB): }, "version_1_0_4": { # version 1.0.4 is introduced for updating the buffer settings - "pool_configuration_list": ["spc1_t0_pool", "spc1_t1_pool", "spc2_t0_pool", "spc2_t1_pool", "spc2_3800_t0_pool", "spc2_3800_t1_pool"], + "pool_configuration_list": ["spc1_t0_pool", "spc1_t1_pool", "spc2_t0_pool", "spc2_t1_pool", "spc2_3800_t0_pool", "spc2_3800_t1_pool", "spc3_t0_pool", "spc3_t1_pool"], # Buffer pool info for normal mode "buffer_pool_list" : ['ingress_lossless_pool', 'ingress_lossy_pool', 'egress_lossless_pool', 'egress_lossy_pool'], @@ -169,6 +169,16 @@ def __init__(self, configDB): "egress_lossless_pool": { "size": "34287552", "type": "egress", "mode": "dynamic" }, "egress_lossy_pool": {"size": "12457984", "type": "egress", "mode": "dynamic" } }, + # SPC3 is used only when migrating from 1.0.4 to 1.0.5 + "spc3_t0_pool": {"ingress_lossless_pool": { "size": "26451968", "type": "ingress", "mode": "dynamic" }, + "ingress_lossy_pool": { "size": "26451968", "type": "ingress", "mode": "dynamic" }, + "egress_lossless_pool": { "size": "60817392", "type": "egress", "mode": "dynamic" }, + "egress_lossy_pool": {"size": "26451968", "type": "egress", "mode": "dynamic" } }, + "spc3_t1_pool": {"ingress_lossless_pool": { "size": "20627456", "type": "ingress", "mode": "dynamic" }, + "ingress_lossy_pool": { "size": "20627456", "type": "ingress", "mode": "dynamic" }, + "egress_lossless_pool": { "size": "60817392", "type": "egress", "mode": "dynamic" }, + "egress_lossy_pool": {"size": "20627456", "type": "egress", "mode": "dynamic" } }, + # Lossless headroom info "spc1_headroom": {"pg_lossless_10000_5m_profile": {"size": "49152", "xon":"19456"}, "pg_lossless_25000_5m_profile": {"size": "49152", "xon":"19456"}, @@ -225,7 +235,37 @@ def __init__(self, configDB): "egress_lossless_profile": {"dynamic_th": "7", "pool": "[BUFFER_POOL|egress_lossless_pool]", "size": "0"}, "egress_lossy_profile": {"dynamic_th": "7", "pool": "[BUFFER_POOL|egress_lossy_pool]", "size": "9216"}, "q_lossy_profile": {"dynamic_th": "3", "pool": "[BUFFER_POOL|egress_lossy_pool]", "size": "0"}} - } + }, + "version_1_0_5": { + # version 1.0.5 is introduced for dynamic buffer calculation + # + "pool_configuration_list": ["spc1_pool", "spc2_pool", "spc3_pool"], + "pool_mapped_from_old_version": { + "spc1_t0_pool": "spc1_pool", + "spc1_t1_pool": "spc1_pool", + "spc2_t0_pool": "spc2_pool", + "spc2_t1_pool": "spc2_pool", + "spc2_3800_t0_pool": "spc2_pool", + "spc2_3800_t1_pool": "spc2_pool", + "spc3_t0_pool": "spc3_pool", + "spc3_t1_pool": "spc3_pool" + }, + + # Buffer pool info for normal mode + "buffer_pool_list" : ['ingress_lossless_pool', 'ingress_lossy_pool', 'egress_lossless_pool', 'egress_lossy_pool'], + "spc1_pool": {"ingress_lossless_pool": { "type": "ingress", "mode": "dynamic" }, + "ingress_lossy_pool": { "type": "ingress", "mode": "dynamic" }, + "egress_lossless_pool": { "size": "13945824", "type": "egress", "mode": "dynamic" }, + "egress_lossy_pool": {"type": "egress", "mode": "dynamic" } }, + "spc2_pool": {"ingress_lossless_pool": { "type": "ingress", "mode": "dynamic" }, + "ingress_lossy_pool": { "type": "ingress", "mode": "dynamic" }, + "egress_lossless_pool": { "size": "34287552", "type": "egress", "mode": "dynamic" }, + "egress_lossy_pool": { "type": "egress", "mode": "dynamic" } }, + "spc3_pool": {"ingress_lossless_pool": { "type": "ingress", "mode": "dynamic" }, + "ingress_lossy_pool": { "type": "ingress", "mode": "dynamic" }, + "egress_lossless_pool": { "size": "60817392", "type": "egress", "mode": "dynamic" }, + "egress_lossy_pool": {"type": "egress", "mode": "dynamic" } } + } } def mlnx_default_buffer_parameters(self, db_version, table): @@ -247,6 +287,8 @@ def mlnx_migrate_buffer_pool_size(self, old_version, new_version): """ To migrate buffer pool configuration """ + buffer_pool_conf_in_db = {} + # Buffer pools defined in old version old_default_buffer_pools = self.mlnx_default_buffer_parameters(old_version, "buffer_pool_list") @@ -274,12 +316,16 @@ def mlnx_migrate_buffer_pool_size(self, old_version, new_version): return False new_config_name = None + pool_mapping = self.mlnx_default_buffer_parameters(new_version, "pool_mapped_from_old_version") for old_config_name in old_pool_configuration_list: old_config = self.mlnx_default_buffer_parameters(old_version, old_config_name) log.log_info("Checking old pool configuration {}".format(old_config_name)) if buffer_pool_conf_in_db == old_config: - new_config_name = old_config_name - log.log_info("Old buffer pool configuration {} will be migrate to new one".format(old_config_name)) + if pool_mapping: + new_config_name = pool_mapping[old_config_name] + else: + new_config_name = old_config_name + log.log_info("Old buffer pool configuration {} will be migrate to new one {}".format(old_config_name, new_config_name)) break if not new_config_name: diff --git a/scripts/mmuconfig b/scripts/mmuconfig old mode 100644 new mode 100755 index e77a46d9de..3ccae34a60 --- a/scripts/mmuconfig +++ b/scripts/mmuconfig @@ -31,35 +31,61 @@ BUFFER_PROFILE_FIELDS = { class MmuConfig(object): - def __init__(self, verbose): + def __init__(self, verbose, config): self.verbose = verbose + self.config = config # Set up db connections - self.db = swsssdk.ConfigDBConnector() - self.db.connect() + if self.config: + self.db = swsssdk.ConfigDBConnector() + self.db.connect() + else: + self.db = swsssdk.SonicV2Connector(host='127.0.0.1') + self.db.connect(self.db.STATE_DB, False) - def list(self): - buf_pools = self.db.get_table(BUFFER_POOL_TABLE_NAME) - for pool_name, pool_data in buf_pools.items(): - config = [] - - print("Pool: " + pool_name) - for field, value in pool_data.items(): - config.append([field, value]) - print(tabulate.tabulate(config) + "\n") - if self.verbose: - print("Total pools: %d\n\n" % len(buf_pools)) + def get_table(self, tablename): + if self.config: + return self.db.get_table(tablename) - buf_profs = self.db.get_table(BUFFER_PROFILE_TABLE_NAME) - for prof_name, prof_data in buf_profs.items(): - config = [] + entries = {} + keys = self.db.keys(self.db.STATE_DB, tablename + '*') - print("Profile: " + prof_name) - for field, value in prof_data.items(): - config.append([field, value]) - print(tabulate.tabulate(config) + "\n") - if self.verbose: - print("Total profiles: %d" % len(buf_profs)) + if not keys: + return None + + for key in keys: + entries[key.split('|')[1]] = self.db.get_all(self.db.STATE_DB, key) + + return entries + + def list(self): + buf_pools = self.get_table(BUFFER_POOL_TABLE_NAME) + if buf_pools: + for pool_name, pool_data in buf_pools.items(): + config = [] + + print("Pool: " + pool_name) + for field, value in pool_data.items(): + config.append([field, value]) + print(tabulate.tabulate(config) + "\n") + if self.verbose: + print("Total pools: %d\n\n" % len(buf_pools)) + else: + print("No buffer pool information available") + + buf_profs = self.get_table(BUFFER_PROFILE_TABLE_NAME) + if buf_profs: + for prof_name, prof_data in buf_profs.items(): + config = [] + + print("Profile: " + prof_name) + for field, value in prof_data.items(): + config.append([field, value]) + print(tabulate.tabulate(config) + "\n") + if self.verbose: + print("Total profiles: %d" % len(buf_profs)) + else: + print("No buffer profile information available") def set(self, profile, field_alias, value): if os.geteuid() != 0: @@ -82,23 +108,33 @@ class MmuConfig(object): self.db.mod_entry(BUFFER_PROFILE_TABLE_NAME, profile, {field: value}) -def main(): - parser = argparse.ArgumentParser(description='Show and change: mmu configuration', - formatter_class=argparse.RawTextHelpFormatter) +def main(config): + if config: + parser = argparse.ArgumentParser(description='Show and change: mmu configuration', + version='1.0.0', + formatter_class=argparse.RawTextHelpFormatter) + + parser.add_argument('-l', '--list', action='store_true', help='show mmu configuration') + parser.add_argument('-p', '--profile', type=str, help='specify buffer profile name', default=None) + parser.add_argument('-a', '--alpha', type=str, help='set n for dyanmic threshold alpha 2^(n)', default=None) + parser.add_argument('-v', '--version', action='version', version='%(prog)s 1.0') + else: + parser = argparse.ArgumentParser(description='Show buffer state', + version='1.0.0', + formatter_class=argparse.RawTextHelpFormatter) + + parser.add_argument('-l', '--list', action='store_true', help='show buffer state') + parser.add_argument('-v', '--version', action='version', version='%(prog)s 1.0') - parser.add_argument('-l', '--list', action='store_true', help='show mmu configuration') - parser.add_argument('-p', '--profile', type=str, help='specify buffer profile name', default=None) - parser.add_argument('-a', '--alpha', type=str, help='set n for dyanmic threshold alpha 2^(n)', default=None) - parser.add_argument('-v', '--version', action='version', version='%(prog)s 1.0') parser.add_argument('-vv', '--verbose', action='store_true', help='verbose output', default=False) args = parser.parse_args() try: - mmu_cfg = MmuConfig(args.verbose) + mmu_cfg = MmuConfig(args.verbose, config) if args.list: mmu_cfg.list() - elif args.profile: + elif config and args.profile: if args.alpha: mmu_cfg.set(args.profile, "alpha", args.alpha) else: @@ -110,4 +146,7 @@ def main(): sys.exit(1) if __name__ == "__main__": - main() + if sys.argv[0].split('/')[-1] == "mmuconfig": + main(True) + else: + main(False) diff --git a/setup.py b/setup.py index 08ae72959d..7614bd12f7 100644 --- a/setup.py +++ b/setup.py @@ -66,6 +66,7 @@ 'scripts/aclshow', 'scripts/asic_config_check', 'scripts/boot_part', + 'scripts/buffershow', 'scripts/coredump-compress', 'scripts/configlet', 'scripts/db_migrator.py', diff --git a/show/main.py b/show/main.py old mode 100755 new mode 100644 index 60a3e6b683..5dc30698b0 --- a/show/main.py +++ b/show/main.py @@ -1430,6 +1430,7 @@ def boot(): click.echo(proc.stdout.read()) +# # 'mmu' command ("show mmu") # @cli.command('mmu') @@ -1439,6 +1440,35 @@ def mmu(): proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True, text=True) click.echo(proc.stdout.read()) +# +# 'buffer' command ("show buffer") +# +@cli.group(cls=clicommon.AliasedGroup) +def buffer(): + """Show buffer information""" + pass + +# +# 'configuration' command ("show buffer command") +# +@buffer.command() +def configuration(): + """show buffer configuration""" + cmd = "mmuconfig -l" + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True) + click.echo(proc.stdout.read()) + +# +# 'information' command ("show buffer state") +# +@buffer.command() +def information(): + """show buffer information""" + cmd = "buffershow -l" + proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True) + click.echo(proc.stdout.read()) + + # # 'line' command ("show line") # diff --git a/tests/buffer_test.py b/tests/buffer_test.py new file mode 100644 index 0000000000..3cb42fd2a6 --- /dev/null +++ b/tests/buffer_test.py @@ -0,0 +1,70 @@ +import os +import sys +from click.testing import CliRunner +from unittest import TestCase +from swsssdk import ConfigDBConnector + +import mock_tables.dbconnector + +import config.main as config +from utilities_common.db import Db + +class TestBuffer(object): + @classmethod + def setup_class(cls): + os.environ['UTILITIES_UNIT_TESTING'] = "1" + print("SETUP") + + def setUp(self): + self.runner = CliRunner() + self.config_db = ConfigDBConnector() + self.config_db.connect() + self.obj = {'db': self.config_db} + + def test_config_buffer_profile_headroom(self): + runner = CliRunner() + result = runner.invoke(config.config.commands["buffer-profile"].commands["add"], + ["testprofile", "-dynamic_th", "3", "-xon", "18432", "-xoff", "32768"]) + print(result.exit_code) + print(result.output) + assert result.exit_code == 0 + + def test_config_buffer_profile_dynamic_th(self): + runner = CliRunner() + result = runner.invoke(config.config.commands["buffer-profile"].commands["add"], + ["testprofile", "-dynamic_th", "3"]) + print(result.exit_code) + print(result.output) + assert result.exit_code == 0 + + def test_config_buffer_profile_add_existing(self): + runner = CliRunner() + result = runner.invoke(config.config.commands["buffer-profile"].commands["add"], + ["headroom_profile", "-dynamic_th", "3"]) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Profile headroom_profile already exist" in result.output + + def test_config_buffer_profile_set_non_existing(self): + runner = CliRunner() + result = runner.invoke(config.config.commands["buffer-profile"].commands["set"], + ["non_existing_profile", "-dynamic_th", "3"]) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Profile non_existing_profile doesn't exist" in result.output + + def test_config_buffer_profile_add_headroom_to_dynamic_profile(self): + runner = CliRunner() + result = runner.invoke(config.config.commands["buffer-profile"].commands["set"], + ["alpha_profile", "-dynamic_th", "3", "-xon", "18432", "-xoff", "32768"]) + print(result.exit_code) + print(result.output) + assert result.exit_code != 0 + assert "Can't change profile alpha_profile from dynamically calculating headroom to non-dynamically one" in result.output + + @classmethod + def teardown_class(cls): + os.environ['UTILITIES_UNIT_TESTING'] = "0" + print("TEARDOWN") diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index 4a3c21e29f..982a512b6a 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -1332,5 +1332,49 @@ }, "PEER_SWITCH|sonic-switch" : { "address_ipv4": "10.2.2.2" + }, + "BUFFER_POOL|egress_lossless_pool": { + "mode": "dynamic", + "size": "13945824", + "type": "egress" + }, + "BUFFER_POOL|egress_lossy_pool": { + "mode": "dynamic", + "type": "egress" + }, + "BUFFER_POOL|ingress_lossless_pool": { + "mode": "dynamic", + "type": "ingress" + }, + "BUFFER_POOL|ingress_lossy_pool": { + "mode": "dynamic", + "type": "ingress" + }, + "BUFFER_PROFILE|ingress_lossy_profile": { + "dynamic_th": "3", + "pool": "[BUFFER_POOL|ingress_lossy_pool]", + "size": "0" + }, + "BUFFER_PROFILE|headroom_profile": { + "dynamic_th": "0", + "pool": "[BUFFER_POOL|ingress_lossless_pool]", + "xon": "18432", + "xoff": "32768", + "size": "51200" + }, + "BUFFER_PROFILE|alpha_profile": { + "dynamic_th": "0" + }, + "BUFFER_PG|Ethernet0|3-4": { + "profile": "NULL" + }, + "BUFFER_PG|Ethernet0|0": { + "profile": "[BUFFER_PROFILE|ingress_lossy_profile]" + }, + "PORT_QOS_MAP|Ethernet0": { + "pfc_enable": "3,4" + }, + "DEFAULT_LOSSLESS_BUFFER_PARAMETER|AZURE": { + "default_dynamic_th": "0" } } From 06854483a3118d39afd64da46f4f07a59b684112 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Tue, 29 Sep 2020 01:24:10 +0000 Subject: [PATCH 2/5] Update the upgrading behavior according to the discussion with MSFT 1. If all the buffer configuration aligns the default, use dynamic buffer calculation mode. Otherwise, use the traditional mode 2. Dynamic mode is adopted in switches newly installed from scratch and traditional mode in switches installed from minigraph This is done by: - introducing the option --no-dynamic-buffer in "config qos reload" to designate whether the dynamic or traditional mode is used - introducing a new filed "buffer_model" in DEVICE_METADATA|localhost to store which buffer model currently is used - updating db_migrator accordingly Signed-off-by: Stephen Sun --- config/main.py | 116 ++++++++++++++++------ scripts/db_migrator.py | 106 ++++++++++++++------ scripts/mellanox_buffer_migrator.py | 146 ++++++++++++++++++++++------ tests/buffer_test.py | 20 ++-- tests/config_test.py | 2 +- 5 files changed, 288 insertions(+), 102 deletions(-) mode change 100644 => 100755 scripts/mellanox_buffer_migrator.py diff --git a/config/main.py b/config/main.py index 8ad5d0b8d3..90967efd00 100644 --- a/config/main.py +++ b/config/main.py @@ -1195,7 +1195,7 @@ def load_minigraph(db, no_service_restart): clicommon.run_command("acl-loader update full /etc/sonic/acl.json", display_cmd=True) # generate QoS and Buffer configs - clicommon.run_command("config qos reload", display_cmd=True) + clicommon.run_command("config qos reload --no-dynamic-buffer", display_cmd=True) # Write latest db version string into db db_migrator='/usr/local/bin/db_migrator.py' @@ -1624,8 +1624,20 @@ def clear(): log.log_info("'qos clear' executing...") _clear_qos() +def _update_buffer_calculation_model(config_db, model): + """Update the buffer calculation model into CONFIG_DB""" + buffer_model_changed = False + device_metadata = config_db.get_entry('DEVICE_METADATA', 'localhost') + if device_metadata.get('buffer_model') != model: + buffer_model_changed = True + device_metadata['buffer_model'] = model + config_db.set_entry('DEVICE_METADATA', 'localhost', device_metadata) + return buffer_model_changed + @qos.command('reload') -def reload(): +@click.pass_context +@click.option('--no-dynamic-buffer', is_flag=True, help="Disable dynamic buffer calculation") +def reload(ctx, no_dynamic_buffer): """Reload QoS configuration""" log.log_info("'qos reload' executing...") _clear_qos() @@ -1636,9 +1648,13 @@ def reload(): if multi_asic.get_num_asics() > 1: namespace_list = multi_asic.get_namespaces_from_linux() + buffer_model_updated = False + vendors_supporting_dynamic_buffer = ["mellanox"] + for ns in namespace_list: if ns is DEFAULT_NAMESPACE: asic_id_suffix = "" + config_db = ConfigDBConnector() else: asic_id = multi_asic.get_asic_id_from_name(ns) if asic_id is None: @@ -1650,7 +1666,19 @@ def reload(): raise click.Abort() asic_id_suffix = str(asic_id) - buffer_template_file = os.path.join(hwsku_path, asic_id_suffix, "buffers.json.j2") + config_db = ConfigDBConnector( + use_unix_socket_path=True, namespace=ns + ) + + config_db.connect() + + if not no_dynamic_buffer and asic_type in vendors_supporting_dynamic_buffer: + buffer_template_file = os.path.join(hwsku_path, asic_id_suffix, "buffers_dynamic.json.j2") + buffer_model_updated |= _update_buffer_calculation_model(config_db, "dynamic") + else: + buffer_template_file = os.path.join(hwsku_path, asic_id_suffix, "buffers.json.j2") + if asic_type in vendors_supporting_dynamic_buffer: + buffer_model_updated |= _update_buffer_calculation_model(config_db, "traditional") if os.path.isfile(buffer_template_file): qos_template_file = os.path.join(hwsku_path, asic_id_suffix, "qos.json.j2") if os.path.isfile(qos_template_file): @@ -1675,6 +1703,14 @@ def reload(): buffer_template_file ), fg="yellow") + if buffer_model_updated: + print "Buffer calculation model updated, restarting swss is required to take effect" + +def is_dynamic_buffer_enabled(config_db): + """Return whether the current system supports dynamic buffer calculation""" + device_metadata = config_db.get_entry('DEVICE_METADATA', 'localhost') + return 'dynamic' == device_metadata.get('buffer_model') + # # 'warm_restart' group ('config warm_restart ...') # @@ -2661,7 +2697,9 @@ def adjust_pfc_enable(ctx, interface_name, pg_map, add): @click.pass_context def buffer(ctx): """Set or clear buffer configuration""" - pass + config_db = ctx.obj["config_db"] + if not is_dynamic_buffer_enabled(config_db): + ctx.fail("This command can only be executed on a system with dynamic buffer enabled") # @@ -2734,6 +2772,9 @@ def cable_length(ctx, interface_name, length): """Set lossless PGs for the interface""" config_db = ctx.obj["config_db"] + if not is_dynamic_buffer_enabled(config_db): + ctx.fail("This command can only be supported on a system with dynamic buffer enabled") + # Check whether port is legal ports = config_db.get_entry("PORT", interface_name) if not ports: @@ -3336,19 +3377,31 @@ def priority(ctx, interface_name, priority, status): @config.group(cls=clicommon.AbbreviationGroup) @click.pass_context -def buffer_profile(ctx): +def buffer(ctx): """Configure buffer_profile""" + config_db = ConfigDBConnector() + config_db.connect() + + if not is_dynamic_buffer_enabled(config_db): + ctx.fail("This command can only be supported on a system with dynamic buffer enabled") + + +@buffer.group(cls=clicommon.AbbreviationGroup) +@click.pass_context +def profile(ctx): + """Configure buffer profile""" + pass + -@buffer_profile.command('add') -#@click.option('-profile', metavar='', type=str, help="Profile name", required=True) +@profile.command('add') @click.argument('profile', metavar='', required=True) -@click.option('-xon', metavar='', type=int, help="Set xon threshold") -@click.option('-xoff', metavar='', type=int, help="Set xoff threshold") -@click.option('-headroom', metavar='', type=int, help="Set reserved headroom size") -@click.option('-dynamic_th', metavar='', type=str, help="Set dynamic threshold") -@click.option('-pool', metavar='', type=str, help="Buffer pool") +@click.option('--xon', metavar='', type=int, help="Set xon threshold") +@click.option('--xoff', metavar='', type=int, help="Set xoff threshold") +@click.option('--size', metavar='', type=int, help="Set reserved size size") +@click.option('--dynamic_th', metavar='', type=str, help="Set dynamic threshold") +@click.option('--pool', metavar='', type=str, help="Buffer pool") @click.pass_context -def add_profile(ctx, profile, xon, xoff, headroom, dynamic_th, pool): +def add_profile(ctx, profile, xon, xoff, size, dynamic_th, pool): """Add or modify a buffer profile""" config_db = ConfigDBConnector() config_db.connect() @@ -3357,19 +3410,18 @@ def add_profile(ctx, profile, xon, xoff, headroom, dynamic_th, pool): if profile_entry: ctx.fail("Profile {} already exist".format(profile)) - update_profile(ctx, config_db, profile, xon, xoff, headroom, dynamic_th, pool) + update_profile(ctx, config_db, profile, xon, xoff, size, dynamic_th, pool) -@buffer_profile.command('set') -#@click.option('-profile', metavar='', type=str, help="Profile name", required=True) +@profile.command('set') @click.argument('profile', metavar='', required=True) -@click.option('-xon', metavar='', type=int, help="Set xon threshold") -@click.option('-xoff', metavar='', type=int, help="Set xoff threshold") -@click.option('-headroom', metavar='', type=int, help="Set reserved headroom size") -@click.option('-dynamic_th', metavar='', type=str, help="Set dynamic threshold") -@click.option('-pool', metavar='', type=str, help="Buffer pool") +@click.option('--xon', metavar='', type=int, help="Set xon threshold") +@click.option('--xoff', metavar='', type=int, help="Set xoff threshold") +@click.option('--size', metavar='', type=int, help="Set reserved size size") +@click.option('--dynamic_th', metavar='', type=str, help="Set dynamic threshold") +@click.option('--pool', metavar='', type=str, help="Buffer pool") @click.pass_context -def set_profile(ctx, profile, xon, xoff, headroom, dynamic_th, pool): +def set_profile(ctx, profile, xon, xoff, size, dynamic_th, pool): """Add or modify a buffer profile""" config_db = ConfigDBConnector() config_db.connect() @@ -3381,10 +3433,10 @@ def set_profile(ctx, profile, xon, xoff, headroom, dynamic_th, pool): if not 'xoff' in profile_entry.keys() and xoff: ctx.fail("Can't change profile {} from dynamically calculating headroom to non-dynamically one".format(profile)) - update_profile(ctx, config_db, profile, xon, xoff, headroom, dynamic_th, pool, profile_entry) + update_profile(ctx, config_db, profile, xon, xoff, size, dynamic_th, pool, profile_entry) -def update_profile(ctx, config_db, profile_name, xon, xoff, headroom, dynamic_th, pool, profile_entry = None): +def update_profile(ctx, config_db, profile_name, xon, xoff, size, dynamic_th, pool, profile_entry = None): params = {} if profile_entry: params = profile_entry @@ -3408,23 +3460,23 @@ def update_profile(ctx, config_db, profile_name, xon, xoff, headroom, dynamic_th else: xoff = params.get('xoff') - if headroom: - params['size'] = headroom + if size: + params['size'] = size dynamic_calculate = False if xon and not xoff: - xoff = int(headroom) - int (xon) + xoff = int(size) - int (xon) params['xoff'] = xoff elif not dynamic_calculate: if xon and xoff: - headroom = int(xon) + int(xoff) - params['size'] = headroom + size = int(xon) + int(xoff) + params['size'] = size else: - ctx.fail("Either both xon and xoff or headroom should be provided") + ctx.fail("Either both xon and xoff or size should be provided") if dynamic_calculate: params['headroom_type'] = 'dynamic' if not dynamic_th: - ctx.fail("Either headroom information (xon, xoff, headroom) or dynamic_th needs to be provided") + ctx.fail("Either size information (xon, xoff, size) or dynamic_th needs to be provided") if dynamic_th: params['dynamic_th'] = dynamic_th @@ -3443,7 +3495,7 @@ def update_profile(ctx, config_db, profile_name, xon, xoff, headroom, dynamic_th config_db.set_entry("BUFFER_PROFILE", (profile_name), params) -@buffer_profile.command('remove') +@profile.command('remove') @click.argument('profile', metavar='', required=True) @click.pass_context def remove_profile(ctx, profile): diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py index b67851b795..e3fd033591 100755 --- a/scripts/db_migrator.py +++ b/scripts/db_migrator.py @@ -31,7 +31,7 @@ def __init__(self, namespace, socket=None): none-zero values. build: sequentially increase within a minor version domain. """ - self.CURRENT_VERSION = 'version_1_0_5' + self.CURRENT_VERSION = 'version_2_0_0' self.TABLE_NAME = 'VERSIONS' self.TABLE_KEY = 'DATABASE' @@ -161,13 +161,19 @@ def migrate_copp_table(self): for copp_key in keys: self.appDB.delete(self.appDB.APPL_DB, copp_key) - def migrate_config_db_buffer_tables_for_dynamic_calculation(self, speed_list, cable_len_list, default_dynamic_th): + def migrate_config_db_buffer_tables_for_dynamic_calculation(self, speed_list, cable_len_list, default_dynamic_th, default_lossless_profiles, abandon_method, append_item_method): ''' Migrate buffer tables to dynamic calculation mode parameters @speed_list - list of speed supported @cable_len_list - list of cable length supported @default_dynamic_th - default dynamic th + @default_lossless_profiles - default lossless profiles from the previous image + @abandon_method - a function which is called to abandon the migration and keep the current configuration + if the current one doesn't match the default one + @append_item_method - a function which is called to append an item to the list of pending commit items + any update to buffer configuration will be pended and won't be applied until + all configuration is checked and aligns with the default one 1. Buffer profiles for lossless PGs in BUFFER_PROFILE table will be removed if their names have the convention of pg_lossless___profile @@ -189,14 +195,17 @@ def migrate_config_db_buffer_tables_for_dynamic_calculation(self, speed_list, ca continue speed = m.group(1) cable_length = m.group(2) - if speed in speed_list and cable_length in cable_len_list and info['dynamic_th'] == default_dynamic_th: - self.configDB.set_entry('BUFFER_PROFILE', name, None) - log.log_info("Lossless profile {} has been removed".format(name)) - - # Insert other tables required for dynamic buffer calculation - self.configDB.set_entry('DEFAULT_LOSSLESS_BUFFER_PARAMETER', 'AZURE', {'default_dynamic_th': default_dynamic_th}) - self.configDB.set_entry('LOSSLESS_TRAFFIC_PATTERN', 'AZURE', { - 'mtu': '1500', 'small_packet_percentage': '100'}) + if speed in speed_list and cable_length in cable_len_list: + log.log_info("current profile {} {}".format(name, info)) + log.log_info("default profile {} {}".format(name, default_lossless_profiles.get(name))) + default_profile = default_lossless_profiles.get(name); + if info.get("xon") == default_profile.get("xon") and info.get("size") == default_profile.get("size") and info.get('dynamic_th') == default_dynamic_th: + append_item_method(('BUFFER_PROFILE', name, None)) + log.log_info("Lossless profile {} has been removed".format(name)) + else: + log.log_notice("Lossless profile {} doesn't match the default configuration, keep using traditional buffer calculation mode") + abandon_method() + return True # Migrate BUFFER_PGs, removing the explicit designated profiles buffer_pgs = self.configDB.get_table('BUFFER_PG') @@ -204,6 +213,7 @@ def migrate_config_db_buffer_tables_for_dynamic_calculation(self, speed_list, ca all_cable_lengths = self.configDB.get_table('CABLE_LENGTH') if not buffer_pgs or not ports or not all_cable_lengths: log.log_notice("At lease one of tables BUFFER_PG, PORT and CABLE_LENGTH hasn't been defined, skip following mitration") + abandon_method() return True cable_lengths = all_cable_lengths[all_cable_lengths.keys()[0]] @@ -213,7 +223,8 @@ def migrate_config_db_buffer_tables_for_dynamic_calculation(self, speed_list, ca if pg != '3-4': continue try: - m = re.search(profile_pattern, profile['profile'][1:-1].split('|')[1]) + profile_name = profile['profile'][1:-1].split('|')[1] + m = re.search(profile_pattern, profile_name) except Exception: continue if not m: @@ -222,10 +233,22 @@ def migrate_config_db_buffer_tables_for_dynamic_calculation(self, speed_list, ca cable_length = m.group(2) try: if speed == ports[port]['speed'] and cable_length == cable_lengths[port]: - self.configDB.set_entry('BUFFER_PG', name, {'profile': 'NULL'}) + append_item_method(('BUFFER_PG', name, {'profile': 'NULL'})) + else: + log.log_notice("Lossless PG profile {} for port {} doesn't match its speed {} or cable length {}, keep using traditional buffer calculation mode".format( + profile_name, port, speed, cable_length)) + abandon_method() + return True except Exception: continue + # Insert other tables required for dynamic buffer calculation + metadata = self.configDB.get_entry('DEVICE_METADATA', 'localhost') + metadata['buffer_model'] = 'dynamic' + append_item_method(('DEVICE_METADATA', 'localhost', metadata)) + append_item_method(('DEFAULT_LOSSLESS_BUFFER_PARAMETER', 'AZURE', {'default_dynamic_th': default_dynamic_th})) + append_item_method(('LOSSLESS_TRAFFIC_PATTERN', 'AZURE', {'mtu': '1500', 'small_packet_percentage': '100'})) + return True def prepare_dynamic_buffer_for_warm_reboot(self, buffer_pools = None, buffer_profiles = None, buffer_pgs = None): @@ -253,11 +276,11 @@ def prepare_dynamic_buffer_for_warm_reboot(self, buffer_pools = None, buffer_pro ''' warmreboot_state = self.stateDB.get(self.stateDB.STATE_DB, 'WARM_RESTART_ENABLE_TABLE|system', 'enable') - if warmreboot_state == 'true': - self.stateDB.set(self.stateDB.STATE_DB, 'WARM_RESTART_TABLE|buffermgrd', 'restore_count', '0') + mmu_size = self.stateDB.get(self.stateDB.STATE_DB, 'BUFFER_MAX_PARAM_TABLE|global', 'mmu_size') + if warmreboot_state == 'true' and not mmu_size: log.log_notice("This is the very first run of buffermgrd (dynamc), prepare info requred from warm reboot") else: - return + return True buffer_table_list = [ ('BUFFER_POOL', buffer_pools, None), @@ -279,7 +302,7 @@ def prepare_dynamic_buffer_for_warm_reboot(self, buffer_pools = None, buffer_pro # copy items to appl db if reference_field_name: confdb_ref = items.get(reference_field_name) - if not confdb_ref: + if not confdb_ref or confdb_ref == "NULL": keys_ignored.append(key) continue items_referenced = confdb_ref.split(',') @@ -298,7 +321,7 @@ def prepare_dynamic_buffer_for_warm_reboot(self, buffer_pools = None, buffer_pro first_key = False else: appdb_ref += ':' + subitem - + items[reference_field_name] = appdb_ref keys_copied.append(key) if type(key) is tuple: @@ -313,6 +336,8 @@ def prepare_dynamic_buffer_for_warm_reboot(self, buffer_pools = None, buffer_pro if keys_ignored: log.log_info("The following items in table {} in CONFIG_DB have been ignored: {}".format(table_name, keys_copied)) + return True + def version_unknown(self): """ version_unknown tracks all SONiC versions that doesn't have a version @@ -354,7 +379,8 @@ def version_1_0_2(self): log.log_info('Handling version_1_0_2') # Check ASIC type, if Mellanox platform then need DB migration if self.asic_type == "mellanox": - if self.mellanox_buffer_migrator.mlnx_migrate_buffer_pool_size('version_1_0_2', 'version_1_0_3'): + if self.mellanox_buffer_migrator.mlnx_migrate_buffer_pool_size('version_1_0_2', 'version_1_0_3') \ + and self.mellanox_buffer_migrator.mlnx_flush_new_buffer_configuration(): self.set_version('version_1_0_3') else: self.set_version('version_1_0_3') @@ -368,7 +394,9 @@ def version_1_0_3(self): # Check ASIC type, if Mellanox platform then need DB migration if self.asic_type == "mellanox": - if self.mellanox_buffer_migrator.mlnx_migrate_buffer_pool_size('version_1_0_3', 'version_1_0_4') and self.mellanox_buffer_migrator.mlnx_migrate_buffer_profile('version_1_0_3', 'version_1_0_4'): + if self.mellanox_buffer_migrator.mlnx_migrate_buffer_pool_size('version_1_0_3', 'version_1_0_4') \ + and self.mellanox_buffer_migrator.mlnx_migrate_buffer_profile('version_1_0_3', 'version_1_0_4') \ + and self.mellanox_buffer_migrator.mlnx_flush_new_buffer_configuration(): self.set_version('version_1_0_4') else: self.set_version('version_1_0_4') @@ -383,27 +411,47 @@ def version_1_0_4(self): # Check ASIC type, if Mellanox platform then need DB migration if self.asic_type == "mellanox": - speed_list = ['1000', '10000', '25000', '40000', '50000', '100000', '200000', '400000'] - cable_len_list = ['5m', '40m', '300m'] + speed_list = self.mellanox_buffer_migrator.default_speed_list + cable_len_list = self.mellanox_buffer_migrator.default_cable_len_list buffer_pools = self.configDB.get_table('BUFFER_POOL') buffer_profiles = self.configDB.get_table('BUFFER_PROFILE') buffer_pgs = self.configDB.get_table('BUFFER_PG') - if self.mellanox_buffer_migrator.mlnx_migrate_buffer_pool_size('version_1_0_4', 'version_1_0_5') \ - and self.mellanox_buffer_migrator.mlnx_migrate_buffer_profile('version_1_0_4', 'version_1_0_5') \ - and self.migrate_config_db_buffer_tables_for_dynamic_calculation(speed_list, cable_len_list, '0') \ + default_lossless_profiles = self.mellanox_buffer_migrator.mlnx_get_default_lossless_profile('version_1_0_4') + abandon_method = self.mellanox_buffer_migrator.mlnx_abandon_pending_buffer_configuration + append_method = self.mellanox_buffer_migrator.mlnx_append_item_on_pending_configuration_list + + if self.mellanox_buffer_migrator.mlnx_migrate_buffer_pool_size('version_1_0_4', 'version_2_0_0') \ + and self.mellanox_buffer_migrator.mlnx_migrate_buffer_profile('version_1_0_4', 'version_2_0_0') \ + and self.migrate_config_db_buffer_tables_for_dynamic_calculation(speed_list, cable_len_list, '0', default_lossless_profiles, + abandon_method, append_method) \ + and self.mellanox_buffer_migrator.mlnx_flush_new_buffer_configuration() \ and self.prepare_dynamic_buffer_for_warm_reboot(buffer_pools, buffer_profiles, buffer_pgs): - self.set_version('version_1_0_5') + metadata = self.configDB.get_entry('DEVICE_METADATA', 'localhost') + if not metadata.get('buffer_model'): + metadata['buffer_model'] = 'traditional' + self.configDB.set_entry('DEVICE_METADATA', 'localhost', metadata) + log.log_notice('Setting buffer_model to traditional') + else: + log.log_notice('Got buffer_model {}'.format(metadata.get('buffer_model'))) + + self.set_version('version_2_0_0') else: self.prepare_dynamic_buffer_for_warm_reboot() - self.set_version('version_1_0_5') - return 'version_1_0_5' + metadata = self.configDB.get_entry('DEVICE_METADATA', 'localhost') + metadata['buffer_model'] = 'traditional' + self.configDB.set_entry('DEVICE_METADATA', 'localhost', metadata) + log.log_notice('Setting buffer_model to traditional') + + self.set_version('version_2_0_0') + + return 'version_2_0_0' - def version_1_0_5(self): + def version_2_0_0(self): """ Current latest version. Nothing to do here. """ - log.log_info('Handling version_1_0_5') + log.log_info('Handling version_2_0_0') return None diff --git a/scripts/mellanox_buffer_migrator.py b/scripts/mellanox_buffer_migrator.py old mode 100644 new mode 100755 index 6a8726dee5..e449f3657e --- a/scripts/mellanox_buffer_migrator.py +++ b/scripts/mellanox_buffer_migrator.py @@ -8,6 +8,10 @@ class MellanoxBufferMigrator(): def __init__(self, configDB): self.configDB = configDB + self.pending_update_items = list() + self.default_speed_list = ['1000', '10000', '25000', '40000', '50000', '100000', '200000', '400000'] + self.default_cable_len_list = ['5m', '40m', '300m'] + self.is_buffer_config_default = True mellanox_default_parameter = { "version_1_0_2": { @@ -169,7 +173,7 @@ def __init__(self, configDB): "egress_lossless_pool": { "size": "34287552", "type": "egress", "mode": "dynamic" }, "egress_lossy_pool": {"size": "12457984", "type": "egress", "mode": "dynamic" } }, - # SPC3 is used only when migrating from 1.0.4 to 1.0.5 + # SPC3 is used only when migrating from 1.0.4 to newer version "spc3_t0_pool": {"ingress_lossless_pool": { "size": "26451968", "type": "ingress", "mode": "dynamic" }, "ingress_lossy_pool": { "size": "26451968", "type": "ingress", "mode": "dynamic" }, "egress_lossless_pool": { "size": "60817392", "type": "egress", "mode": "dynamic" }, @@ -228,6 +232,27 @@ def __init__(self, configDB): "pg_lossless_40000_300m_profile": {"size": "95232", "xon":"19456"}, "pg_lossless_50000_300m_profile": {"size": "106496", "xon":"19456"}, "pg_lossless_100000_300m_profile": {"size": "159744", "xon":"19456"}}, + "spc3_headroom": {"pg_lossless_10000_5m_profile": {"size": "52224", "xon":"19456"}, + "pg_lossless_25000_5m_profile": {"size": "52224", "xon":"19456"}, + "pg_lossless_40000_5m_profile": {"size": "53248", "xon":"19456"}, + "pg_lossless_50000_5m_profile": {"size": "53248", "xon":"19456"}, + "pg_lossless_100000_5m_profile": {"size": "53248", "xon":"19456"}, + "pg_lossless_200000_5m_profile": {"size": "55296", "xon":"19456"}, + "pg_lossless_400000_5m_profile": {"size": "86016", "xon":"37888"}, + "pg_lossless_10000_40m_profile": {"size": "53248", "xon":"19456"}, + "pg_lossless_25000_40m_profile": {"size": "55296", "xon":"19456"}, + "pg_lossless_40000_40m_profile": {"size": "57344", "xon":"19456"}, + "pg_lossless_50000_40m_profile": {"size": "58368", "xon":"19456"}, + "pg_lossless_100000_40m_profile": {"size": "63488", "xon":"19456"}, + "pg_lossless_200000_40m_profile": {"size": "74752", "xon":"19456"}, + "pg_lossless_400000_40m_profile": {"size": "124928", "xon":"37888"}, + "pg_lossless_10000_300m_profile": {"size": "60416", "xon":"19456"}, + "pg_lossless_25000_300m_profile": {"size": "73728", "xon":"19456"}, + "pg_lossless_40000_300m_profile": {"size": "86016", "xon":"19456"}, + "pg_lossless_50000_300m_profile": {"size": "95232", "xon":"19456"}, + "pg_lossless_100000_300m_profile": {"size": "137216", "xon":"19456"}, + "pg_lossless_200000_300m_profile": {"size": "223232", "xon":"19456"}, + "pg_lossless_400000_300m_profile": {"size": "420864", "xon":"37888"}}, # Buffer profile info "buffer_profiles": {"ingress_lossless_profile": {"dynamic_th": "7", "pool": "[BUFFER_POOL|ingress_lossless_pool]", "size": "0"}, @@ -236,8 +261,8 @@ def __init__(self, configDB): "egress_lossy_profile": {"dynamic_th": "7", "pool": "[BUFFER_POOL|egress_lossy_pool]", "size": "9216"}, "q_lossy_profile": {"dynamic_th": "3", "pool": "[BUFFER_POOL|egress_lossy_pool]", "size": "0"}} }, - "version_1_0_5": { - # version 1.0.5 is introduced for dynamic buffer calculation + "version_2_0_0": { + # version 2.0.0 is introduced for dynamic buffer calculation # "pool_configuration_list": ["spc1_pool", "spc2_pool", "spc3_pool"], "pool_mapped_from_old_version": { @@ -287,6 +312,7 @@ def mlnx_migrate_buffer_pool_size(self, old_version, new_version): """ To migrate buffer pool configuration """ + self.is_buffer_config_default = False buffer_pool_conf_in_db = {} # Buffer pools defined in old version @@ -337,27 +363,46 @@ def mlnx_migrate_buffer_pool_size(self, old_version, new_version): log.log_error("Can't find the buffer pool configuration for {} in {}".format(new_config_name, new_version)) return False - # Migrate old buffer conf to latest. + # Don't migrate the old buffer pool conf to latest until we know all the following buffer configuration matches default value. for pool in old_default_buffer_pools: - self.configDB.set_entry('BUFFER_POOL', pool, new_buffer_pool_conf.get(pool)) + self.pending_update_items.append(('BUFFER_POOL', pool, new_buffer_pool_conf.get(pool))) - log.log_info("Successfully migrate mlnx buffer pool {} size to the latest.".format(pool)) + self.is_buffer_config_default = True return True + def mlnx_get_buffer_profile_key(self): + device_data = self.configDB.get_entry('DEVICE_METADATA', 'localhost') + if device_data: + platform = device_data.get('platform') + if not platform: + log.log_error("Trying to get DEVICE_METADATA from DB but doesn't exist, skip migration") + return None + + spc1_platforms = ["x86_64-mlnx_msn2010-r0", "x86_64-mlnx_msn2100-r0", "x86_64-mlnx_msn2410-r0", "x86_64-mlnx_msn2700-r0", "x86_64-mlnx_msn2740-r0"] + spc2_platforms = ["x86_64-mlnx_msn3700-r0", "x86_64-mlnx_msn3700c-r0"] + spc2_platform_with_gearbox = ['x86_64-mlnx_msn3800-r0'] + spc3_platforms = ["x86_64-mlnx_msn4600c-r0", "x86_64-mlnx_msn4700-r0"] + + if platform in spc2_platform_with_gearbox: + return "spc2_3800_headroom" + elif platform in spc3_platforms: + return "spc3_headroom" + elif platform in spc2_platforms: + return "spc2_headroom" + elif platform in spc1_platforms: + return "spc1_headroom" + + return None + def mlnx_migrate_buffer_profile(self, old_version, new_version): """ This is to migrate BUFFER_PROFILE configuration """ - device_data = self.configDB.get_table('DEVICE_METADATA') - if 'localhost' in device_data: - platform = device_data['localhost']['platform'] + if not self.is_buffer_config_default: + return True else: - log.log_error("Trying to get DEVICE_METADATA from DB but doesn't exist, skip migration") - return False - - spc1_platforms = ["x86_64-mlnx_msn2010-r0", "x86_64-mlnx_msn2100-r0", "x86_64-mlnx_msn2410-r0", "x86_64-mlnx_msn2700-r0", "x86_64-mlnx_msn2740-r0"] - spc2_platforms = ["x86_64-mlnx_msn3700-r0", "x86_64-mlnx_msn3700c-r0"] + self.is_buffer_config_default = False # get profile buffer_profile_old_configure = self.mlnx_default_buffer_parameters(old_version, "buffer_profiles") @@ -368,15 +413,13 @@ def mlnx_migrate_buffer_profile(self, old_version, new_version): # we need to transform lossless pg profiles to new settings # to achieve that, we just need to remove this kind of profiles, buffermgrd will generate them automatically default_lossless_profiles = None - if platform == 'x86_64-mlnx_msn3800-r0': - default_lossless_profiles = self.mlnx_default_buffer_parameters(old_version, "spc2_3800_headroom") - new_lossless_profiles = self.mlnx_default_buffer_parameters(new_version, "spc2_3800_headroom") - elif platform in spc2_platforms: - default_lossless_profiles = self.mlnx_default_buffer_parameters(old_version, "spc2_headroom") - new_lossless_profiles = self.mlnx_default_buffer_parameters(new_version, "spc2_headroom") - elif platform in spc1_platforms: - default_lossless_profiles = self.mlnx_default_buffer_parameters(old_version, "spc1_headroom") - new_lossless_profiles = self.mlnx_default_buffer_parameters(new_version, "spc1_headroom") + headroom_key = self.mlnx_get_buffer_profile_key() + if not headroom_key: + default_lossless_profiles = None + new_lossless_profiles = None + else: + default_lossless_profiles = self.mlnx_default_buffer_parameters(old_version, headroom_key) + new_lossless_profiles = self.mlnx_default_buffer_parameters(new_version, headroom_key) if default_lossless_profiles and new_lossless_profiles: for name, profile in buffer_profile_conf.items(): @@ -392,12 +435,7 @@ def mlnx_migrate_buffer_profile(self, old_version, new_version): default_profile['size'] = new_profile['size'] default_profile['xon'] = new_profile['xon'] default_profile['xoff'] = str(int(default_profile['size']) - int(default_profile['xon'])) - self.configDB.set_entry('BUFFER_PROFILE', name, default_profile) - - if not buffer_profile_new_configure: - # Not providing new profile configure in new version means they do need to be changed - log.log_notice("No buffer profile in {}, don't need to migrate non-lossless profiles".format(new_version)) - return True + self.pending_update_items.append(('BUFFER_PROFILE', name, default_profile)) for name, profile in buffer_profile_old_configure.items(): if name in buffer_profile_conf and profile == buffer_profile_conf[name]: @@ -406,8 +444,56 @@ def mlnx_migrate_buffer_profile(self, old_version, new_version): log.log_notice("Default profile {} isn't in database or doesn't match default value".format(name)) return True + self.is_buffer_config_default = True + + if not buffer_profile_new_configure: + # Not providing new profile configure in new version means they do need to be changed + log.log_notice("No buffer profile in {}, don't need to migrate non-lossless profiles".format(new_version)) + return True + for name, profile in buffer_profile_new_configure.items(): log.log_info("Successfully migrate profile {}".format(name)) - self.configDB.set_entry('BUFFER_PROFILE', name, profile) + self.pending_update_items.append(('BUFFER_PROFILE', name, profile)) + + return True + + def mlnx_append_item_on_pending_configuration_list(self, item): + self.pending_update_items.append(item) + + def mlnx_abandon_pending_buffer_configuration(self): + """ + We found the buffer configuration on the device doesn't match the default one, so no migration performed + Clear pending update item list in this case + """ + self.pending_update_items = [] + self.is_buffer_config_default = False + + def mlnx_flush_new_buffer_configuration(self): + """ + Flush all the pending items to config database + """ + if not self.is_buffer_config_default: + log.log_notice("No item pending to be updated") + metadata = self.configDB.get_entry('DEVICE_METADATA', 'localhost') + metadata['buffer_model'] = 'traditional' + self.configDB.set_entry('DEVICE_METADATA', 'localhost', metadata) + log.log_notice("Set buffer_model as traditional") + return True + + for item in self.pending_update_items: + table, key, value = item + self.configDB.set_entry(table, key, value) + if value: + log.log_notice("Successfully migrate {} {} to {}".format(table, key, value)) + else: + log.log_notice("Successfully remove {} {} which is no longer used".format(table, key)) return True + + def mlnx_get_default_lossless_profile(self, db_version): + key = self.mlnx_get_buffer_profile_key() + if not key: + return None + + default_profiles = self.mlnx_default_buffer_parameters(db_version, key) + return default_profiles diff --git a/tests/buffer_test.py b/tests/buffer_test.py index 3cb42fd2a6..2c315d608d 100644 --- a/tests/buffer_test.py +++ b/tests/buffer_test.py @@ -23,24 +23,24 @@ def setUp(self): def test_config_buffer_profile_headroom(self): runner = CliRunner() - result = runner.invoke(config.config.commands["buffer-profile"].commands["add"], - ["testprofile", "-dynamic_th", "3", "-xon", "18432", "-xoff", "32768"]) + result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["add"], + ["testprofile", "--dynamic_th", "3", "--xon", "18432", "--xoff", "32768"]) print(result.exit_code) print(result.output) assert result.exit_code == 0 def test_config_buffer_profile_dynamic_th(self): runner = CliRunner() - result = runner.invoke(config.config.commands["buffer-profile"].commands["add"], - ["testprofile", "-dynamic_th", "3"]) + result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["add"], + ["testprofile", "--dynamic_th", "3"]) print(result.exit_code) print(result.output) assert result.exit_code == 0 def test_config_buffer_profile_add_existing(self): runner = CliRunner() - result = runner.invoke(config.config.commands["buffer-profile"].commands["add"], - ["headroom_profile", "-dynamic_th", "3"]) + result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["add"], + ["headroom_profile", "--dynamic_th", "3"]) print(result.exit_code) print(result.output) assert result.exit_code != 0 @@ -48,8 +48,8 @@ def test_config_buffer_profile_add_existing(self): def test_config_buffer_profile_set_non_existing(self): runner = CliRunner() - result = runner.invoke(config.config.commands["buffer-profile"].commands["set"], - ["non_existing_profile", "-dynamic_th", "3"]) + result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["set"], + ["non_existing_profile", "--dynamic_th", "3"]) print(result.exit_code) print(result.output) assert result.exit_code != 0 @@ -57,8 +57,8 @@ def test_config_buffer_profile_set_non_existing(self): def test_config_buffer_profile_add_headroom_to_dynamic_profile(self): runner = CliRunner() - result = runner.invoke(config.config.commands["buffer-profile"].commands["set"], - ["alpha_profile", "-dynamic_th", "3", "-xon", "18432", "-xoff", "32768"]) + result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["set"], + ["alpha_profile", "--dynamic_th", "3", "--xon", "18432", "--xoff", "32768"]) print(result.exit_code) print(result.output) assert result.exit_code != 0 diff --git a/tests/config_test.py b/tests/config_test.py index 94b21a4f9b..1ca37fb7a9 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -17,7 +17,7 @@ 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: config qos reload --no-dynamic-buffer Executing reset-failed of service bgp... Executing reset-failed of service dhcp_relay... Executing reset-failed of service hostcfgd... From c23761909bdf49c2641e621e57b93639463a4327 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Mon, 16 Nov 2020 18:18:04 +0800 Subject: [PATCH 3/5] Fix all review comments and support python 3 Review comments: - Add testcases for show and config command - Address review comments in db_migrator - Use "size" to represent the size of the PG and "headroom" for xoff - Fix typo Signed-off-by: Stephen Sun --- config/main.py | 34 +++---- doc/Command-Reference.md | 14 +-- scripts/db_migrator.py | 16 ++-- scripts/mmuconfig | 13 ++- show/main.py | 9 +- tests/buffer_input/buffer_test_vectors.py | 108 ++++++++++++++++++++++ tests/buffer_test.py | 46 ++++++++- tests/mock_tables/config_db.json | 4 +- tests/mock_tables/state_db.json | 32 +++++++ 9 files changed, 231 insertions(+), 45 deletions(-) create mode 100644 tests/buffer_input/buffer_test_vectors.py diff --git a/config/main.py b/config/main.py index 90967efd00..122ca960f0 100644 --- a/config/main.py +++ b/config/main.py @@ -1704,7 +1704,7 @@ def reload(ctx, no_dynamic_buffer): ), fg="yellow") if buffer_model_updated: - print "Buffer calculation model updated, restarting swss is required to take effect" + print("Buffer calculation model updated, restarting swss is required to take effect") def is_dynamic_buffer_enabled(config_db): """Return whether the current system supports dynamic buffer calculation""" @@ -2584,7 +2584,7 @@ def pgmaps_check_legality(ctx, interface_name, input_pg, is_new_pg): ctx.fail("PG {} doesn't exist".format(input_pg)) return - for k, v in existing_pgs.iteritems(): + for k, v in existing_pgs.items(): port, existing_pg = k if port == interface_name: existing_lower = int(existing_pg[0]) @@ -2633,7 +2633,7 @@ def remove_pg_on_port(ctx, interface_name, pg_map): # Remvoe all dynamic lossless PGs on the port existing_pgs = config_db.get_table("BUFFER_PG") removed = False - for k, v in existing_pgs.iteritems(): + for k, v in existing_pgs.items(): port, existing_pg = k if port == interface_name and (not pg_map or pg_map == existing_pg): need_to_remove = False @@ -3372,7 +3372,7 @@ def priority(ctx, interface_name, priority, status): clicommon.run_command("pfc config priority {0} {1} {2}".format(status, interface_name, priority)) # -# 'buffer_profile' group ('config buffer_profile ...') +# 'buffer' group ('config buffer ...') # @config.group(cls=clicommon.AbbreviationGroup) @@ -3400,11 +3400,11 @@ def profile(ctx): @click.option('--size', metavar='', type=int, help="Set reserved size size") @click.option('--dynamic_th', metavar='', type=str, help="Set dynamic threshold") @click.option('--pool', metavar='', type=str, help="Buffer pool") -@click.pass_context -def add_profile(ctx, profile, xon, xoff, size, dynamic_th, pool): +@clicommon.pass_db +def add_profile(db, profile, xon, xoff, size, dynamic_th, pool): """Add or modify a buffer profile""" - config_db = ConfigDBConnector() - config_db.connect() + config_db = db.cfgdb + ctx = click.get_current_context() profile_entry = config_db.get_entry('BUFFER_PROFILE', profile) if profile_entry: @@ -3420,11 +3420,11 @@ def add_profile(ctx, profile, xon, xoff, size, dynamic_th, pool): @click.option('--size', metavar='', type=int, help="Set reserved size size") @click.option('--dynamic_th', metavar='', type=str, help="Set dynamic threshold") @click.option('--pool', metavar='', type=str, help="Buffer pool") -@click.pass_context -def set_profile(ctx, profile, xon, xoff, size, dynamic_th, pool): +@clicommon.pass_db +def set_profile(db, profile, xon, xoff, size, dynamic_th, pool): """Add or modify a buffer profile""" - config_db = ConfigDBConnector() - config_db.connect() + config_db = db.cfgdb + ctx = click.get_current_context() profile_entry = config_db.get_entry('BUFFER_PROFILE', profile) if not profile_entry: @@ -3497,15 +3497,15 @@ def update_profile(ctx, config_db, profile_name, xon, xoff, size, dynamic_th, po @profile.command('remove') @click.argument('profile', metavar='', required=True) -@click.pass_context -def remove_profile(ctx, profile): +@clicommon.pass_db +def remove_profile(db, profile): """Delete a buffer profile""" - config_db = ConfigDBConnector() - config_db.connect() + config_db = db.cfgdb + ctx = click.get_current_context() full_profile_name = '[BUFFER_PROFILE|{}]'.format(profile) existing_pgs = config_db.get_table("BUFFER_PG") - for k, v in existing_pgs.iteritems(): + for k, v in existing_pgs.items(): port, pg = k referenced_profile = v.get('profile') if referenced_profile and referenced_profile == full_profile_name: diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index bf6366fc2a..8f8c8c83d5 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -2365,18 +2365,18 @@ This command is used to configure a lossless buffer profile. - Usage: ``` - config buffer_profile add -xon -xoff [-headroom ] [-dynamic_th ] [-pool ] - config buffer_profile set -xon -xoff [-headroom ] [-dynamic_th ] [-pool ] + config buffer_profile add -xon -xoff [-size ] [-dynamic_th ] [-pool ] + config buffer_profile set -xon -xoff [-size ] [-dynamic_th ] [-pool ] config buffer_profile remove ``` All the parameters are devided to two groups, one for headroom and one for dynamic_th. For any command at lease one group of parameters should be provided. For headroom parameters: - - At lease one of `xoff` and `headroom` should be provided and the other will be optional and conducted via the formula `xon + xoff = headroom`. + - At lease one of `xoff` and `size` should be provided and the other will be optional and conducted via the formula `xon + xoff = size`. All other parameters are optional. - `xon` is madantory. - - `xon` + `xoff` <= `headroom`; For Mellanox platform xon + xoff == headroom + - `xon` + `xoff` <= `size`; For Mellanox platform xon + xoff == size If only headroom parameters are provided, the `dynamic_th` will be taken from `CONFIG_DB.DEFAULT_LOSSLESS_BUFFER_PARAMETER.default_dynamic_th`. @@ -2578,20 +2578,20 @@ This command is used to display the status of buffer pools and profiles currentl ---------- -------------------------------- ``` -**show buffer configure** +**show buffer configuration** This command is used to display the status of buffer pools and profiles currently configured. - Usage: ``` - show buffer configure + show buffer configuration ``` - Example: ``` - admin@sonic:~$ show buffer configure + admin@sonic:~$ show buffer configuration Pool: ingress_lossless_pool ---- -------- type ingress diff --git a/scripts/db_migrator.py b/scripts/db_migrator.py index e3fd033591..5b571ce758 100755 --- a/scripts/db_migrator.py +++ b/scripts/db_migrator.py @@ -188,8 +188,8 @@ def migrate_config_db_buffer_tables_for_dynamic_calculation(self, speed_list, ca ''' # Migrate BUFFER_PROFILEs, removing dynamically generated profiles dynamic_profile = self.configDB.get_table('BUFFER_PROFILE') - profile_pattern = 'pg_lossless_([0-9]*000)_([0-9]*m)_profile' - for name, info in dynamic_profile.iteritems(): + profile_pattern = 'pg_lossless_([1-9][0-9]*000)_([1-9][0-9]*m)_profile' + for name, info in dynamic_profile.items(): m = re.search(profile_pattern, name) if not m: continue @@ -212,12 +212,12 @@ def migrate_config_db_buffer_tables_for_dynamic_calculation(self, speed_list, ca ports = self.configDB.get_table('PORT') all_cable_lengths = self.configDB.get_table('CABLE_LENGTH') if not buffer_pgs or not ports or not all_cable_lengths: - log.log_notice("At lease one of tables BUFFER_PG, PORT and CABLE_LENGTH hasn't been defined, skip following mitration") + log.log_notice("At lease one of tables BUFFER_PG, PORT and CABLE_LENGTH hasn't been defined, skip following migration") abandon_method() return True - cable_lengths = all_cable_lengths[all_cable_lengths.keys()[0]] - for name, profile in buffer_pgs.iteritems(): + cable_lengths = all_cable_lengths[list(all_cable_lengths.keys())[0]] + for name, profile in buffer_pgs.items(): # do the db migration port, pg = name if pg != '3-4': @@ -278,7 +278,7 @@ def prepare_dynamic_buffer_for_warm_reboot(self, buffer_pools = None, buffer_pro warmreboot_state = self.stateDB.get(self.stateDB.STATE_DB, 'WARM_RESTART_ENABLE_TABLE|system', 'enable') mmu_size = self.stateDB.get(self.stateDB.STATE_DB, 'BUFFER_MAX_PARAM_TABLE|global', 'mmu_size') if warmreboot_state == 'true' and not mmu_size: - log.log_notice("This is the very first run of buffermgrd (dynamc), prepare info requred from warm reboot") + log.log_notice("This is the very first run of buffermgrd (dynamic), prepare info required from warm reboot") else: return True @@ -298,7 +298,7 @@ def prepare_dynamic_buffer_for_warm_reboot(self, buffer_pools = None, buffer_pro app_table_name = table_name + "_TABLE" if not entries: entries = self.configDB.get_table(table_name) - for key, items in entries.iteritems(): + for key, items in entries.items(): # copy items to appl db if reference_field_name: confdb_ref = items.get(reference_field_name) @@ -328,7 +328,7 @@ def prepare_dynamic_buffer_for_warm_reboot(self, buffer_pools = None, buffer_pro appl_db_key = app_table_name + ':' + ':'.join(key) else: appl_db_key = app_table_name + ':' + key - for field, data in items.iteritems(): + for field, data in items.items(): self.appDB.set(self.appDB.APPL_DB, appl_db_key, field, data) if keys_copied: diff --git a/scripts/mmuconfig b/scripts/mmuconfig index 3ccae34a60..e44fcf0e4d 100755 --- a/scripts/mmuconfig +++ b/scripts/mmuconfig @@ -29,6 +29,17 @@ BUFFER_PROFILE_FIELDS = { "alpha": DYNAMIC_THRESHOLD } +# mock the redis for unit test purposes # +try: + if os.environ["UTILITIES_UNIT_TESTING"] == "2": + modules_path = os.path.join(os.path.dirname(__file__), "..") + tests_path = os.path.join(modules_path, "tests") + sys.path.insert(0, modules_path) + sys.path.insert(0, tests_path) + import mock_tables.dbconnector + +except KeyError: + pass class MmuConfig(object): def __init__(self, verbose, config): @@ -111,7 +122,6 @@ class MmuConfig(object): def main(config): if config: parser = argparse.ArgumentParser(description='Show and change: mmu configuration', - version='1.0.0', formatter_class=argparse.RawTextHelpFormatter) parser.add_argument('-l', '--list', action='store_true', help='show mmu configuration') @@ -120,7 +130,6 @@ def main(config): parser.add_argument('-v', '--version', action='version', version='%(prog)s 1.0') else: parser = argparse.ArgumentParser(description='Show buffer state', - version='1.0.0', formatter_class=argparse.RawTextHelpFormatter) parser.add_argument('-l', '--list', action='store_true', help='show buffer state') diff --git a/show/main.py b/show/main.py index 5dc30698b0..f86b936505 100644 --- a/show/main.py +++ b/show/main.py @@ -1437,8 +1437,7 @@ def boot(): def mmu(): """Show mmu configuration""" cmd = "mmuconfig -l" - proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True, text=True) - click.echo(proc.stdout.read()) + run_command(cmd) # # 'buffer' command ("show buffer") @@ -1455,8 +1454,7 @@ def buffer(): def configuration(): """show buffer configuration""" cmd = "mmuconfig -l" - proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True) - click.echo(proc.stdout.read()) + run_command(cmd) # # 'information' command ("show buffer state") @@ -1465,8 +1463,7 @@ def configuration(): def information(): """show buffer information""" cmd = "buffershow -l" - proc = subprocess.Popen(cmd, stdout=subprocess.PIPE, shell=True) - click.echo(proc.stdout.read()) + run_command(cmd) # diff --git a/tests/buffer_input/buffer_test_vectors.py b/tests/buffer_input/buffer_test_vectors.py new file mode 100644 index 0000000000..82f0a09a35 --- /dev/null +++ b/tests/buffer_input/buffer_test_vectors.py @@ -0,0 +1,108 @@ +show_buffer_configuration="""\ +Pool: egress_lossless_pool +---- -------- +mode dynamic +size 13945824 +type egress +---- -------- + +Pool: egress_lossy_pool +---- ------- +mode dynamic +type egress +---- ------- + +Pool: ingress_lossless_pool +---- ------- +mode dynamic +type ingress +---- ------- + +Pool: ingress_lossy_pool +---- ------- +mode dynamic +type ingress +---- ------- + +Profile: ingress_lossy_profile +---------- -------------------------------- +dynamic_th 3 +pool [BUFFER_POOL|ingress_lossy_pool] +size 0 +---------- -------------------------------- + +Profile: headroom_profile +---------- ----------------------------------- +dynamic_th 0 +pool [BUFFER_POOL|ingress_lossless_pool] +xon 18432 +xoff 32768 +size 51200 +---------- ----------------------------------- + +Profile: alpha_profile +------------- ----------------------------------- +dynamic_th 0 +pool [BUFFER_POOL|ingress_lossless_pool] +headroom_type dynamic +------------- ----------------------------------- + +""" + +show_buffer_information_output="""\ +Pool: egress_lossless_pool +---- -------- +mode dynamic +size 13945824 +type egress +---- -------- + +Pool: egress_lossy_pool +---- ------- +mode dynamic +size 4580864 +type egress +---- ------- + +Pool: ingress_lossless_pool +---- ------- +mode dynamic +size 4580864 +type ingress +---- ------- + +Pool: ingress_lossy_pool +---- ------- +mode dynamic +size 4580864 +type ingress +---- ------- + +Profile: ingress_lossy_profile +---------- -------------------------------------- +dynamic_th 3 +pool [BUFFER_POOL_TABLE|ingress_lossy_pool] +size 0 +---------- -------------------------------------- + +Profile: headroom_profile +---------- ----------------------------------------- +dynamic_th 0 +pool [BUFFER_POOL_TABLE|ingress_lossless_pool] +xon 18432 +xoff 32768 +size 51200 +---------- ----------------------------------------- + +""" + +testData = { + 'show_buffer_configuration' : [ {'cmd' : ['buffer', 'configuration'], + 'rc_output': show_buffer_configuration + } + ], + 'show_buffer_information' : [ {'cmd' : ['buffer', 'information'], + 'rc_output': show_buffer_information_output + } + ] + } diff --git a/tests/buffer_test.py b/tests/buffer_test.py index 2c315d608d..cf32f39d55 100644 --- a/tests/buffer_test.py +++ b/tests/buffer_test.py @@ -1,18 +1,29 @@ +import imp import os import sys from click.testing import CliRunner from unittest import TestCase from swsssdk import ConfigDBConnector -import mock_tables.dbconnector +from .mock_tables import dbconnector +import show.main as show import config.main as config from utilities_common.db import Db +from .buffer_input.buffer_test_vectors import * + +test_path = os.path.dirname(os.path.abspath(__file__)) +modules_path = os.path.dirname(test_path) +scripts_path = os.path.join(modules_path, "scripts") +sys.path.insert(0, test_path) +sys.path.insert(0, modules_path) + class TestBuffer(object): @classmethod def setup_class(cls): - os.environ['UTILITIES_UNIT_TESTING'] = "1" + os.environ["PATH"] += os.pathsep + scripts_path + os.environ['UTILITIES_UNIT_TESTING'] = "2" print("SETUP") def setUp(self): @@ -23,19 +34,25 @@ def setUp(self): def test_config_buffer_profile_headroom(self): runner = CliRunner() + db = Db() result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["add"], - ["testprofile", "--dynamic_th", "3", "--xon", "18432", "--xoff", "32768"]) + ["testprofile", "--dynamic_th", "3", "--xon", "18432", "--xoff", "32768"], obj=db) print(result.exit_code) print(result.output) assert result.exit_code == 0 + profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'testprofile') + assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '18432', 'xoff': '32768', 'size': '51200'} def test_config_buffer_profile_dynamic_th(self): runner = CliRunner() + db = Db() result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["add"], - ["testprofile", "--dynamic_th", "3"]) + ["testprofile", "--dynamic_th", "3"], obj=db) print(result.exit_code) print(result.output) assert result.exit_code == 0 + profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'testprofile') + assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'headroom_type': 'dynamic'} def test_config_buffer_profile_add_existing(self): runner = CliRunner() @@ -64,7 +81,28 @@ def test_config_buffer_profile_add_headroom_to_dynamic_profile(self): assert result.exit_code != 0 assert "Can't change profile alpha_profile from dynamically calculating headroom to non-dynamically one" in result.output + def test_show_buffer_configuration(self): + self.executor(testData['show_buffer_configuration']) + + def test_show_buffer_information(self): + self.executor(testData['show_buffer_information']) + + def executor(self, testcase): + runner = CliRunner() + + for input in testcase: + exec_cmd = show.cli.commands[input['cmd'][0]].commands[input['cmd'][1]] + + result = runner.invoke(exec_cmd, []) + + print(result.exit_code) + print(result.output) + + assert result.exit_code == 0 + assert result.output == input['rc_output'] + @classmethod def teardown_class(cls): + os.environ["PATH"] = os.pathsep.join(os.environ["PATH"].split(os.pathsep)[:-1]) os.environ['UTILITIES_UNIT_TESTING'] = "0" print("TEARDOWN") diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index 982a512b6a..c80187dec9 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -1363,7 +1363,9 @@ "size": "51200" }, "BUFFER_PROFILE|alpha_profile": { - "dynamic_th": "0" + "dynamic_th": "0", + "pool": "[BUFFER_POOL|ingress_lossless_pool]", + "headroom_type": "dynamic" }, "BUFFER_PG|Ethernet0|3-4": { "profile": "NULL" diff --git a/tests/mock_tables/state_db.json b/tests/mock_tables/state_db.json index b8cd11315e..e489470bc7 100644 --- a/tests/mock_tables/state_db.json +++ b/tests/mock_tables/state_db.json @@ -327,5 +327,37 @@ }, "MUX_CABLE_TABLE|Ethernet12": { "state": "unknown" + }, + "BUFFER_POOL_TABLE|egress_lossless_pool": { + "mode": "dynamic", + "size": "13945824", + "type": "egress" + }, + "BUFFER_POOL_TABLE|egress_lossy_pool": { + "mode": "dynamic", + "size": "4580864", + "type": "egress" + }, + "BUFFER_POOL_TABLE|ingress_lossless_pool": { + "mode": "dynamic", + "size": "4580864", + "type": "ingress" + }, + "BUFFER_POOL_TABLE|ingress_lossy_pool": { + "mode": "dynamic", + "size": "4580864", + "type": "ingress" + }, + "BUFFER_PROFILE_TABLE|ingress_lossy_profile": { + "dynamic_th": "3", + "pool": "[BUFFER_POOL_TABLE|ingress_lossy_pool]", + "size": "0" + }, + "BUFFER_PROFILE_TABLE|headroom_profile": { + "dynamic_th": "0", + "pool": "[BUFFER_POOL_TABLE|ingress_lossless_pool]", + "xon": "18432", + "xoff": "32768", + "size": "51200" } } From a63b850a77adaf3822e485e00a344e12cb0e186a Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Fri, 4 Dec 2020 08:09:52 +0000 Subject: [PATCH 4/5] use formal command in the manual Signed-off-by: Stephen Sun --- doc/Command-Reference.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/Command-Reference.md b/doc/Command-Reference.md index 8f8c8c83d5..4122c0d865 100644 --- a/doc/Command-Reference.md +++ b/doc/Command-Reference.md @@ -2486,7 +2486,7 @@ This command is used to display the status of buffer pools and profiles currentl - Example: ``` - admin@sonic:~$ buffershow -l + admin@sonic:~$ show buffer information Pool: ingress_lossless_pool ---- -------- type ingress From aee20c13bded48b8c948de96e3d41d36252c01d8 Mon Sep 17 00:00:00 2001 From: Stephen Sun Date: Tue, 8 Dec 2020 07:47:32 +0000 Subject: [PATCH 5/5] Fix review comments: - Fix alignment error - Update DEVICE_METADATA|localhost.buffer_model only if it is changed Signed-off-by: Stephen Sun --- config/main.py | 4 ++-- tests/mock_tables/config_db.json | 2 +- tests/mock_tables/state_db.json | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/config/main.py b/config/main.py index 122ca960f0..778e35cf99 100644 --- a/config/main.py +++ b/config/main.py @@ -1630,8 +1630,8 @@ def _update_buffer_calculation_model(config_db, model): device_metadata = config_db.get_entry('DEVICE_METADATA', 'localhost') if device_metadata.get('buffer_model') != model: buffer_model_changed = True - device_metadata['buffer_model'] = model - config_db.set_entry('DEVICE_METADATA', 'localhost', device_metadata) + device_metadata['buffer_model'] = model + config_db.set_entry('DEVICE_METADATA', 'localhost', device_metadata) return buffer_model_changed @qos.command('reload') diff --git a/tests/mock_tables/config_db.json b/tests/mock_tables/config_db.json index c80187dec9..55bf817587 100644 --- a/tests/mock_tables/config_db.json +++ b/tests/mock_tables/config_db.json @@ -1365,7 +1365,7 @@ "BUFFER_PROFILE|alpha_profile": { "dynamic_th": "0", "pool": "[BUFFER_POOL|ingress_lossless_pool]", - "headroom_type": "dynamic" + "headroom_type": "dynamic" }, "BUFFER_PG|Ethernet0|3-4": { "profile": "NULL" diff --git a/tests/mock_tables/state_db.json b/tests/mock_tables/state_db.json index e489470bc7..b1c2f1cea1 100644 --- a/tests/mock_tables/state_db.json +++ b/tests/mock_tables/state_db.json @@ -349,9 +349,9 @@ "type": "ingress" }, "BUFFER_PROFILE_TABLE|ingress_lossy_profile": { - "dynamic_th": "3", - "pool": "[BUFFER_POOL_TABLE|ingress_lossy_pool]", - "size": "0" + "dynamic_th": "3", + "pool": "[BUFFER_POOL_TABLE|ingress_lossy_pool]", + "size": "0" }, "BUFFER_PROFILE_TABLE|headroom_profile": { "dynamic_th": "0",