QOS fieldvalue refernce ABNF format to string (#1626)

Qos tables in config db and app db used ABNF format i.e "[TABLE_NAME|name] to refer fieldvalue other qos tables.

Example:
Config DB:
"Ethernet92|3": {
"scheduler": "[SCHEDULER|scheduler.1]",
"wred_profile": "[WRED_PROFILE|AZURE_LOSSLESS]"
},
"Ethernet0|0": {
"profile": "[BUFFER_PROFILE|ingress_lossy_profile]"
},
"Ethernet0": {
"dscp_to_tc_map": "[DSCP_TO_TC_MAP|AZURE]",
"pfc_enable": "3,4",
"pfc_to_queue_map": "[MAP_PFC_PRIORITY_TO_QUEUE|AZURE]",
"tc_to_pg_map": "[TC_TO_PRIORITY_GROUP_MAP|AZURE]",
"tc_to_queue_map": "[TC_TO_QUEUE_MAP|AZURE]"
},

AppDB:
"BUFFER_QUEUE_TABLE:Ethernet88:3-4": {
"profile": "[BUFFER_PROFILE_TABLE:egress_lossless_profile]"
},

1#This format is not consistent with other DB schema followed in sonic.
2# Added db_migrator.py case to  change from old format in config_db and appl_db  to new format. 
3#Modified the test case 

Dependent pull requests: 
sonic-net/sonic-buildimage#7752  - To modify platfrom files 
sonic-net/sonic-buildimage#7281 - Yang model 
sonic-net/sonic-swss#1754    - swss change to remove ABNF format

config/main.py

@@ -3786,8 +3786,7 @@ def update_pg(ctx, interface_name, pg_map, override_profile, add = True):
  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})
+ config_db.set_entry("BUFFER_PG", (interface_name, pg_map), {"profile": override_profile})
  else:
  config_db.set_entry("BUFFER_PG", (interface_name, pg_map), {"profile": "NULL"})
  adjust_pfc_enable(ctx, interface_name, pg_map, True)
@@ -3809,7 +3808,7 @@ def remove_pg_on_port(ctx, interface_name, pg_map):
  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 referenced_profile and referenced_profile == 'ingress_lossy_profile':
  if pg_map:
  ctx.fail("Lossy PG {} can't be removed".format(pg_map))
  else:
