Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QOS fieldvalue reference ABNF format to string changes #1754

Merged
merged 2 commits into from
Sep 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions cfgmgr/buffer_check_headroom_mellanox.lua
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,11 @@ end
table.insert(debuginfo, 'debug:other overhead:' .. accumulative_size)
local pg_keys = redis.call('KEYS', 'BUFFER_PG_TABLE:' .. port .. ':*')
for i = 1, #pg_keys do
local profile = string.sub(redis.call('HGET', pg_keys[i], 'profile'), 2, -2)
local profile = redis.call('HGET', pg_keys[i], 'profile')
local current_profile_size
if profile ~= 'BUFFER_PROFILE_TABLE:ingress_lossy_profile' and (no_input_pg or new_pg ~= pg_keys[i]) then
if profile ~= 'ingress_lossy_profile' and (no_input_pg or new_pg ~= pg_keys[i]) then
if profile ~= input_profile_name and not no_input_pg then
local referenced_profile = redis.call('HGETALL', profile)
local referenced_profile = redis.call('HGETALL', 'BUFFER_PROFILE_TABLE:' .. profile)
for j = 1, #referenced_profile, 2 do
if referenced_profile[j] == 'size' then
current_profile_size = tonumber(referenced_profile[j+1])
Expand Down
2 changes: 1 addition & 1 deletion cfgmgr/buffer_pool_mellanox.lua
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ local function iterate_all_items(all_items, check_lossless)
if not profile_name then
return 1
end
profile_name = string.sub(profile_name, 2, -2)
profile_name = "BUFFER_PROFILE_TABLE:" .. profile_name
local profile_ref_count = profiles[profile_name]
if profile_ref_count == nil then
-- Indicate an error in case the referenced profile hasn't been inserted or has been removed
Expand Down
49 changes: 4 additions & 45 deletions cfgmgr/buffermgr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ Create/update two tables: profile (in m_cfgBufferProfileTable) and port buffer (

"BUFFER_PROFILE": {
"pg_lossless_100G_300m_profile": {
"pool":"[BUFFER_POOL_TABLE:ingress_lossless_pool]",
"pool":"ingress_lossless_pool",
"xon":"18432",
"xon_offset":"2496",
"xoff":"165888",
Expand All @@ -117,7 +117,7 @@ Create/update two tables: profile (in m_cfgBufferProfileTable) and port buffer (
}
"BUFFER_PG" :{
Ethernet44|3-4": {
"profile" : "[BUFFER_PROFILE:pg_lossless_100000_300m_profile]"
"profile" : "pg_lossless_100000_300m_profile"
}
}
*/
Expand Down Expand Up @@ -168,11 +168,8 @@ task_process_status BufferMgr::doSpeedUpdateTask(string port)

// profile threshold field name
mode += "_th";
string pg_pool_reference = string(CFG_BUFFER_POOL_TABLE_NAME) +
m_cfgBufferProfileTable.getTableNameSeparator() +
INGRESS_LOSSLESS_PG_POOL_NAME;

fvVector.push_back(make_pair("pool", "[" + pg_pool_reference + "]"));
fvVector.push_back(make_pair("pool", INGRESS_LOSSLESS_PG_POOL_NAME));
fvVector.push_back(make_pair("xon", m_pgProfileLookup[speed][cable].xon));
if (m_pgProfileLookup[speed][cable].xon_offset.length() > 0) {
fvVector.push_back(make_pair("xon_offset",
Expand All @@ -192,11 +189,7 @@ task_process_status BufferMgr::doSpeedUpdateTask(string port)

string buffer_pg_key = port + m_cfgBufferPgTable.getTableNameSeparator() + LOSSLESS_PGS;

string profile_ref = string("[") +
CFG_BUFFER_PROFILE_TABLE_NAME +
m_cfgBufferPgTable.getTableNameSeparator() +
buffer_profile_key +
"]";
string profile_ref = buffer_profile_key;

/* Check if PG Mapping is already then log message and return. */
m_cfgBufferPgTable.get(buffer_pg_key, fvVector);
Expand Down Expand Up @@ -224,32 +217,6 @@ void BufferMgr::transformSeperator(string &name)
name.replace(pos, 1, ":");
}

void BufferMgr::transformReference(string &name)
{
auto references = tokenize(name, list_item_delimiter);
int ref_index = 0;

name = "";

for (auto &reference : references)
{
if (ref_index != 0)
name += list_item_delimiter;
ref_index ++;

auto keys = tokenize(reference, config_db_key_delimiter);
int key_index = 0;
for (auto &key : keys)
{
if (key_index == 0)
name += key + "_TABLE";
else
name += delimiter + key;
key_index ++;
}
}
}

/*
* This function copies the data from tables in CONFIG_DB to APPL_DB.
* With dynamically buffer calculation supported, the following tables
Expand Down Expand Up @@ -292,14 +259,6 @@ void BufferMgr::doBufferTableTask(Consumer &consumer, ProducerStateTable &applTa

for (auto i : kfvFieldsValues(t))
{
SWSS_LOG_INFO("Inserting field %s value %s", fvField(i).c_str(), fvValue(i).c_str());
Copy link
Collaborator

Choose a reason for hiding this comment

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

transformSeperator which is called in L269 can be removed as well. There is no separator now.
The function transformSeperator and transformReference can be removed as they are no longer called

Copy link
Contributor Author

Choose a reason for hiding this comment

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

transformSeperator is used to convert from config_db to app_db

//transform the separator in values from "|" to ":"
if (fvField(i) == "pool")
transformReference(fvValue(i));
if (fvField(i) == "profile")
transformReference(fvValue(i));
if (fvField(i) == "profile_list")
transformReference(fvValue(i));
fvVector.emplace_back(FieldValueTuple(fvField(i), fvValue(i)));
SWSS_LOG_INFO("Inserting field %s value %s", fvField(i).c_str(), fvValue(i).c_str());
}
Expand Down
1 change: 0 additions & 1 deletion cfgmgr/buffermgr.h
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ class BufferMgr : public Orch
void doBufferTableTask(Consumer &consumer, ProducerStateTable &applTable);

void transformSeperator(std::string &name);
void transformReference(std::string &name);

void doTask(Consumer &consumer);
};
Expand Down
60 changes: 4 additions & 56 deletions cfgmgr/buffermgrdyn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,32 +202,6 @@ void BufferMgrDynamic::transformSeperator(string &name)
name.replace(pos, 1, ":");
}

void BufferMgrDynamic::transformReference(string &name)
{
auto references = tokenize(name, list_item_delimiter);
int ref_index = 0;

name = "";

for (auto &reference : references)
{
if (ref_index != 0)
name += list_item_delimiter;
ref_index ++;

auto keys = tokenize(reference, config_db_key_delimiter);
int key_index = 0;
for (auto &key : keys)
{
if (key_index == 0)
name += key + "_TABLE";
else
name += delimiter + key;
key_index ++;
}
}
}

// For string "TABLE_NAME|objectname", returns "objectname"
string BufferMgrDynamic::parseObjectNameFromKey(const string &key, size_t pos = 0)
{
Expand All @@ -240,13 +214,6 @@ string BufferMgrDynamic::parseObjectNameFromKey(const string &key, size_t pos =
return keys[pos];
}

// For string "[foo]", returns "foo"
string BufferMgrDynamic::parseObjectNameFromReference(const string &reference)
{
auto objName = reference.substr(1, reference.size() - 2);
return parseObjectNameFromKey(objName, 1);
}

string BufferMgrDynamic::getDynamicProfileName(const string &speed, const string &cable, const string &mtu, const string &threshold, const string &gearbox_model, long lane_count)
{
string buffer_profile_key;
Expand Down Expand Up @@ -619,17 +586,14 @@ void BufferMgrDynamic::updateBufferProfileToDb(const string &name, const buffer_

// profile threshold field name
mode += "_th";
string pg_pool_reference = string(APP_BUFFER_POOL_TABLE_NAME) +
m_applBufferProfileTable.getTableNameSeparator() +
INGRESS_LOSSLESS_PG_POOL_NAME;

fvVector.emplace_back("xon", profile.xon);
if (!profile.xon_offset.empty()) {
fvVector.emplace_back("xon_offset", profile.xon_offset);
}
fvVector.emplace_back("xoff", profile.xoff);
fvVector.emplace_back("size", profile.size);
fvVector.emplace_back("pool", "[" + pg_pool_reference + "]");
fvVector.emplace_back("pool", INGRESS_LOSSLESS_PG_POOL_NAME);
fvVector.emplace_back(mode, profile.threshold);

m_applBufferProfileTable.set(name, fvVector);
Expand All @@ -646,15 +610,7 @@ void BufferMgrDynamic::updateBufferPgToDb(const string &key, const string &profi

fvVector.clear();

string profile_ref = string("[") +
APP_BUFFER_PROFILE_TABLE_NAME +
m_applBufferPgTable.getTableNameSeparator() +
profile +
"]";

fvVector.clear();

fvVector.push_back(make_pair("profile", profile_ref));
fvVector.push_back(make_pair("profile", profile));
m_applBufferPgTable.set(key, fvVector);
}
else
Expand Down Expand Up @@ -1779,8 +1735,7 @@ task_process_status BufferMgrDynamic::handleBufferProfileTable(KeyOpFieldsValues
{
if (!value.empty())
{
transformReference(value);
auto poolName = parseObjectNameFromReference(value);
auto poolName = value;
if (poolName.empty())
{
SWSS_LOG_ERROR("BUFFER_PROFILE: Invalid format of reference to pool: %s", value.c_str());
Expand Down Expand Up @@ -1953,8 +1908,7 @@ task_process_status BufferMgrDynamic::handleOneBufferPgEntry(const string &key,
{
// Headroom override
pureDynamic = false;
transformReference(value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

transformReference is no longer called and can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

string profileName = parseObjectNameFromReference(value);
string profileName = value;
if (profileName.empty())
{
SWSS_LOG_ERROR("BUFFER_PG: Invalid format of reference to profile: %s", value.c_str());
Expand Down Expand Up @@ -2170,12 +2124,6 @@ task_process_status BufferMgrDynamic::doBufferTableTask(KeyOpFieldsValuesTuple &
for (auto i : kfvFieldsValues(tuple))
{
// Transform the separator in values from "|" to ":"
if (fvField(i) == "pool")
transformReference(fvValue(i));
if (fvField(i) == "profile")
transformReference(fvValue(i));
if (fvField(i) == "profile_list")
transformReference(fvValue(i));
fvVector.emplace_back(fvField(i), fvValue(i));
SWSS_LOG_INFO("Inserting field %s value %s", fvField(i).c_str(), fvValue(i).c_str());
}
Expand Down
2 changes: 0 additions & 2 deletions cfgmgr/buffermgrdyn.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,7 @@ class BufferMgrDynamic : public Orch
// Tool functions to parse keys and references
std::string getPgPoolMode();
void transformSeperator(std::string &name);
void transformReference(std::string &name);
std::string parseObjectNameFromKey(const std::string &key, size_t pos/* = 1*/);
std::string parseObjectNameFromReference(const std::string &reference);
std::string getDynamicProfileName(const std::string &speed, const std::string &cable, const std::string &mtu, const std::string &threshold, const std::string &gearbox_model, long lane_count);
inline bool isNonZero(const std::string &value) const
{
Expand Down
42 changes: 21 additions & 21 deletions doc/Configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -343,13 +343,13 @@ When the system is running in traditional buffer model, profiles needs to explic
{
"BUFFER_PG": {
"Ethernet0|3-4": {
"profile": "[BUFFER_PROFILE|pg_lossless_40000_5m_profile]"
"profile": "pg_lossless_40000_5m_profile"
},
"Ethernet1|3-4": {
"profile": "[BUFFER_PROFILE|pg_lossless_40000_5m_profile]"
"profile": "pg_lossless_40000_5m_profile"
},
"Ethernet2|3-4": {
"profile": "[BUFFER_PROFILE|pg_lossless_40000_5m_profile]"
"profile": "pg_lossless_40000_5m_profile"
}
}
}
Expand All @@ -371,7 +371,7 @@ When the system is running in dynamic buffer model, profiles can be:
"profile": "NULL"
},
"Ethernet2|3-4": {
"profile": "[BUFFER_PROFILE|static_profile]"
"profile": "static_profile"
}
}
}
Expand Down Expand Up @@ -437,33 +437,33 @@ When the system is running in dynamic buffer model, the size of some of the buff
"BUFFER_PROFILE": {
"egress_lossless_profile": {
"static_th": "3995680",
"pool": "[BUFFER_POOL|egress_lossless_pool]",
"pool": "egress_lossless_pool",
"size": "1518"
},
"egress_lossy_profile": {
"dynamic_th": "3",
"pool": "[BUFFER_POOL|egress_lossy_pool]",
"pool": "egress_lossy_pool",
"size": "1518"
},
"ingress_lossy_profile": {
"dynamic_th": "3",
"pool": "[BUFFER_POOL|ingress_lossless_pool]",
"pool": "ingress_lossless_pool",
"size": "0"
},
"pg_lossless_40000_5m_profile": {
"xon_offset": "2288",
"dynamic_th": "-3",
"xon": "2288",
"xoff": "66560",
"pool": "[BUFFER_POOL|ingress_lossless_pool]",
"pool": "ingress_lossless_pool",
"size": "1248"
},
"pg_lossless_40000_40m_profile": {
"xon_offset": "2288",
"dynamic_th": "-3",
"xon": "2288",
"xoff": "71552",
"pool": "[BUFFER_POOL|ingress_lossless_pool]",
"pool": "ingress_lossless_pool",
"size": "1248"
}
}
Expand Down Expand Up @@ -491,13 +491,13 @@ This kind of profiles will be handled by buffer manager and won't be applied to
{
"BUFFER_QUEUE": {
"Ethernet50,Ethernet52,Ethernet54,Ethernet56|0-2": {
"profile": "[BUFFER_PROFILE|egress_lossy_profile]"
"profile": "egress_lossy_profile"
},
"Ethernet50,Ethernet52,Ethernet54,Ethernet56|3-4": {
"profile": "[BUFFER_PROFILE|egress_lossless_profile]"
"profile": "egress_lossless_profile"
},
"Ethernet50,Ethernet52,Ethernet54,Ethernet56|5-6": {
"profile": "[BUFFER_PROFILE|egress_lossy_profile]"
"profile": "egress_lossy_profile"
}
}
}
Expand Down Expand Up @@ -1104,12 +1104,12 @@ name as object key and member list as attribute.
{
"PORT_QOS_MAP": {
"Ethernet50,Ethernet52,Ethernet54,Ethernet56": {
"tc_to_pg_map": "[TC_TO_PRIORITY_GROUP_MAP|AZURE]",
"tc_to_queue_map": "[TC_TO_QUEUE_MAP|AZURE]",
"tc_to_pg_map": "AZURE",
"tc_to_queue_map": "AZURE",
"pfc_enable": "3,4",
"pfc_to_queue_map": "[MAP_PFC_PRIORITY_TO_QUEUE|AZURE]",
"dscp_to_tc_map": "[DSCP_TO_TC_MAP|AZURE]",
"scheduler": "[SCHEDULER|scheduler.port]"
"pfc_to_queue_map": "AZURE",
"dscp_to_tc_map": "AZURE",
"scheduler": "scheduler.port"
}
}
}
Expand All @@ -1120,14 +1120,14 @@ name as object key and member list as attribute.
{
"QUEUE": {
"Ethernet56|4": {
"wred_profile": "[WRED_PROFILE|AZURE_LOSSLESS]",
"scheduler": "[SCHEDULER|scheduler.1]"
"wred_profile": "AZURE_LOSSLESS",
"scheduler": "scheduler.1"
},
"Ethernet56|5": {
"scheduler": "[SCHEDULER|scheduler.0]"
"scheduler": "scheduler.0"
},
"Ethernet56|6": {
"scheduler": "[SCHEDULER|scheduler.0]"
"scheduler": "scheduler.0"
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions doc/swss-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ Stores information for physical switch ports managed by the switch chip. Ports t
Example:
127.0.0.1:6379> hgetall PORT_TABLE:ETHERNET4
1) "dscp_to_tc_map"
2) "[DSCP_TO_TC_MAP_TABLE:AZURE]"
2) "AZURE"
3) "tc_to_queue_map"
4) "[TC_TO_QUEUE_MAP_TABLE:AZURE]"
4) "AZURE"

---------------------------------------------
### INTF_TABLE
Expand Down Expand Up @@ -209,9 +209,9 @@ and reflects the LAG ports into the redis under: `LAG_TABLE:<team0>:port`
Example:
127.0.0.1:6379> hgetall QUEUE_TABLE:ETHERNET4:1
1) "scheduler"
2) "[SCHEDULER_TABLE:BEST_EFFORT]"
2) "BEST_EFFORT"
3) "wred_profile"
4) "[WRED_PROFILE_TABLE:AZURE]"
4) "AZURE"

---------------------------------------------
### TC\_TO\_QUEUE\_MAP\_TABLE
Expand Down
Loading