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

[QoS] Enforce drop probability only for colors whose WRED are enabled #2422

Merged
merged 4 commits into from
Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
48 changes: 38 additions & 10 deletions orchagent/qosorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ enum {
RED_DROP_PROBABILITY_SET = (1U << 2)
};

enum {
GREEN_WRED_ENABLED = (1U << 0),
YELLOW_WRED_ENABLED = (1U << 1),
RED_WRED_ENABLED = (1U << 2)
};

// field_name is what is expected in CONFIG_DB PORT_QOS_MAP table
map<string, sai_port_attr_t> qos_to_attr_map = {
{dscp_to_tc_field_name, SAI_PORT_ATTR_QOS_DSCP_TO_TC_MAP},
Expand Down Expand Up @@ -720,6 +726,7 @@ sai_object_id_t WredMapHandler::addQosItem(const vector<sai_attribute_t> &attrib
sai_attribute_t attr;
vector<sai_attribute_t> attrs;
uint8_t drop_prob_set = 0;
uint8_t wred_enable_set = 0;

attr.id = SAI_WRED_ATTR_WEIGHT;
attr.value.s32 = 0;
Expand All @@ -729,32 +736,53 @@ sai_object_id_t WredMapHandler::addQosItem(const vector<sai_attribute_t> &attrib
{
attrs.push_back(attrib);

if (attrib.id == SAI_WRED_ATTR_GREEN_DROP_PROBABILITY)
switch (attrib.id)
{
case SAI_WRED_ATTR_GREEN_ENABLE:
if (attrib.value.booldata)
{
wred_enable_set |= GREEN_WRED_ENABLED;
}
break;
case SAI_WRED_ATTR_YELLOW_ENABLE:
if (attrib.value.booldata)
{
wred_enable_set |= YELLOW_WRED_ENABLED;
}
break;
case SAI_WRED_ATTR_RED_ENABLE:
if (attrib.value.booldata)
{
wred_enable_set |= RED_WRED_ENABLED;
}
break;
case SAI_WRED_ATTR_GREEN_DROP_PROBABILITY:
drop_prob_set |= GREEN_DROP_PROBABILITY_SET;
}
else if (attrib.id == SAI_WRED_ATTR_YELLOW_DROP_PROBABILITY)
{
break;
case SAI_WRED_ATTR_YELLOW_DROP_PROBABILITY:
drop_prob_set |= YELLOW_DROP_PROBABILITY_SET;
}
else if (attrib.id == SAI_WRED_ATTR_RED_DROP_PROBABILITY)
{
break;
case SAI_WRED_ATTR_RED_DROP_PROBABILITY:
drop_prob_set |= RED_DROP_PROBABILITY_SET;
break;
default:
break;
}
}
if (!(drop_prob_set & GREEN_DROP_PROBABILITY_SET))

if (!(drop_prob_set & GREEN_DROP_PROBABILITY_SET) && (wred_enable_set & GREEN_WRED_ENABLED))
{
attr.id = SAI_WRED_ATTR_GREEN_DROP_PROBABILITY;
attr.value.s32 = 100;
attrs.push_back(attr);
}
if (!(drop_prob_set & YELLOW_DROP_PROBABILITY_SET))
if (!(drop_prob_set & YELLOW_DROP_PROBABILITY_SET) && (wred_enable_set & YELLOW_WRED_ENABLED))
{
attr.id = SAI_WRED_ATTR_YELLOW_DROP_PROBABILITY;
attr.value.s32 = 100;
attrs.push_back(attr);
}
if (!(drop_prob_set & RED_DROP_PROBABILITY_SET))
if (!(drop_prob_set & RED_DROP_PROBABILITY_SET) && (wred_enable_set & RED_WRED_ENABLED))
{
attr.id = SAI_WRED_ATTR_RED_DROP_PROBABILITY;
attr.value.s32 = 100;
Expand Down
90 changes: 90 additions & 0 deletions tests/mock_tests/qosorch_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ namespace qosorch_test
sai_set_switch_attribute_fn old_set_switch_attribute_fn;
sai_switch_api_t ut_sai_switch_api, *pold_sai_switch_api;

typedef struct
{
sai_uint32_t green_max_drop_probability;
sai_uint32_t yellow_max_drop_probability;
sai_uint32_t red_max_drop_probability;
} qos_wred_max_drop_probability_t;

sai_status_t _ut_stub_sai_set_switch_attribute(sai_object_id_t switch_id, const sai_attribute_t *attr)
{
auto rc = old_set_switch_attribute_fn(switch_id, attr);
Expand All @@ -55,6 +62,7 @@ namespace qosorch_test

bool testing_wred_thresholds;
WredMapHandler::qos_wred_thresholds_t saiThresholds;
qos_wred_max_drop_probability_t saiMaxDropProbabilities;
void _ut_stub_sai_check_wred_attributes(const sai_attribute_t &attr)
{
if (!testing_wred_thresholds)
Expand Down Expand Up @@ -88,6 +96,15 @@ namespace qosorch_test
ASSERT_TRUE(!saiThresholds.red_max_threshold || saiThresholds.red_max_threshold > attr.value.u32);
saiThresholds.red_min_threshold = attr.value.u32;
break;
case SAI_WRED_ATTR_GREEN_DROP_PROBABILITY:
saiMaxDropProbabilities.green_max_drop_probability = attr.value.u32;
break;
case SAI_WRED_ATTR_YELLOW_DROP_PROBABILITY:
saiMaxDropProbabilities.yellow_max_drop_probability = attr.value.u32;
break;
case SAI_WRED_ATTR_RED_DROP_PROBABILITY:
saiMaxDropProbabilities.red_max_drop_probability = attr.value.u32;
break;
default:
break;
}
Expand Down Expand Up @@ -132,6 +149,23 @@ namespace qosorch_test
ASSERT_TRUE(ts.empty());
}

void updateMaxDropProbabilityAndCheck(string name, vector<FieldValueTuple> &maxDropProbabilityVector, qos_wred_max_drop_probability_t &maxDropProbabilities)
{
std::deque<KeyOpFieldsValuesTuple> entries;
vector<string> ts;
entries.push_back({name, "SET", maxDropProbabilityVector});
auto consumer = dynamic_cast<Consumer *>(gQosOrch->getExecutor(CFG_WRED_PROFILE_TABLE_NAME));
consumer->addToSync(entries);
entries.clear();
saiMaxDropProbabilities.green_max_drop_probability = 0;
saiMaxDropProbabilities.yellow_max_drop_probability = 0;
saiMaxDropProbabilities.red_max_drop_probability = 0;
static_cast<Orch *>(gQosOrch)->doTask();
ASSERT_EQ(saiMaxDropProbabilities.green_max_drop_probability, maxDropProbabilities.green_max_drop_probability);
ASSERT_EQ(saiMaxDropProbabilities.yellow_max_drop_probability, maxDropProbabilities.yellow_max_drop_probability);
ASSERT_EQ(saiMaxDropProbabilities.red_max_drop_probability, maxDropProbabilities.red_max_drop_probability);
}

sai_status_t _ut_stub_sai_create_wred(
_Out_ sai_object_id_t *wred_id,
_In_ sai_object_id_t switch_id,
Expand Down Expand Up @@ -1338,6 +1372,62 @@ namespace qosorch_test
testing_wred_thresholds = false;
}

TEST_F(QosOrchTest, QosOrchTestWredDropProbability)
{
testing_wred_thresholds = true;

// The order of fields matters when the wred profile is updated from the upper set to the lower set
// It should be max, min for each color. In this order, the new max is less then the current min
// QoS orchagent should guarantee that the new min is configured first and then new max
vector<FieldValueTuple> greenProfile = {
{"wred_green_enable", "true"},
{"wred_yellow_enable", "false"},
};
qos_wred_max_drop_probability_t greenProbabilities = {
100, // green_max_drop_probability
0, // yellow_max_drop_probability
0 // red_max_drop_probability
};
updateMaxDropProbabilityAndCheck("green_default", greenProfile, greenProbabilities);

greenProfile.push_back({"green_drop_probability", "5"});
greenProbabilities.green_max_drop_probability = 5;
updateMaxDropProbabilityAndCheck("green", greenProfile, greenProbabilities);

vector<FieldValueTuple> yellowProfile = {
{"wred_yellow_enable", "true"},
{"wred_red_enable", "false"},
};
qos_wred_max_drop_probability_t yellowProbabilities = {
0, // green_max_drop_probability
100, // yellow_max_drop_probability
0 // red_max_drop_probability
};
updateMaxDropProbabilityAndCheck("yellow_default", yellowProfile, yellowProbabilities);

yellowProfile.push_back({"yellow_drop_probability", "5"});
yellowProbabilities.yellow_max_drop_probability = 5;
updateMaxDropProbabilityAndCheck("yellow", yellowProfile, yellowProbabilities);

vector<FieldValueTuple> redProfile = {
{"wred_green_enable", "false"},
{"wred_red_enable", "true"},
};
qos_wred_max_drop_probability_t redProbabilities = {
0, // green_max_drop_probability
0, // yellow_max_drop_probability
100 // red_max_drop_probability
};
updateMaxDropProbabilityAndCheck("red_default", redProfile, redProbabilities);

redProfile.push_back({"red_drop_probability", "5"});
redProbabilities.red_max_drop_probability = 5;
updateMaxDropProbabilityAndCheck("red", redProfile, redProbabilities);

testing_wred_thresholds = false;
}


/*
* Make sure empty fields won't cause orchagent crash
*/
Expand Down