[orchagent]: Support for setting switch level DSCP to TC QoS map #2023
Conversation
vmittal-msft
force-pushed
the
vmittal/dscp_to_tc
branch
from
0abd30a
to
97062de
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
orchagent/qosorch.cpp
Outdated
| @@ -20,7 +21,9 @@ extern sai_qos_map_api_t *sai_qos_map_api; | |||
| extern sai_scheduler_group_api_t *sai_scheduler_group_api; | |||
| extern sai_switch_api_t *sai_switch_api; | |||
| extern sai_acl_api_t* sai_acl_api; | |||
| extern bool querySwitchDscpToTcCapability(sai_object_type_t, sai_attr_id_t); | |||
There was a problem hiding this comment.
this extern is not required
There was a problem hiding this comment.
This function is defined in another file hence added extern.
| @@ -12,6 +12,7 @@ | |||
|
|
|||
| using namespace std; | |||
|
|
|||
| extern sai_switch_api_t *sai_switch_api; | |||
There was a problem hiding this comment.
this is already done in line 22
orchagent/qosorch.cpp
Outdated
| if (rv == false) | ||
| { | ||
| SWSS_LOG_ERROR("Switch level DSCP to TC QoS map configuration is not supported"); | ||
| return false; |
There was a problem hiding this comment.
why is it returned from here? IMO, we should check query capability and apply the switch setting if capability is true. Another suggestion is to make it part of applyDscpToTcMapToSwitch
There was a problem hiding this comment.
If it is true the following code is applying DSCP to TC QoS map at switch.
orchagent/switchorch.cpp
Outdated
| SWSS_LOG_WARN("Could not query switch level DSCP to TC map %d", status); | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
You have to check capability.set_implemented is true as well, before returning from this function
There was a problem hiding this comment.
I didn't check for capability.set_implemented to keep changes in SAI code to minimum. Please let me know if this is still needed.
vmittal-msft
force-pushed
the
vmittal/dscp_to_tc
branch
from
97062de
to
6516be2
Compare
orchagent/qosorch.cpp
Outdated
| extern sai_acl_api_t* sai_acl_api; | ||
| bool querySwitchDscpToTcCapability(sai_object_type_t, sai_attr_id_t); |
There was a problem hiding this comment.
Please remove this. It is not required
orchagent/qosorch.cpp
Outdated
| sai_object_id_t DscpToTcMapHandler::addQosItem(const vector<sai_attribute_t> &attributes) | ||
| { | ||
| SWSS_LOG_ENTER(); | ||
| sai_status_t sai_status; | ||
| sai_object_id_t sai_object; | ||
| vector<sai_attribute_t> qos_map_attrs; | ||
| bool rv; |
orchagent/qosorch.cpp
Outdated
| @@ -231,6 +262,10 @@ sai_object_id_t DscpToTcMapHandler::addQosItem(const vector<sai_attribute_t> &at | |||
| return SAI_NULL_OBJECT_ID; | |||
| } | |||
| SWSS_LOG_DEBUG("created QosMap object:%" PRIx64, sai_object); | |||
|
|
|||
|
|
|||
vmittal-msft
force-pushed
the
vmittal/dscp_to_tc
branch
from
6516be2
to
ba154c6
Compare
Signed-off-by: Vineet Mittal <vineetmittal@microsoft.com>
vmittal-msft
force-pushed
the
vmittal/dscp_to_tc
branch
from
ba154c6
to
3fcb158
Compare
|
/azp run |
1 similar comment
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
This commit could not be cleanly cherry-pick to 202012. Please submit another PR. |
What I did
Added support for setting switch level DSCP to TC QoS map.
Why I did it
This is required to set correct DSCP -> TC QoS map for tunnels else they will point to some default QoS map (DSCP/8 = TC) which is not correct.
How I verified it
This needs SAI change to support this capability. Verified this to be working fine on T0 testbed with mock dual ToR topology.
Details if related