@@ -4963,7 +4962,7 @@ def update_profile(ctx, config_db, profile_name, xon, xoff, size, dynamic_th, po
 
  if not pool:
  pool = 'ingress_lossless_pool'
- params['pool'] = '[BUFFER_POOL|' + pool + ']'
+ params['pool'] = pool
  if not config_db.get_entry('BUFFER_POOL', pool):
  ctx.fail("Pool {} doesn't exist".format(pool))
 
@@ -5036,12 +5035,11 @@ def remove_profile(db, profile):
  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.items():
  port, pg = k
  referenced_profile = v.get('profile')
- if referenced_profile and referenced_profile == full_profile_name:
+ if referenced_profile and referenced_profile == profile:
  ctx.fail("Profile {} is referenced by {}|{} and can't be removed".format(profile, port, pg))
 
  entry = config_db.get_entry("BUFFER_PROFILE", profile)

scripts/db_migrator.py

@@ -44,7 +44,7 @@ def __init__(self, namespace, socket=None):
  none-zero values.
  build: sequentially increase within a minor version domain.
  """
- self.CURRENT_VERSION = 'version_2_0_2'
+ self.CURRENT_VERSION = 'version_2_0_3'
 
  self.TABLE_NAME = 'VERSIONS'
  self.TABLE_KEY = 'DATABASE'
@@ -60,9 +60,11 @@ def __init__(self, namespace, socket=None):
  self.configDB = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace, **db_kwargs)
  self.configDB.db_connect('CONFIG_DB')
 
- self.appDB = SonicV2Connector(host='127.0.0.1')
- if self.appDB is not None:
- self.appDB.connect(self.appDB.APPL_DB)
+ if namespace is None:
+ self.appDB = ConfigDBConnector(**db_kwargs)
+ else:
+ self.appDB = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace, **db_kwargs)
+ self.appDB.db_connect('APPL_DB')
 
  self.stateDB = SonicV2Connector(host='127.0.0.1')
  if self.stateDB is not None:
@@ -382,6 +384,69 @@ def migrate_config_db_port_table_for_auto_neg(self):
  elif value['autoneg'] == '0':
  self.configDB.set(self.configDB.CONFIG_DB, '{}|{}'.format(table_name, key), 'autoneg', 'off')
 
+ def migrate_qos_db_fieldval_reference_remove(self, table_list, db, db_num, db_delimeter):
+ for pair in table_list:
+ table_name, fields_list = pair
+ qos_table = db.get_table(table_name)
+ for key, value in qos_table.items():
+ if type(key) is tuple:
+ db_key = table_name + db_delimeter + db_delimeter.join(key)
+ else:
+ db_key = table_name + db_delimeter + key
+
+ for field in fields_list:
+ if field in value:
+ fieldVal = value.get(field)
+ if not fieldVal or fieldVal == "NULL":
+ continue
+ newFiledVal = ""
+ # Check for ABNF format presence and convert ABNF to string
+ if "[" in fieldVal and db_delimeter in fieldVal and "]" in fieldVal:
+ log.log_info("Found ABNF format field value in table {} key {} field {} val {}".format(table_name, db_key, field, fieldVal))
+ value_list = fieldVal.split(",")
+ for item in value_list:
+ if "[" != item[0] or db_delimeter not in item or "]" != item[-1]:
+ continue
+ newFiledVal = newFiledVal + item[1:-1].split(db_delimeter)[1] + ','
+ newFiledVal = newFiledVal[:-1]
+ db.set(db_num, db_key, field, newFiledVal)
+ log.log_info("Modified ABNF format field value to string in table {} key {} field {} val {}".format(table_name, db_key, field, newFiledVal))
+ return True
+
+ def migrate_qos_fieldval_reference_format(self):
+ '''
+ This is to change for first time to remove field refernces of ABNF format
+ in APPL DB for warm boot.
+ i.e "[Tabale_name:name]" to string in APPL_DB. Reasons for doing this
+ - To consistent with all other SoNIC CONFIG_DB/APPL_DB tables and fields
+ - References in DB is not required, this will be taken care by YANG model leafref.
+ '''
+ qos_app_table_list = [
+ ('BUFFER_PG_TABLE', ['profile']),
+ ('BUFFER_QUEUE_TABLE', ['profile']),
+ ('BUFFER_PROFILE_TABLE', ['pool']),
+ ('BUFFER_PORT_INGRESS_PROFILE_LIST_TABLE', ['profile_list']),
+ ('BUFFER_PORT_EGRESS_PROFILE_LIST_TABLE', ['profile_list'])
+ ]
+
+ log.log_info("Remove APPL_DB QOS tables field reference ABNF format")
+ self.migrate_qos_db_fieldval_reference_remove(qos_app_table_list, self.appDB, self.appDB.APPL_DB, ':')
+
+ qos_table_list = [
+ ('QUEUE', ['scheduler', 'wred_profile']),
+ ('PORT_QOS_MAP', ['dscp_to_tc_map', 'dot1p_to_tc_map',
+ 'pfc_to_queue_map', 'tc_to_pg_map',
+ 'tc_to_queue_map', 'pfc_to_pg_map']),
+ ('BUFFER_PG', ['profile']),
+ ('BUFFER_QUEUE', ['profile']),
+ ('BUFFER_PROFILE', ['pool']),
+ ('BUFFER_PORT_INGRESS_PROFILE_LIST', ['profile_list']),
+ ('BUFFER_PORT_EGRESS_PROFILE_LIST', ['profile_list'])
+ ]
+ log.log_info("Remove CONFIG_DB QOS tables field reference ABNF format")
+ self.migrate_qos_db_fieldval_reference_remove(qos_table_list, self.configDB, self.configDB.CONFIG_DB, '|')
+ return True
+
  def version_unknown(self):
  """
  version_unknown tracks all SONiC versions that doesn't have a version
@@ -542,9 +607,19 @@ def version_2_0_1(self):
 
  def version_2_0_2(self):
  """
- Current latest version. Nothing to do here.
+ Version 2_0_2.
  """
  log.log_info('Handling version_2_0_2')
+ self.migrate_qos_fieldval_reference_format()
+ self.set_version('version_2_0_3')
+ return 'version_2_0_3'
+
+
+ def version_2_0_3(self):
+ """
+ Current latest version. Nothing to do here.
+ """
+ log.log_info('Handling version_2_0_3')
  return None
 
  def get_version(self):

tests/buffer_input/buffer_test_vectors.py

@@ -31,27 +31,27 @@
 ---- -------
 
 Profile: ingress_lossy_profile
----------- --------------------------------
+---------- ------------------
 dynamic_th 3
-pool [BUFFER_POOL|ingress_lossy_pool]
+pool ingress_lossy_pool
 size 0
----------- --------------------------------
+---------- ------------------
 
 Profile: headroom_profile
----------- -----------------------------------
+---------- ---------------------
 dynamic_th 0
-pool [BUFFER_POOL|ingress_lossless_pool]
+pool ingress_lossless_pool
 xon 18432
 xoff 32768
 size 51200
----------- -----------------------------------
+---------- ---------------------
 
 Profile: alpha_profile
-------------- -----------------------------------
+------------- ---------------------
 dynamic_th 0
-pool [BUFFER_POOL|ingress_lossless_pool]
+pool ingress_lossless_pool
 headroom_type dynamic
-------------- -----------------------------------
+------------- ---------------------
 
 """
 
@@ -85,20 +85,20 @@
 ---- -------
 
 Profile: ingress_lossy_profile
----------- --------------------------------------
+---------- ------------------
 dynamic_th 3
-pool [BUFFER_POOL_TABLE|ingress_lossy_pool]
+pool ingress_lossy_pool
 size 0
----------- --------------------------------------
+---------- ------------------
 
 Profile: headroom_profile
----------- -----------------------------------------
+---------- ---------------------
 dynamic_th 0
-pool [BUFFER_POOL_TABLE|ingress_lossless_pool]
+pool ingress_lossless_pool
 xon 18432
 xoff 32768
 size 51200
----------- -----------------------------------------
+---------- ---------------------
 
 """
 

tests/buffer_test.py

@@ -34,7 +34,7 @@ def test_config_buffer_profile_headroom(self):
  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': '18432'}
+ assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '18432', 'xoff': '32768', 'size': '18432'}
 
  def test_config_buffer_profile_dynamic_th(self):
  runner = CliRunner()
