Skip to content

Commit c7b3b59

Browse files
Add check for empty m_priority_group_ids in initializePriorityGroupsBulk
1 parent 3ccfa62 commit c7b3b59

File tree

2 files changed

+110
-1
lines changed

2 files changed

+110
-1
lines changed

orchagent/portsorch.cpp

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6464,10 +6464,16 @@ void PortsOrch::initializePriorityGroupsBulk(std::vector<Port>& ports)
64646464

64656465
bulker.executeGet();
64666466

6467+
// Track separate bulker index for verification
6468+
size_t bulker_idx = 0;
64676469
for (size_t idx = 0; idx < portCount; idx++)
64686470
{
64696471
const auto& port = ports[idx];
6470-
const auto status = bulker.statuses[idx];
6472+
if (port.m_priority_group_ids.size() == 0)
6473+
{
6474+
continue;
6475+
}
6476+
const auto status = bulker.statuses[bulker_idx];
64716477

64726478
if (status != SAI_STATUS_SUCCESS)
64736479
{
@@ -6477,6 +6483,7 @@ void PortsOrch::initializePriorityGroupsBulk(std::vector<Port>& ports)
64776483
}
64786484

64796485
SWSS_LOG_INFO("Get priority groups for port %s", port.m_alias.c_str());
6486+
bulker_idx++;
64806487
}
64816488
}
64826489
}

tests/mock_tests/portsorch_ut.cpp

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3860,6 +3860,69 @@ namespace portsorch_test
38603860
ASSERT_FALSE(bridgePortCalledBeforeLagMember); // bridge port created on lag before lag member was created
38613861
}
38623862

3863+
// Mock variables for priority groups test
3864+
// Map of port OID to number of priority groups
3865+
std::map<sai_object_id_t, uint32_t> mock_port_pg_counts;
3866+
3867+
sai_status_t _ut_stub_sai_get_ports_attribute_zero_pg(
3868+
_In_ uint32_t object_count,
3869+
_In_ const sai_object_id_t *object_id,
3870+
_In_ const uint32_t *attr_count,
3871+
_Inout_ sai_attribute_t **attr_list,
3872+
_In_ sai_bulk_op_error_mode_t mode,
3873+
_Out_ sai_status_t *object_statuses)
3874+
{
3875+
// Implement the mock ourselves instead of calling original
3876+
for (uint32_t i = 0; i < object_count; i++)
3877+
{
3878+
if (attr_count[i] == 1 &&
3879+
attr_list[i] != nullptr &&
3880+
attr_list[i][0].id == SAI_PORT_ATTR_NUMBER_OF_INGRESS_PRIORITY_GROUPS)
3881+
{
3882+
// Check if we have a configured PG count for this port
3883+
auto it = mock_port_pg_counts.find(object_id[i]);
3884+
if (it != mock_port_pg_counts.end())
3885+
{
3886+
// Return configured priority group count
3887+
attr_list[i][0].value.u32 = it->second;
3888+
object_statuses[i] = SAI_STATUS_SUCCESS;
3889+
}
3890+
else
3891+
{
3892+
// Return default 8 priority groups for unconfigured ports
3893+
attr_list[i][0].value.u32 = 8;
3894+
object_statuses[i] = SAI_STATUS_SUCCESS;
3895+
}
3896+
}
3897+
else if (attr_count[i] == 1 &&
3898+
attr_list[i] != nullptr &&
3899+
attr_list[i][0].id == SAI_PORT_ATTR_INGRESS_PRIORITY_GROUP_LIST)
3900+
{
3901+
// For the priority group list query, just return success
3902+
// The list is already allocated by the caller
3903+
object_statuses[i] = SAI_STATUS_SUCCESS;
3904+
}
3905+
else
3906+
{
3907+
// For other attributes, call the original implementation
3908+
object_statuses[i] = pold_sai_port_api->get_port_attribute(
3909+
object_id[i], attr_count[i], attr_list[i]);
3910+
}
3911+
}
3912+
3913+
return SAI_STATUS_SUCCESS;
3914+
}
3915+
3916+
void _hook_sai_port_api_zero_pg()
3917+
{
3918+
ut_sai_port_api = *sai_port_api;
3919+
pold_sai_port_api = sai_port_api;
3920+
ut_sai_port_api.get_port_attribute = _ut_stub_sai_get_port_attribute;
3921+
ut_sai_port_api.set_port_attribute = _ut_stub_sai_set_port_attribute;
3922+
ut_sai_port_api.get_ports_attribute = _ut_stub_sai_get_ports_attribute_zero_pg;
3923+
sai_port_api = &ut_sai_port_api;
3924+
}
3925+
38633926
struct PostPortInitTests : PortsOrchTest
38643927
{
38653928
};
@@ -3907,5 +3970,44 @@ namespace portsorch_test
39073970
// Now the field "max_priority_groups" is set
39083971
stateDbSet = stateTable.hget("Ethernet0", "max_priority_groups", value);
39093972
ASSERT_TRUE(stateDbSet);
3973+
3974+
// Test case for ports with mixed priority group configurations
3975+
// Test pattern: [8 PGs, 0 PGs, 8 PGs, 0 PGs, 8 PGs]
3976+
// This tests the bulker index mapping with multiple zero-PG ports at different positions
3977+
std::vector<std::string> port_names = {"Ethernet0", "Ethernet4", "Ethernet8", "Ethernet12", "Ethernet16"};
3978+
std::vector<uint32_t> expected_pg_counts = {8, 0, 8, 0, 8};
3979+
std::vector<Port> test_ports;
3980+
3981+
// Get ports and configure mock PG counts
3982+
for (size_t i = 0; i < port_names.size(); i++)
3983+
{
3984+
Port port;
3985+
ASSERT_TRUE(gPortsOrch->getPort(port_names[i], port));
3986+
3987+
// Clear existing priority group IDs to simulate fresh initialization
3988+
port.m_priority_group_ids.clear();
3989+
3990+
// Configure mock to return specific PG count for this port
3991+
mock_port_pg_counts[port.m_port_id] = expected_pg_counts[i];
3992+
3993+
test_ports.push_back(port);
3994+
}
3995+
3996+
// Hook SAI API with our mock
3997+
_hook_sai_port_api_zero_pg();
3998+
3999+
// Initialize priority groups in bulk - should handle mixed scenario correctly
4000+
ASSERT_NO_THROW(gPortsOrch->initializePriorityGroupsBulk(test_ports));
4001+
4002+
// Verify each port has the expected number of priority groups
4003+
for (size_t i = 0; i < test_ports.size(); i++)
4004+
{
4005+
ASSERT_EQ(test_ports[i].m_priority_group_ids.size(), expected_pg_counts[i])
4006+
<< "Port " << port_names[i] << " has incorrect PG count";
4007+
}
4008+
4009+
// Cleanup: unhook SAI API and reset mock variables
4010+
_unhook_sai_port_api();
4011+
mock_port_pg_counts.clear();
39104012
}
39114013
}

0 commit comments

Comments
 (0)