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

[orchagent] Increase SAI REDIS response timeout to support FW upgrade during init (Mellanox only) #1637

Merged
merged 1 commit into from
Feb 22, 2021

Conversation

liorghub
Copy link
Contributor

@liorghub liorghub commented Feb 11, 2021

Increase orchagent SAI REDIS response timeout to support FW upgrade during init (Mellanox only).
This change should be merged after the following PRs were merged:
sonic-net/sonic-sairedis#774

Signed-off-by: liora liora@nvidia.com

What I did
Increase orchagent SAI REDIS response timeout during init in order for orchagent to wait enough time for response from syncd since in init, syncd startup script first calls FW upgrade script (that might take up to 7 minutes in systems with Gearbox) and only then launches syncd.

Why I did it
To support FW upgrade during init flow.

How I verified it
I manually changed ASIC and Gearbox FW followed by hard reset in order for FW upgrade to take place on init.

Which release branch to backport

  • 202012

@liorghub
Copy link
Contributor Author

@liat-grozovik Looks like build environment issue, can you re run please?

@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 1637 in repo Azure/sonic-swss

@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 1637 in repo Azure/sonic-swss

@kcudnik
Copy link
Contributor

kcudnik commented Feb 16, 2021

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

retest vs please

1 similar comment
@kcudnik
Copy link
Contributor

kcudnik commented Feb 16, 2021

retest vs please

@liorghub
Copy link
Contributor Author

@kcudnik @liat-grozovik

ARM build is failing.
Unfortunately I couldn’t find the root cause in log, however I noticed a gap of 30 seconds in log (see below).

2021-02-16T20:58:03.3481304Z __i = _M_t._M_emplace_hint_unique(__i, std::piecewise_construct,
2021-02-16T20:58:33.9199964Z make[3]: Leaving directory '/__w/2/s/orchagent'

Did you see it before?
Maybe rerun will help?

@kcudnik
Copy link
Contributor

kcudnik commented Feb 17, 2021

im looking to some recent build like this: #1643, all checks are passing, and i see a lot of similar warnings during compilation: https://dev.azure.com/mssonic/build/_build/results?buildId=4187&view=logs&jobId=e51ebb60-45fa-5846-0822-3aaf597a9c80&j=e51ebb60-45fa-5846-0822-3aaf597a9c80&t=22830a29-09d4-5764-f143-c6b729e2f8db which takes over 2h but completes fine

as for your actual error comes from here:

021-02-16T20:53:58.7599011Z In file included from saihelper.cpp:12:
2021-02-16T20:53:58.7600911Z saihelper.cpp: In function 'void initSaiRedis(const string&, const string&)':
2021-02-16T20:53:58.7602862Z /usr/include/swss/logger.h:17:101: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'sai_uint64_t' {aka 'long long unsigned int'} [-Werror=format=]
2021-02-16T20:53:58.7605002Z  #define SWSS_LOG_NOTICE(MSG, ...)      swss::Logger::getInstance().write(swss::Logger::SWSS_NOTICE, ":- %s: " MSG, __FUNCTION__, ##__VA_ARGS__)
2021-02-16T20:53:58.7606582Z saihelper.cpp:300:9: note: in expansion of macro 'SWSS_LOG_NOTICE'
2021-02-16T20:53:58.7607669Z          SWSS_LOG_NOTICE("SAI REDIS response timeout set successfully to %lu", attr.value.u64);
2021-02-16T20:53:58.7608412Z          ^~~~~~~~~~~~~~~
2021-02-16T20:53:58.7716910Z /usr/include/swss/logger.h:17:101: error: format '%lu' expects argument of type 'long unsigned int', but argument 5 has type 'sai_uint64_t' {aka 'long long unsigned int'} [-Werror=format=]
2021-02-16T20:53:58.7718978Z  #define SWSS_LOG_NOTICE(MSG, ...)      swss::Logger::getInstance().write(swss::Logger::SWSS_NOTICE, ":- %s: " MSG, __FUNCTION__, ##__VA_ARGS__)
2021-02-16T20:53:58.7721025Z saihelper.cpp:327:9: note: in expansion of macro 'SWSS_LOG_NOTICE'
2021-02-16T20:53:58.7721824Z          SWSS_LOG_NOTICE("SAI REDIS response timeout set successfully to %lu", attr.value.u64);
2021-02-16T20:53:58.7722503Z          ^~~~~~~~~~~~~~~

for cross arm compilation you need to use PRIu64 prefixes instead of "%lu"

as for warning described here: https://stackoverflow.com/questions/48149323/what-does-the-gcc-warning-project-parameter-passing-for-x-changed-in-gcc-7-1-m
i think we can pass "-Wno-psabi" on arm to not pollute compilation logs to better see whats heppening

… during init (Mellanox only).

Signed-off-by: liora <liora@nvidia.com>
@liorghub
Copy link
Contributor Author

@kcudnik Kamil thank you so much, I fixed the PRIu64 thing.
Actually I also looked on other logs that passed and saw all the warnings but those are not failing the compilation and I have no masochistic desires to add changes in build system :)

Can you please rerun for me?

@liorghub
Copy link
Contributor Author

@kcudnik Please merge and backport to 202012.
I verified clean cherry-pick. I also verified that related commits are already there.

@dprital
Copy link
Collaborator

dprital commented Feb 22, 2021

Hi @kcudnik - can you please merge it and also backport to 202012 ?

@kcudnik kcudnik merged commit 16cd6c2 into sonic-net:master Feb 22, 2021
@liorghub
Copy link
Contributor Author

@liat-grozovik can you please add backport to 202012 tag?

@liat-grozovik
Copy link
Collaborator

no i cannot.
@daall your help is needed. I cannot add the label to this repository.

@liorghub
Copy link
Contributor Author

liorghub commented Mar 3, 2021

@yxieca Hello Ying, I couldn't find this commit in swss 202012, please advise.

@yxieca
Copy link
Contributor

yxieca commented Mar 3, 2021

@liorghub the submodule update PR just got merged. You should be able to see it now.

yxieca pushed a commit that referenced this pull request Mar 3, 2021
… during init (Mellanox only). (#1637)

Signed-off-by: liora <liora@nvidia.com>

Co-authored-by: liora <liora@nvidia.com>
DavidZagury pushed a commit to DavidZagury/sonic-swss that referenced this pull request Mar 4, 2021
… during init (Mellanox only). (sonic-net#1637)

Signed-off-by: liora <liora@nvidia.com>

Co-authored-by: liora <liora@nvidia.com>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
… during init (Mellanox only). (sonic-net#1637)

Signed-off-by: liora <liora@nvidia.com>

Co-authored-by: liora <liora@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants