[sairedis] Add get response timeout knob #774
Conversation
|
can this be set on-the-fly? |
|
yes it can |
|
retest vs please |
lib/inc/sairedis.h
Outdated
| * @flgs CREATE_AND_SET | ||
| * @default 60000 | ||
| */ | ||
| SAI_REDIS_SWITCH_ATTR_GET_RESPONSE_TIMEOUT_MS, |
There was a problem hiding this comment.
I believe it should be SET_RESPONSE_TIMEOUT_MS
There was a problem hiding this comment.
mainly this timeout was designed for GET api in any quad api, but with adding synchronous mode it's used for all of them
lib/src/Channel.cpp
Outdated
| @@ -4,9 +4,12 @@ | |||
|
|
|||
| using namespace sairedis; | |||
|
|
|||
| #define REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS (60*1000) | |||
There was a problem hiding this comment.
I would like to use this define in sonic-swss/orchagent/saihelper.cpp
Can you please move this define to .h file that can be included from both Channel.cpp and saihelper.cpp?
If there is such...
There was a problem hiding this comment.
why would you need this define on saihelper?
There was a problem hiding this comment.
I am using the new API to change the timeout and later on I want to change it back to default, this is where I want to use your define.
In the PR bellow I used new define SAI_REDIS_GET_RESPONSE_TIMEOUT_MS_DEFAULT but I would like to replace it to REDIS_ASIC_STATE_COMMAND_GETRESPONSE_TIMEOUT_MS if it is possible.
[(https://github.com/liorghub/sonic-swss/pull/3)]
There was a problem hiding this comment.
i was actually thinking, that this define maybe should not be exposed as define here, but you should do SAI api GET api, to obtain this attribute default value
There was a problem hiding this comment.
Generally, if we have set API we can have get API, basic stuff.
However, there is nothing wrong with exposing the default value in header file and for my needs it will do the work just fine. I usually vote for the option with less lines of code.
There was a problem hiding this comment.
ok i leave it as is
Knob needed by mlnx when doing firmware update on some platforms, which can exceed default 1 min timeout.
Knob needed by mlnx when doing firmware update on some platforms, which can exceed default 1 min timeout.
Knob needed by mlnx when doing firmware update on some platforms, which can exceed default 1 min timeout.
Need to be set by orchagent.