Support ecn on/off on a queue #577
Conversation
orchagent/orch.cpp
Outdated
| } | ||
| } | ||
|
|
||
| SWSS_LOG_ERROR("malformed reference:%s. Must contain 1 token or 2 tokens\n", ref_content.c_str()); |
There was a problem hiding this comment.
the nested tokenization is a little bit confusing to me. could you add some sample format of the string before tokenization? also, in the line 261, if the tokens.size() is not 2, then anyway it will return false. am i understanding it correctly? then what's the purpose of checking if tokens.size() == 1? I wonder if it is better to parallel the condition checks e.g. if (size == 1) {} else { return false; }.
orchagent/orch.cpp
Outdated
| if (type_it != type_maps.end()) | ||
| { | ||
| type_name = tokens[0]; | ||
| SWSS_LOG_INFO("Incomplete reference received: type_name:%s, object_name: missing\n", tokens[0].c_str()); |
There was a problem hiding this comment.
no need to add '\n' in the log. for the rest of the code not in this pull request that has '\n', it could be removed later.
|
retest this please |
|
hi wenda, the correct config model here is to delete the attribute. |
…ueue or a list of queues; Add the parsing and processing logic in qosorch to support such an operation Signed-off-by: Wenda <wenni@microsoft.com>
modified: orchagent/qosorch.cpp
Signed-off-by: Wenda <wenni@microsoft.com>
or a list of queues Signed-off-by: Wenda <wenni@microsoft.com>
|
hdel is not supported. Use hset with special value '[]' to indicate the user want to unbind the wred profile from a queue |
|
If you design the special case '[|]', the implementation will be simplified. In reply to: 413722729 [](ancestors = 413722729) |
|
I am not sure if user would be confused by this symbol for the sake of implementation convenience. What does @lguohan think? The symbol was chosen by him yesterday. |
|
I susgest use '[]' |
orchagent/orch.cpp
Outdated
| // clear both type_name and object_name | ||
| // as an indication to the caller that | ||
| // such a case has been encountered | ||
| SWSS_LOG_INFO("special reference:%s.", ref_in.c_str()); |
There was a problem hiding this comment.
remove this log, not necessary.
orchagent/orch.cpp
Outdated
| sai_object = (*(type_maps[ref_type_name]))[object_name]; | ||
| if (ref_type_name.empty() && object_name.empty()) | ||
| { | ||
| sai_object = SAI_NULL_OBJECT_ID; |
There was a problem hiding this comment.
you can return ref_resolve_status::empty here.
|
i have minor comments, looks good. |
Signed-off-by: Wenda <wenni@microsoft.com>
|
retest this please |
|
retest this please |
2 similar comments
|
retest this please |
|
retest this please |
The approach is to use "[]" as a CLI signal to unbind wred profile from a queue or a list of queues; Add the parsing and processing logic in qosorch to support such an operation.
With the correct SAI support, we can see asic_db is correctly set to "SAI_QUEUE_ATTR_WRED_PROFILE_ID":"oid:0x0"
"ASIC_STATE:SAI_OBJECT_TYPE_QUEUE:oid:0x15000000000285":{"type":"hash","value":
{"SAI_QUEUE_ATTR_BUFFER_PROFILE_ID":"oid:0x190000000005d6","SAI_QUEUE_ATTR_WRED_PROFILE_ID":"oid:0x0","NULL":"NULL","SAI_QUEUE_ATTR_TYPE":"SAI_QUEUE_TYPE_UNICAST"}}
What I did
Why I did it
How I verified it
Tested on dut
Details if related