Skip to content

Commit 46699ab

Browse files
committed
Prevent profile creation for invalid cable length
Added a check to skip profile creation when the cable length is 0m. This change ensures that profiles are not created for invalid cable lengths, preventing potential configuration errors. Logged a notice message when skipping profile creation due to 0m cable length. Signed-off-by: Jianyue Wu <jianyuew@nvidia.com>
1 parent 4eb74f0 commit 46699ab

File tree

2 files changed

+91
-0
lines changed

2 files changed

+91
-0
lines changed

cfgmgr/buffermgrdyn.cpp

+18
Original file line numberDiff line numberDiff line change
@@ -1440,6 +1440,23 @@ task_process_status BufferMgrDynamic::refreshPgsForPort(const string &port, cons
14401440

14411441
SWSS_LOG_NOTICE("Refresh priority groups for port %s", port.c_str());
14421442

1443+
if (cable_length == "0m")
1444+
{
1445+
SWSS_LOG_NOTICE("Skipping profile creation for cable length '0m'");
1446+
auto &portPgs = m_portPgLookup[port];
1447+
for (auto &pg : portPgs)
1448+
{
1449+
if (!pg.second.running_profile_name.empty())
1450+
{
1451+
m_bufferProfileLookup.erase(pg.second.running_profile_name);
1452+
}
1453+
doRemovePgTask(pg.first, port);
1454+
}
1455+
1456+
SWSS_LOG_NOTICE("All profiles for port %s have been removed due to cable length being set to '0m'", port.c_str());
1457+
return task_process_status::task_success;
1458+
}
1459+
14431460
// Iterate all the lossless PGs configured on this port
14441461
for (auto it = portPgs.begin(); it != portPgs.end(); ++it)
14451462
{
@@ -1474,6 +1491,7 @@ task_process_status BufferMgrDynamic::refreshPgsForPort(const string &port, cons
14741491
{
14751492
threshold = m_defaultThreshold;
14761493
}
1494+
14771495
newProfile = getDynamicProfileName(speed, cable_length, mtu, threshold, gearbox_model, laneCount);
14781496
auto rc = allocateProfile(speed, cable_length, mtu, threshold, gearbox_model, laneCount, newProfile);
14791497
if (task_process_status::task_success != rc)

tests/mock_tests/buffermgrdyn_ut.cpp

+73
Original file line numberDiff line numberDiff line change
@@ -1471,4 +1471,77 @@ namespace buffermgrdyn_test
14711471
HandleTable(cableLengthTable);
14721472
ASSERT_EQ(m_dynamicBuffer->m_portInfoLookup["Ethernet12"].state, PORT_READY);
14731473
}
1474+
1475+
TEST_F(BufferMgrDynTest, SkipProfileCreationForZeroCableLength)
1476+
{
1477+
vector<FieldValueTuple> fieldValues;
1478+
vector<string> keys;
1479+
1480+
// Prepare information that will be read at the beginning
1481+
InitDefaultLosslessParameter();
1482+
InitMmuSize();
1483+
1484+
StartBufferManager();
1485+
1486+
InitPort();
1487+
ASSERT_EQ(m_dynamicBuffer->m_portInfoLookup["Ethernet0"].state, PORT_INITIALIZING);
1488+
1489+
SetPortInitDone();
1490+
// Timer will be called
1491+
m_dynamicBuffer->doTask(m_selectableTable);
1492+
1493+
ASSERT_EQ(m_dynamicBuffer->m_bufferPoolLookup.size(), 0);
1494+
InitBufferPool();
1495+
ASSERT_EQ(m_dynamicBuffer->m_bufferPoolLookup.size(), 3);
1496+
appBufferPoolTable.getKeys(keys);
1497+
ASSERT_EQ(keys.size(), 3);
1498+
1499+
// Initialize buffer profiles
1500+
InitBufferPg("Ethernet0|3-4");
1501+
InitDefaultBufferProfile();
1502+
appBufferProfileTable.getKeys(keys);
1503+
ASSERT_EQ(keys.size(), 3);
1504+
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.size(), 3);
1505+
1506+
// 1. Set cable length to "0m"
1507+
InitCableLength("Ethernet0", "0m");
1508+
ASSERT_EQ(m_dynamicBuffer->m_portInfoLookup["Ethernet0"].state, PORT_READY);
1509+
// Expect profile not created
1510+
auto zeroMProfile = "pg_lossless_100000_0m_profile";
1511+
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.find(zeroMProfile), m_dynamicBuffer->m_bufferProfileLookup.end());
1512+
1513+
// 2. Check if the reference for 5m profile of this port had been deleted in buffer PG table
1514+
// Since the cable length is set to 0m, previous profile for 5m should not exist.
1515+
ASSERT_TRUE(m_dynamicBuffer->m_bufferProfileLookup.find("pg_lossless_100000_5m_profile") == m_dynamicBuffer->m_bufferProfileLookup.end());
1516+
1517+
// 3. Add a PG, Ethernet0:6, it should not be created with 0m profile
1518+
// The expectation is that no new PGs should be created with a cable length of 0m.
1519+
InitBufferPg("Ethernet0|6");
1520+
ASSERT_TRUE(m_dynamicBuffer->m_portPgLookup.find("Ethernet0:6") == m_dynamicBuffer->m_portPgLookup.end());
1521+
1522+
// 4. Update cable length to 5m, then Ethernet0:6 and Ethernet0:3-4 should be created with 5m profile
1523+
InitCableLength("Ethernet0", "5m");
1524+
m_dynamicBuffer->doTask();
1525+
// Check if the profiles are created correctly
1526+
CheckPg("Ethernet0", "Ethernet0:3-4", "pg_lossless_100000_5m_profile");
1527+
CheckPg("Ethernet0", "Ethernet0:6", "pg_lossless_100000_5m_profile");
1528+
1529+
// 5. Update cable length to 0m, then Ethernet0:6 should be deleted, Ethernet0:3-4 should be deleted and 0m profile also not exist, and 5m profile should be deleted, PG also deleted, as profiles not exist
1530+
InitCableLength("Ethernet0", "0m");
1531+
m_dynamicBuffer->doTask();
1532+
// Check that the profiles for 0m and 5m both do not exist
1533+
ASSERT_TRUE(m_dynamicBuffer->m_bufferProfileLookup.find("pg_lossless_100000_0m_profile") == m_dynamicBuffer->m_bufferProfileLookup.end());
1534+
ASSERT_TRUE(m_dynamicBuffer->m_bufferProfileLookup.find("pg_lossless_100000_5m_profile") == m_dynamicBuffer->m_bufferProfileLookup.end());
1535+
// Check that the PGs for Ethernet0:3-4 and Ethernet0:6 have been deleted
1536+
ASSERT_TRUE(m_dynamicBuffer->m_portPgLookup.find("Ethernet0:3-4") == m_dynamicBuffer->m_portPgLookup.end());
1537+
ASSERT_TRUE(m_dynamicBuffer->m_portPgLookup.find("Ethernet0:6") == m_dynamicBuffer->m_portPgLookup.end());
1538+
1539+
// 6. Check if port MTU update, PG and profile still there
1540+
InitCableLength("Ethernet0", "5m");
1541+
string mtu = "4096";
1542+
m_dynamicBuffer->m_portInfoLookup["Ethernet0"].mtu = mtu;
1543+
// Check if the profile is created correctly
1544+
CheckPg("Ethernet0", "Ethernet0:3-4", "pg_lossless_100000_5m_profile");
1545+
CheckPg("Ethernet0", "Ethernet0:6", "pg_lossless_100000_5m_profile");
1546+
}
14741547
}

0 commit comments

Comments
 (0)