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

Prevent Profile Creation for Invalid Cable Length #2

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
0abfe00
[SWSS:portsorch] fix child_ports checking in addLagMember and removeL…
baorliu Feb 24, 2025
82632ea
Added Change to Skip Route Programming if NH is link/oper down (#3520)
abdosi Feb 24, 2025
3a80d64
Set Port UPDATE_DSCP attribute when TC_TO_DSCP map is attached (#3517)
kperumalbfn Feb 24, 2025
887e3a5
Add appliance entry validation (#3494)
mukeshmv Feb 27, 2025
8c778bf
[smartswitch] Add support for ENI Based Forwarding (#3398)
vivekrnv Feb 27, 2025
a0fcac9
Initialize Port oper error map only once (#3538)
prgeor Mar 3, 2025
1b2a8e6
[copp]: Use non-zero trap priority for default trap group (#3502)
prabhataravind Mar 3, 2025
7a965ca
Optimize counter initialization by reducing the number of bulk counte…
stephenxs Mar 4, 2025
74e9e63
Prevent lossless profile creation for 0m cable
jianyuewu Feb 24, 2025
3494eae
Keep PGs, remove losslesss profiles when 0m cable
jianyuewu Mar 5, 2025
439ac82
Remove unnecessary prints and refactor UT
jianyuewu Mar 5, 2025
6a93999
Update log level to info
jianyuewu Mar 7, 2025
4b0bb1c
Merge branch 'master' into skip_0m_profile
jianyuewu Mar 7, 2025
8447919
Update test_macsec.py (#3549)
Pterosaur Mar 10, 2025
da18966
[MCLAG] Fix a race condition when moving MAC addresses to MCLAG peer …
puffc Mar 10, 2025
8398865
Update gitignore for fabricmgrd, stpmgrd, and the p4orch_tests binari…
theasianpianist Mar 11, 2025
ae4789c
Use software_bfd instead of switch_type. (#3525)
dypet Mar 11, 2025
92be63e
Merge branch 'master' into skip_0m_profile
jianyuewu Mar 12, 2025
a07838d
[orchagent] Do not restore port admin if port admin is configured (#3…
PJHsieh Mar 12, 2025
f763d20
Merge branch 'master' into skip_0m_profile
jianyuewu Mar 13, 2025
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
15 changes: 15 additions & 0 deletions cfgmgr/buffermgrdyn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1462,6 +1462,21 @@ task_process_status BufferMgrDynamic::refreshPgsForPort(const string &port, cons
continue;
}

// If cable len is 0m, remove lossless PG, keep lossy PG.
if (cable_length == "0m" && portPg.lossless)
{
if (oldProfile.empty())
{
SWSS_LOG_NOTICE("No lossless profile found for port %s when cable length is set to '0m'.", port.c_str());
continue;
}
updateBufferObjectToDb(key, oldProfile, false);
profilesToBeReleased.insert(oldProfile);
m_bufferProfileLookup[oldProfile].port_pgs.erase(key);
SWSS_LOG_NOTICE("All lossless profiles for port %s will be removed due to cable length being set to '0m'", port.c_str());
continue;
}

string threshold;
// Calculate new headroom size
if (portPg.static_configured)
Expand Down
156 changes: 149 additions & 7 deletions tests/mock_tests/buffermgrdyn_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,11 @@ namespace buffermgrdyn_test
{"size", "1024000"}
};

testBufferProfile["ingress_lossy_profile"] = {
{"dynamic_th", "7"},
{"pool", "ingress_lossless_pool"},
{"size", "0"}
};
testBufferProfile["ingress_lossless_profile"] = {
{"dynamic_th", "7"},
{"pool", "ingress_lossless_pool"},
Expand Down Expand Up @@ -522,8 +527,8 @@ namespace buffermgrdyn_test

InitDefaultBufferProfile();
appBufferProfileTable.getKeys(keys);
ASSERT_EQ(keys.size(), 3);
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.size(), 3);
ASSERT_EQ(keys.size(), 4);
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.size(), 4);
for (auto i : testBufferProfile)
{
CheckProfile(m_dynamicBuffer->m_bufferProfileLookup[i.first], testBufferProfile[i.first]);
Expand Down Expand Up @@ -647,7 +652,7 @@ namespace buffermgrdyn_test
appBufferPoolTable.getKeys(keys);
ASSERT_EQ(keys.size(), 3);
ASSERT_EQ(m_dynamicBuffer->m_bufferPoolLookup.size(), 3);
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.size(), 3);
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.size(), 4);
for (auto i : testBufferProfile)
{
CheckProfile(m_dynamicBuffer->m_bufferProfileLookup[i.first], testBufferProfile[i.first]);
Expand Down Expand Up @@ -933,8 +938,8 @@ namespace buffermgrdyn_test

InitDefaultBufferProfile();
appBufferProfileTable.getKeys(keys);
ASSERT_EQ(keys.size(), 3);
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.size(), 3);
ASSERT_EQ(keys.size(), 4);
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.size(), 4);
for (auto i : testBufferProfile)
{
CheckProfile(m_dynamicBuffer->m_bufferProfileLookup[i.first], testBufferProfile[i.first]);
Expand Down Expand Up @@ -1267,8 +1272,8 @@ namespace buffermgrdyn_test
ASSERT_EQ(keys.size(), 3);
InitDefaultBufferProfile();
appBufferProfileTable.getKeys(keys);
ASSERT_EQ(keys.size(), 3);
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.size(), 3);
ASSERT_EQ(keys.size(), 4);
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.size(), 4);

m_dynamicBuffer->m_bufferCompletelyInitialized = true;
m_dynamicBuffer->m_waitApplyAdditionalZeroProfiles = 0;
Expand Down Expand Up @@ -1471,4 +1476,141 @@ namespace buffermgrdyn_test
HandleTable(cableLengthTable);
ASSERT_EQ(m_dynamicBuffer->m_portInfoLookup["Ethernet12"].state, PORT_READY);
}

/*
Purpose: To verify the behavior of the buffer mgr dynamic when the cable length is set to "0m".
Here set to 0m indicates no lossless profile will be created, can still create lossy profile.
Steps:
1. Initialize the environment, including default lossless parameters and MMU size.
2. Start the Buffer Manager and initialize the port.
3. No new lossless profile is created when the cable length is "0m".
4. Existing lossless profiles are removed when the cable length is "0m".
5. No new lossless PG is created when the cable length is "0m".
6. When the cable length is updated to "5m", the correct lossless profiles and PGs are created.
7. When the cable length is updated back to "0m", the lossless profiles and PGs are removed.
8. Check if port MTU update, PG and profile still there.
9. When the cable length is "0m", lossy PGs remain, while lossless PGs are deleted.
*/
TEST_F(BufferMgrDynTest, SkipProfileCreationForZeroCableLength)
{
vector<FieldValueTuple> fieldValues;
vector<string> keys;

// 1. Initialize the environment, including default lossless parameters and MMU size.
InitDefaultLosslessParameter();
InitMmuSize();

// 2. Start the Buffer Manager and initialize the port.
StartBufferManager();

InitPort();
ASSERT_EQ(m_dynamicBuffer->m_portInfoLookup["Ethernet0"].state, PORT_INITIALIZING);

SetPortInitDone();
m_dynamicBuffer->doTask(m_selectableTable);

ASSERT_EQ(m_dynamicBuffer->m_bufferPoolLookup.size(), 0);
InitBufferPool();
ASSERT_EQ(m_dynamicBuffer->m_bufferPoolLookup.size(), 3);
appBufferPoolTable.getKeys(keys);
ASSERT_EQ(keys.size(), 3);

// Initialize buffer profiles
InitBufferPg("Ethernet0|3-4");
InitDefaultBufferProfile();
appBufferProfileTable.getKeys(keys);
ASSERT_EQ(keys.size(), 4);
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.size(), 4);
InitCableLength("Ethernet0", "5m");
ASSERT_EQ(m_dynamicBuffer->m_portInfoLookup["Ethernet0"].state, PORT_READY);

auto expectedProfile = "pg_lossless_100000_5m_profile";
CheckPg("Ethernet0", "Ethernet0:3-4", expectedProfile);

// 3. No new lossless profile is created when the cable length is "0m".
cableLengthTable.set("AZURE",
{
{"Ethernet0", "0m"}
});
HandleTable(cableLengthTable);
// Expect profile not created
auto zeroMProfile = "pg_lossless_100000_0m_profile";
ASSERT_EQ(m_dynamicBuffer->m_bufferProfileLookup.find(zeroMProfile), m_dynamicBuffer->m_bufferProfileLookup.end());
ASSERT_TRUE(m_dynamicBuffer->m_portPgLookup.find("Ethernet0:3-4") == m_dynamicBuffer->m_portPgLookup.end());

// 4. Existing lossless profiles are removed when the cable length is "0m".
// Since the cable length is set to 0m, previous profile for 5m should not exist.
ASSERT_TRUE(m_dynamicBuffer->m_bufferProfileLookup.find("pg_lossless_100000_5m_profile") == m_dynamicBuffer->m_bufferProfileLookup.end());

// 5. No new lossless PG is created when the cable length is "0m".
// The expectation is that no new PGs should be created with a cable length of 0m.
InitBufferPg("Ethernet0|6");
ASSERT_TRUE(m_dynamicBuffer->m_portPgLookup.find("Ethernet0:6") == m_dynamicBuffer->m_portPgLookup.end());

// 6. When the cable length is updated to "5m", the correct lossless profiles and PGs are created.
cableLengthTable.set("AZURE",
{
{"Ethernet0", "5m"}
});
HandleTable(cableLengthTable);
// Check if the profiles are created correctly
CheckPg("Ethernet0", "Ethernet0:3-4", "pg_lossless_100000_5m_profile");
CheckPg("Ethernet0", "Ethernet0:6", "pg_lossless_100000_5m_profile");

// 7. When the cable length is updated back to "0m", the lossless profiles and PGs are removed.
cableLengthTable.set("AZURE",
{
{"Ethernet0", "0m"}
});
HandleTable(cableLengthTable);
// Check that the profiles for 0m do not exist, 5m profile could be still there, because it is shared by other ports
ASSERT_TRUE(m_dynamicBuffer->m_bufferProfileLookup.find("pg_lossless_100000_0m_profile") == m_dynamicBuffer->m_bufferProfileLookup.end());
ASSERT_TRUE(m_dynamicBuffer->m_bufferProfileLookup.find("pg_lossless_100000_5m_profile") == m_dynamicBuffer->m_bufferProfileLookup.end());
// Check that the PGs for Ethernet0:3-4 and Ethernet0:6 have been deleted
ASSERT_TRUE(m_dynamicBuffer->m_portPgLookup.find("Ethernet0:3-4") == m_dynamicBuffer->m_portPgLookup.end());
ASSERT_TRUE(m_dynamicBuffer->m_portPgLookup.find("Ethernet0:6") == m_dynamicBuffer->m_portPgLookup.end());

// 8. Check if port MTU update, PG and profile still there.
cableLengthTable.set("AZURE",
{
{"Ethernet0", "5m"}
});
HandleTable(cableLengthTable);
string mtu = "4096";
m_dynamicBuffer->m_portInfoLookup["Ethernet0"].mtu = mtu;
// Check if the profile is created correctly
CheckPg("Ethernet0", "Ethernet0:3-4", "pg_lossless_100000_5m_profile");
CheckPg("Ethernet0", "Ethernet0:6", "pg_lossless_100000_5m_profile");

// 7. Check if lossy PG can still be created
InitBufferPg("Ethernet0|0", "ingress_lossy_profile");
cableLengthTable.set("AZURE",
{
{"Ethernet0", "0m"}
});
HandleTable(cableLengthTable);
bool found = false;
auto it = m_dynamicBuffer->m_portPgLookup.find("Ethernet0");
if (it != m_dynamicBuffer->m_portPgLookup.end()) {
found = (it->second.find("Ethernet0:0") != it->second.end());
}
ASSERT_TRUE(found);

// 9. When the cable length is "0m", lossy PGs remain, while lossless PGs are deleted.
cableLengthTable.set("AZURE",
{
{"Ethernet0", "0m"}
});
HandleTable(cableLengthTable);
it = m_dynamicBuffer->m_portPgLookup.find("Ethernet0");
if (it != m_dynamicBuffer->m_portPgLookup.end()) {
found = (it->second.find("Ethernet0:0") != it->second.end());
}
ASSERT_TRUE(found);
ASSERT_TRUE(m_dynamicBuffer->m_bufferProfileLookup.find("pg_lossless_100000_0m_profile") == m_dynamicBuffer->m_bufferProfileLookup.end());
ASSERT_TRUE(m_dynamicBuffer->m_bufferProfileLookup.find("pg_lossless_100000_5m_profile") == m_dynamicBuffer->m_bufferProfileLookup.end());
// Check that the PGs for Ethernet0:3-4 and Ethernet0:6 have been deleted
ASSERT_TRUE(m_dynamicBuffer->m_portPgLookup.find("Ethernet0:3-4") == m_dynamicBuffer->m_portPgLookup.end());
ASSERT_TRUE(m_dynamicBuffer->m_portPgLookup.find("Ethernet0:6") == m_dynamicBuffer->m_portPgLookup.end());
}
}