@@ -45,7 +45,7 @@ def test_config_buffer_profile_dynamic_th(self):
  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'}
+ assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'headroom_type': 'dynamic'}
 
  def test_config_buffer_profile_add_existing(self):
  runner = CliRunner()
@@ -164,7 +164,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
  print(result.output)
  assert result.exit_code == 0
  profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'test1')
- assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '18432', 'xoff': '32768', 'size': '51200'}
+ assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '18432', 'xoff': '32768', 'size': '51200'}
 
  # Xoff should equal size - xon
  result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["add"],
@@ -173,7 +173,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
  print(result.output)
  assert result.exit_code == 0
  profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'test2')
- assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '18432', 'xoff': '14336', 'size': '32768'}
+ assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '18432', 'xoff': '14336', 'size': '32768'}
 
  # Neither xon nor size is provided
  result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["add"],
@@ -198,7 +198,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
  print(result.output)
  assert result.exit_code == 0
  profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'test2')
- assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '18432', 'xoff': '14336', 'size': '65536'}
+ assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '18432', 'xoff': '14336', 'size': '65536'}
 
  # Set xon
  result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["set"],
@@ -207,7 +207,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
  print(result.output)
  assert result.exit_code == 0
  profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'test2')
- assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '19456', 'xoff': '14336', 'size': '65536'}
+ assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '19456', 'xoff': '14336', 'size': '65536'}
 
  # Set xoff
  result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["set"],
@@ -216,7 +216,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
  print(result.output)
  assert result.exit_code == 0
  profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'test2')
- assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '19456', 'xoff': '18432', 'size': '65536'}
+ assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '19456', 'xoff': '18432', 'size': '65536'}
 
  # Enable SHP by setting size
  result = runner.invoke(config.config.commands["buffer"].commands["shared-headroom-pool"].commands["size"],
@@ -232,7 +232,7 @@ def test_config_buffer_profile_headroom_toggle_shp(self):
  print(result.output)
  assert result.exit_code == 0
  profile = db.cfgdb.get_entry('BUFFER_PROFILE', 'testprofile3')
- assert profile == {'dynamic_th': '3', 'pool': '[BUFFER_POOL|ingress_lossless_pool]', 'xon': '18432', 'xoff': '32768', 'size': '18432'}
+ assert profile == {'dynamic_th': '3', 'pool': 'ingress_lossless_pool', 'xon': '18432', 'xoff': '32768', 'size': '18432'}
 
  # Negative test: xoff not provided
  result = runner.invoke(config.config.commands["buffer"].commands["profile"].commands["add"],