Skip to content

Commit

Permalink
[Dynamic buffer calculation][Mellanox] Enhance the logic to identify …
Browse files Browse the repository at this point in the history
…buffer pools and profiles (sonic-net#2498)

Signed-off-by: Stephen Sun <stephens@nvidia.com>

What I did
Originally, it was assumed the names of all the buffer pools follow the community buffer pool name convention ({ingress|egress}_{lossless|lossy}_pool). The heuristic algorithm to identify buffer pools and profiles was designed based on the assumption.
However, some users can define the buffer pool names in other ways, which breaks the logic in the Lua plugin and introduces degradation,

the pool sizes of those pools can not be generated
the additional reserved memory for lossy PG can not be calculated
In this PR, the logic is improved to tolerate the case.
Signed-off-by: Stephen Sun stephens@nvidia.com

How I verified it
Manually and regression test.
It has been covered by regression test and vs test. No new test case is required.

Details if related
Separate the buffer pools into two tables according to the direction.
Iterate all the profiles, generating and recording the type (lossless/lossy) for each ingress profile which is identified by checking the pool it references
Identify buffer profiles for lossy PG by checking the type (lossless/lossy) generated in 2 instead of the hardcoded name ingress_lossy_profile
  • Loading branch information
stephenxs authored Nov 2, 2022
1 parent e04bb43 commit 84642f3
Showing 1 changed file with 45 additions and 15 deletions.
60 changes: 45 additions & 15 deletions cfgmgr/buffer_pool_mellanox.lua
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ local port_count_8lanes = 0
-- Number of lossy PG on ports with 8 lanes
local lossypg_8lanes = 0

local ingress_profile_is_lossless = {}

-- Private headrom
local private_headroom = 10 * 1024

local result = {}
local profiles = {}
local lossless_profiles = {}

local total_port = 0

Expand Down Expand Up @@ -52,11 +53,11 @@ local function iterate_all_items(all_items, check_lossless)
port = string.match(all_items[i], "Ethernet%d+")
if port ~= nil then
local range = string.match(all_items[i], "Ethernet%d+:([^%s]+)$")
local profile_name = redis.call('HGET', all_items[i], 'profile')
if not profile_name then
local profile_name_without_table = redis.call('HGET', all_items[i], 'profile')
if not profile_name_without_table then
return 1
end
profile_name = "BUFFER_PROFILE_TABLE:" .. profile_name
local profile_name = "BUFFER_PROFILE_TABLE:" .. profile_name_without_table
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 All @@ -71,10 +72,11 @@ local function iterate_all_items(all_items, check_lossless)
size = 1 + tonumber(string.sub(range, -1)) - tonumber(string.sub(range, 1, 1))
end
profiles[profile_name] = profile_ref_count + size
if port_set_8lanes[port] and profile_name == 'BUFFER_PROFILE_TABLE:ingress_lossy_profile' then
if port_set_8lanes[port] and ingress_profile_is_lossless[profile_name] == false then
-- Handle additional buffer reserved for lossy PG on 8-lane ports
lossypg_8lanes = lossypg_8lanes + size
end
if check_lossless and lossless_profiles[profile_name] then
if check_lossless and ingress_profile_is_lossless[profile_name] then
if lossless_ports[port] == nil then
lossless_port_count = lossless_port_count + 1
lossless_ports[port] = true
Expand Down Expand Up @@ -113,7 +115,8 @@ local function iterate_profile_list(all_items)
-- To distinguish both cases, a new name "ingress_lossy_profile_list" is introduced to indicate
-- the profile is used by the profile list where its size should be zero.
profile_name = 'BUFFER_PROFILE_TABLE:' .. profile_name
if profile_name == 'BUFFER_PROFILE_TABLE:ingress_lossy_profile' then
-- TODO CHECK ALL LOSSY PROFILES
if ingress_profile_is_lossless[profile_name] == false then
profile_name = profile_name .. '_list'
if profiles[profile_name] == nil then
profiles[profile_name] = 0
Expand Down Expand Up @@ -162,9 +165,25 @@ local function fetch_buffer_pool_size_from_appldb()
end
end

-- Main --
-- Connect to CONFIG_DB
redis.call('SELECT', config_db)

-- Parse all the pools and seperate them according to the direction
local ipools = {}
local epools = {}
local pools = redis.call('KEYS', 'BUFFER_POOL|*')
for i = 1, #pools, 1 do
local type = redis.call('HGET', pools[i], 'type')
if type == 'ingress' then
table.insert(ipools, pools[i])
else
if type == 'egress' then
table.insert(epools, pools[i])
end
end
end

local ports_table = redis.call('KEYS', 'PORT|*')

total_port = #ports_table
Expand Down Expand Up @@ -250,9 +269,19 @@ redis.call('SELECT', appl_db)
local all_profiles = redis.call('KEYS', 'BUFFER_PROFILE*')
for i = 1, #all_profiles, 1 do
if all_profiles[i] ~= "BUFFER_PROFILE_TABLE_KEY_SET" and all_profiles[i] ~= "BUFFER_PROFILE_TABLE_DEL_SET" then
local xoff = redis.call('HGET', all_profiles[i], 'xoff')
if xoff then
lossless_profiles[all_profiles[i]] = true
local pool = redis.call('HGET', all_profiles[i], 'pool')
for j = 1, #ipools, 1 do
if "BUFFER_POOL|" .. pool == ipools[j] then
-- For ingress profiles, check whether it is lossless or lossy
-- For lossy profiles, there is buffer implicitly reserved when they are applied on PGs
local xoff = redis.call('HGET', all_profiles[i], 'xoff')
if xoff then
ingress_profile_is_lossless[all_profiles[i]] = true
else
ingress_profile_is_lossless[all_profiles[i]] = false
end
break
end
end
profiles[all_profiles[i]] = 0
end
Expand Down Expand Up @@ -289,9 +318,10 @@ local accumulative_xoff = 0
for name in pairs(profiles) do
if name ~= "BUFFER_PROFILE_TABLE_KEY_SET" and name ~= "BUFFER_PROFILE_TABLE_DEL_SET" then
local size = tonumber(redis.call('HGET', name, 'size'))
if size ~= nil then
if name == "BUFFER_PROFILE_TABLE:ingress_lossy_profile" then
size = size + lossypg_reserved
if size ~= nil then
-- Handle the implicitly reserved buffer for lossy profile applied on PG
if ingress_profile_is_lossless[name] == false then
size = size + lossypg_reserved
end
if size ~= 0 then
if shp_enabled and shp_size == 0 then
Expand All @@ -304,6 +334,8 @@ for name in pairs(profiles) do
accumulative_occupied_buffer = accumulative_occupied_buffer + size * profiles[name]
end
table.insert(statistics, {name, size, profiles[name]})
else
table.insert(statistics, {name, "-", profiles[name]})
end
end
end
Expand Down Expand Up @@ -336,7 +368,6 @@ redis.call('SELECT', config_db)

-- Fetch all the pools that need update
local pools_need_update = {}
local ipools = redis.call('KEYS', 'BUFFER_POOL|ingress*')
local ingress_pool_count = 0
local ingress_lossless_pool_size = nil
for i = 1, #ipools, 1 do
Expand All @@ -351,7 +382,6 @@ for i = 1, #ipools, 1 do
end
end

local epools = redis.call('KEYS', 'BUFFER_POOL|egress*')
for i = 1, #epools, 1 do
local size = redis.call('HGET', epools[i], 'size')
if not size then
Expand Down

0 comments on commit 84642f3

Please sign in to comment.