Skip to content

[QOS] Skip showing unnecessary warning message#3708

Merged
bingwang-ms merged 4 commits intosonic-net:masterfrom
vivekrnv:fix_qos_warn
Feb 10, 2025
Merged

[QOS] Skip showing unnecessary warning message#3708
bingwang-ms merged 4 commits intosonic-net:masterfrom
vivekrnv:fix_qos_warn

Conversation

@vivekrnv
Copy link
Contributor

@vivekrnv vivekrnv commented Jan 9, 2025

What I did

Don't show an unnecessary warning message (Operation not completed successfully, please save and reload configuration.) during config qos reload

To repro:

root@sonic:/home/admin# sonic-cfggen -H -k Mellanox-SN4700-O8V48 --preset t1 > /tmp/default_config.json

root@sonic:/home/admin# config qos reload
Operation not completed successfully, please save and reload configuration.

Running command: /usr/local/bin/sonic-cfggen -d -t /usr/share/sonic/device/x86_64-mlnx_msn4700-r0/Mellanox-SN4700-O8V48/buffers_dynamic.json.j2,/tmp/cfg_buffer.json -t /usr/share/sonic/device/x86_64-mlnx_msn4700-r0/Mellanox-SN4700-O8V48/qos.json.j2,/tmp/cfg_qos.json -y /etc/sonic/sonic_version.yml
Running command: /usr/local/bin/sonic-cfggen -j /tmp/cfg_buffer.json -j /tmp/cfg_qos.json --write-to-db

Buffer calculation model updated, restarting swss is required to take effect

How I did it

When in traditional buffer mode, qos reload will not wait for tables to be cleared, so it will set the timeout to 0 in the _wait_until_clear call. But today, this prints an error log and exits.

Thus fixed to not print the error log when the timeout is zero.

How to verify it

UT and manual testing

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

config/main.py Outdated
time.sleep(interval)
empty = (non_empty_table_count == 0)
if not empty:
if not empty and timeout:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest checking timeout != 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've slightly updated the logic to also return True in case timeout is 0.

This is to be compatible with the following PR: #3710

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

@bingwang-ms @kperumalbfn
can you please review following comments handling?

ayurkiv-nvda pushed a commit to ayurkiv-nvda/sonic-utilities that referenced this pull request Jan 22, 2025
stepanblyschak pushed a commit to stepanblyschak/sonic-utilities that referenced this pull request Jan 27, 2025
config/main.py Outdated
time.sleep(interval)
empty = (non_empty_table_count == 0)
if timeout == 0:
empty = True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code looks a little wired. How about checking the value of timeout and return True directly at the beginning of the function of _wait_until_clear? Or just skip calling this function _wait_until_clear when timeout == 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, Please check

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bingwang-ms Can we signoff this PR?

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@bingwang-ms bingwang-ms merged commit 13619aa into sonic-net:master Feb 10, 2025
6 checks passed
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202411: #3762

nmoray pushed a commit to nmoray/sonic-utilities that referenced this pull request Jun 25, 2025
* [QOS] Skip showing unnecessary warning message

Signed-off-by: Vivek Reddy <vkarri@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.

6 participants