Skip to content

Commit

Permalink
[QoS] Enforce drop probability only for colors whose WRED are enabled (…
Browse files Browse the repository at this point in the history 
…sonic-net#2422)

What I did
Do not enforce drop probability for a color whose WRED is disabled.

Signed-off-by: Stephen Sun [email protected]

Why I did it
Currently, there is a logic to enforce the drop probability if it is not explicitly designated for a color. However, the drop probability is not a mandatory attribute. It can incur vendor SAI complaints to set it when the color is disabled.
The logic was introduced from the very beginning (by PR sonic-net#571) because no drop probability was defined in the QoS template at the time, which is no longer true.
So we will enforce drop probability only if it is not configured and the color is enabled.

How I verified it
Unit test and manual test
  • Loading branch information
@stephenxs
stephenxs authored Sep 14, 2022
1 parent 05c5c2f commit efa0f01
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 10 deletions.
48 changes: 38 additions & 10 deletions orchagent/qosorch.cpp
View file
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
View file
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

0 comments on commit efa0f01

Please sign in to comment.