Skip to content

Commit

Permalink
Do not apply QoS mapping item on the switch until the object is creat…
Browse files Browse the repository at this point in the history
…ed (sonic-net#3163)

What I did

Do not apply the global DSCP to TC map to the switch object until the mapping object has been created.

Why I did it

Fix issue: if orchagent handles tables in the following order, it will fail in step 1 and the configure will never applied.

PORT_QOS_MAP|global object and then DSCP_TO_TC object
  • Loading branch information
stephenxs authored and mssonicbld committed Jun 4, 2024
1 parent c52f084 commit 3accce4
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 0 deletions.
1 change: 1 addition & 0 deletions orchagent/qosorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2018,6 +2018,7 @@ task_process_status QosOrch::handleGlobalQosMap(const string &OP, KeyOpFieldsVal
{
SWSS_LOG_INFO("Global QoS map %s is not yet created", map_name.c_str());
task_status = task_process_status::task_need_retry;
continue;
}

if (applyDscpToTcMapToSwitch(SAI_SWITCH_ATTR_QOS_DSCP_TO_TC_MAP, id))
Expand Down
31 changes: 31 additions & 0 deletions tests/mock_tests/qosorch_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1160,6 +1160,7 @@ namespace qosorch_test
static_cast<Orch *>(gQosOrch)->doTask();
// Check DSCP_TO_TC_MAP|AZURE is applied to switch
ASSERT_EQ((*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME])["AZURE"].m_saiObjectId, switch_dscp_to_tc_map_id);
CheckDependency(CFG_PORT_QOS_MAP_TABLE_NAME, "global", "dscp_to_tc_map", CFG_DSCP_TO_TC_MAP_TABLE_NAME, "AZURE");

// Remove global DSCP_TO_TC_MAP
entries.push_back({"global", "DEL", {}});
Expand All @@ -1182,7 +1183,37 @@ namespace qosorch_test
// Check DSCP_TO_TC_MAP|AZURE is removed, and the switch_level dscp_to_tc_map is set to NULL
ASSERT_EQ(current_sai_remove_qos_map_count + 1, sai_remove_qos_map_count);
ASSERT_EQ((*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME]).count("AZURE"), 0);

// Run the test in reverse order
entries.push_back({"global", "SET",
{
{"dscp_to_tc_map", "AZURE"}
}});
consumer = dynamic_cast<Consumer *>(gQosOrch->getExecutor(CFG_PORT_QOS_MAP_TABLE_NAME));
consumer->addToSync(entries);
entries.clear();

// Try draining PORT_QOS_MAP table
static_cast<Orch *>(gQosOrch)->doTask();
// Check DSCP_TO_TC_MAP|AZURE is applied to switch
CheckDependency(CFG_PORT_QOS_MAP_TABLE_NAME, "global", "dscp_to_tc_map", CFG_DSCP_TO_TC_MAP_TABLE_NAME);
ASSERT_EQ((*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME])["AZURE"].m_saiObjectId, switch_dscp_to_tc_map_id);

entries.push_back({"AZURE", "SET",
{
{"1", "0"},
{"0", "1"}
}});

consumer = dynamic_cast<Consumer *>(gQosOrch->getExecutor(CFG_DSCP_TO_TC_MAP_TABLE_NAME));
consumer->addToSync(entries);
entries.clear();

// Try draining DSCP_TO_TC_MAP and PORT_QOS_MAP table
static_cast<Orch *>(gQosOrch)->doTask();
// Check DSCP_TO_TC_MAP|AZURE is applied to switch
CheckDependency(CFG_PORT_QOS_MAP_TABLE_NAME, "global", "dscp_to_tc_map", CFG_DSCP_TO_TC_MAP_TABLE_NAME, "AZURE");
ASSERT_EQ((*QosOrch::getTypeMap()[CFG_DSCP_TO_TC_MAP_TABLE_NAME])["AZURE"].m_saiObjectId, switch_dscp_to_tc_map_id);
}

TEST_F(QosOrchTest, QosOrchTestRetryFirstItem)
Expand Down

0 comments on commit 3accce4

Please sign in to comment.