Skip to content

Commit 7f80f06

Browse files
authored
Td2: Reclaim buffer from unused ports (sonic-net#1830)
Signed-off-by: Neetha John <[email protected]> By default, lossless profiles are attached to PGs 3, 4 of all ports which results in a lot of buffer wastage when most of those ports are unused (not associated with a DEVICE_NEIGHBOR). In TD2, ingress pool size comprises of the reserved space as well. Hence making use of a special cable length of '0m' to identify unused ports and skip reserving space for those ports What I did * In buffermgr, if port with cable len '0m' is identified, return immediately without creating pg lossless profile or attaching profile to the lossless pgs of that port * Listen to 'admin_status' update as well from the PORT table and update port-speed mapping. This is to handle add-rack scenario where a port is added later - The port starts of with cable length 0m - Configlet to add a port is applied. The order of operations specifc to the PORT/CABLE_LENGTH table are - port is initially set to admin down, cable length is updated for that port, port table attributes are defined and port is set to admin up - speed update might not be seen when the port is set to admin up. Hence port-speed mapping will capture the speed update whenever its seen and once the cable length is updated while the port is brought back up, profiles can be attached to the lossless pgs of the port How I verified it - Manual tests done. Verified that no space is reserved for unused ports - Verified that when a port is added using 'add-rack' scenario, profile is attached to pgs of that port - New VS test added
1 parent 0fe2dfe commit 7f80f06

File tree

3 files changed

+168
-6
lines changed

3 files changed

+168
-6
lines changed

cfgmgr/buffermgr.cpp

+20-5
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ void BufferMgr::readPgProfileLookupFile(string file)
8686
task_process_status BufferMgr::doCableTask(string port, string cable_length)
8787
{
8888
m_cableLenLookup[port] = cable_length;
89+
SWSS_LOG_INFO("Cable length set to %s for port %s", m_cableLenLookup[port].c_str(), port.c_str());
8990
return task_process_status::task_success;
9091
}
9192

@@ -120,10 +121,11 @@ Create/update two tables: profile (in m_cfgBufferProfileTable) and port buffer (
120121
}
121122
}
122123
*/
123-
task_process_status BufferMgr::doSpeedUpdateTask(string port, string speed)
124+
task_process_status BufferMgr::doSpeedUpdateTask(string port)
124125
{
125126
vector<FieldValueTuple> fvVector;
126127
string cable;
128+
string speed;
127129

128130
if (m_cableLenLookup.count(port) == 0)
129131
{
@@ -132,7 +134,13 @@ task_process_status BufferMgr::doSpeedUpdateTask(string port, string speed)
132134
}
133135

134136
cable = m_cableLenLookup[port];
137+
if (cable == "0m")
138+
{
139+
SWSS_LOG_NOTICE("Not creating/updating PG profile for port %s. Cable length is set to %s", port.c_str(), cable.c_str());
140+
return task_process_status::task_success;
141+
}
135142

143+
speed = m_speedLookup[port];
136144
if (m_pgProfileLookup.count(speed) == 0 || m_pgProfileLookup[speed].count(cable) == 0)
137145
{
138146
SWSS_LOG_ERROR("Unable to create/update PG profile for port %s. No PG profile configured for speed %s and cable length %s",
@@ -368,11 +376,18 @@ void BufferMgr::doTask(Consumer &consumer)
368376
// receive and cache cable length table
369377
task_status = doCableTask(fvField(i), fvValue(i));
370378
}
371-
// In case of PORT table update, Buffer Manager is interested in speed update only
372-
if (m_pgfile_processed && table_name == CFG_PORT_TABLE_NAME && fvField(i) == "speed")
379+
if (m_pgfile_processed && table_name == CFG_PORT_TABLE_NAME && (fvField(i) == "speed" || fvField(i) == "admin_status"))
373380
{
374-
// create/update profile for port
375-
task_status = doSpeedUpdateTask(port, fvValue(i));
381+
if (fvField(i) == "speed")
382+
{
383+
m_speedLookup[port] = fvValue(i);
384+
}
385+
386+
if (m_speedLookup.count(port) != 0)
387+
{
388+
// create/update profile for port
389+
task_status = doSpeedUpdateTask(port);
390+
}
376391
}
377392
if (task_status != task_process_status::task_success)
378393
{

cfgmgr/buffermgr.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ typedef std::map<std::string, pg_profile_t> speed_map_t;
2727
typedef std::map<std::string, speed_map_t> pg_profile_lookup_t;
2828

2929
typedef std::map<std::string, std::string> port_cable_length_t;
30+
typedef std::map<std::string, std::string> port_speed_t;
3031

3132
class BufferMgr : public Orch
3233
{
@@ -52,10 +53,11 @@ class BufferMgr : public Orch
5253

5354
pg_profile_lookup_t m_pgProfileLookup;
5455
port_cable_length_t m_cableLenLookup;
56+
port_speed_t m_speedLookup;
5557
std::string getPgPoolMode();
5658
void readPgProfileLookupFile(std::string);
5759
task_process_status doCableTask(std::string port, std::string cable_length);
58-
task_process_status doSpeedUpdateTask(std::string port, std::string speed);
60+
task_process_status doSpeedUpdateTask(std::string port);
5961
void doBufferTableTask(Consumer &consumer, ProducerStateTable &applTable);
6062

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

tests/test_buffer_traditional.py

+145
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
import pytest
2+
import time
3+
4+
5+
class TestBuffer(object):
6+
LOSSLESS_PGS = [3, 4]
7+
INTF = "Ethernet0"
8+
9+
def setup_db(self, dvs):
10+
self.app_db = dvs.get_app_db()
11+
self.asic_db = dvs.get_asic_db()
12+
self.config_db = dvs.get_config_db()
13+
self.counter_db = dvs.get_counters_db()
14+
15+
# enable PG watermark
16+
self.set_pg_wm_status('enable')
17+
18+
def get_pg_oid(self, pg):
19+
fvs = dict()
20+
fvs = self.counter_db.get_entry("COUNTERS_PG_NAME_MAP", "")
21+
return fvs[pg]
22+
23+
def set_pg_wm_status(self, state):
24+
fvs = {'FLEX_COUNTER_STATUS': state}
25+
self.config_db.update_entry("FLEX_COUNTER_TABLE", "PG_WATERMARK", fvs)
26+
time.sleep(1)
27+
28+
def teardown(self):
29+
# disable PG watermark
30+
self.set_pg_wm_status('disable')
31+
32+
def get_asic_buf_profile(self):
33+
return set(self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_PROFILE"))
34+
35+
def check_new_profile_in_asic_db(self, profile):
36+
retry_count = 0
37+
self.new_profile = None
38+
while retry_count < 5:
39+
retry_count += 1
40+
diff = set(self.asic_db.get_keys("ASIC_STATE:SAI_OBJECT_TYPE_BUFFER_PROFILE")) - self.orig_profiles
41+
if len(diff) == 1:
42+
self.new_profile = diff.pop()
43+
break
44+
else:
45+
time.sleep(1)
46+
assert self.new_profile, "Can't get SAI OID for newly created profile {} after retry {} times".format(profile, retry_count)
47+
48+
def get_asic_buf_pg_profiles(self):
49+
self.buf_pg_profile = dict()
50+
for pg in self.pg_name_map:
51+
buf_pg_entries = self.asic_db.get_entry("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP", self.pg_name_map[pg])
52+
self.buf_pg_profile[pg] = buf_pg_entries["SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE"]
53+
54+
def change_cable_len(self, cable_len):
55+
fvs = dict()
56+
fvs[self.INTF] = cable_len
57+
self.config_db.update_entry("CABLE_LENGTH", "AZURE", fvs)
58+
59+
@pytest.fixture
60+
def setup_teardown_test(self, dvs):
61+
try:
62+
self.setup_db(dvs)
63+
pg_name_map = dict()
64+
for pg in self.LOSSLESS_PGS:
65+
pg_name = "{}:{}".format(self.INTF, pg)
66+
pg_name_map[pg_name] = self.get_pg_oid(pg_name)
67+
yield pg_name_map
68+
finally:
69+
self.teardown()
70+
71+
def test_zero_cable_len_profile_update(self, dvs, setup_teardown_test):
72+
self.pg_name_map = setup_teardown_test
73+
orig_cable_len = None
74+
orig_speed = None
75+
try:
76+
dvs.runcmd("config interface startup {}".format(self.INTF))
77+
self.orig_profiles = self.get_asic_buf_profile()
78+
# get orig cable length and speed
79+
fvs = self.config_db.get_entry("CABLE_LENGTH", "AZURE")
80+
orig_cable_len = fvs[self.INTF]
81+
fvs = self.config_db.get_entry("PORT", self.INTF)
82+
orig_speed = fvs["speed"]
83+
84+
if orig_speed == "100000":
85+
test_speed = "40000"
86+
elif orig_speed == "40000":
87+
test_speed = "100000"
88+
test_cable_len = "0m"
89+
90+
# check if the lossless profile for the test speed is already present
91+
fvs = dict()
92+
new_lossless_profile = "pg_lossless_{}_{}_profile".format(test_speed, orig_cable_len)
93+
fvs = self.app_db.get_entry("BUFFER_PROFILE_TABLE", new_lossless_profile)
94+
if len(fvs):
95+
profile_exp_cnt_diff = 0
96+
else:
97+
profile_exp_cnt_diff = 1
98+
99+
# get the orig buf profiles attached to the pgs
100+
self.get_asic_buf_pg_profiles()
101+
102+
# change cable length to 'test_cable_len'
103+
self.change_cable_len(test_cable_len)
104+
105+
# change intf speed to 'test_speed'
106+
dvs.runcmd("config interface speed {} {}".format(self.INTF, test_speed))
107+
test_lossless_profile = "pg_lossless_{}_{}_profile".format(test_speed, test_cable_len)
108+
# buffer profile should not get created
109+
self.app_db.wait_for_deleted_entry("BUFFER_PROFILE_TABLE", test_lossless_profile)
110+
111+
# buffer pgs should still point to the original buffer profile
112+
orig_lossless_profile = "pg_lossless_{}_{}_profile".format(orig_speed, orig_cable_len)
113+
self.app_db.wait_for_field_match("BUFFER_PG_TABLE", self.INTF + ":3-4", {"profile": "[BUFFER_PROFILE_TABLE:{}]".format(orig_lossless_profile)})
114+
fvs = dict()
115+
for pg in self.pg_name_map:
116+
fvs["SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE"] = self.buf_pg_profile[pg]
117+
self.asic_db.wait_for_field_match("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP", self.pg_name_map[pg], fvs)
118+
119+
# change cable length to 'orig_cable_len'
120+
self.change_cable_len(orig_cable_len)
121+
122+
# change intf speed to 'test_speed'
123+
dvs.runcmd("config interface speed {} {}".format(self.INTF, test_speed))
124+
if profile_exp_cnt_diff != 0:
125+
# new profile will get created
126+
self.app_db.wait_for_entry("BUFFER_PROFILE_TABLE", new_lossless_profile)
127+
self.check_new_profile_in_asic_db(new_lossless_profile)
128+
# verify that buffer pgs point to the new profile
129+
fvs = dict()
130+
for pg in self.pg_name_map:
131+
fvs["SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE"] = self.new_profile
132+
self.asic_db.wait_for_field_match("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP", self.pg_name_map[pg], fvs)
133+
134+
else:
135+
# verify that buffer pgs do not point to the old profile since we cannot deduce the new profile oid
136+
fvs = dict()
137+
for pg in self.pg_name_map:
138+
fvs["SAI_INGRESS_PRIORITY_GROUP_ATTR_BUFFER_PROFILE"] = self.buf_pg_profile[pg]
139+
self.asic_db.wait_for_field_negative_match("ASIC_STATE:SAI_OBJECT_TYPE_INGRESS_PRIORITY_GROUP", self.pg_name_map[pg], fvs)
140+
finally:
141+
if orig_cable_len:
142+
self.change_cable_len(orig_cable_len)
143+
if orig_speed:
144+
dvs.runcmd("config interface speed {} {}".format(self.INTF, orig_speed))
145+
dvs.runcmd("config interface shutdown {}".format(self.INTF))

0 commit comments

Comments
 (0)