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

align watermark flow with port configuration #2525

Merged
merged 1 commit into from
Dec 16, 2022

Conversation

dbarashinvd
Copy link
Contributor

@dbarashinvd dbarashinvd commented Nov 15, 2022

This PR fixes the following issue:
sonic-net/sonic-buildimage#11219

What I did
Align watermark flow with port configuration
correct the queue, watermark and pg-drop counterpoll functionality according to cli commands

Why I did it
the flow before this commit was wrong, when watermark counterpoll was enabled, stats wasn't created in FLEX_COUNTERS_DB
and COUNTERS_DB unless enabling queue counterpoll as well.
in the same way when pg-drop counterpoll was enabled stats weren't added until watermark was enabled.
This is due to the wrong flow where queue and pg maps were generated only when queue or watermark (pg-watermark)
key was detected in flexcountersorch.cpp

How I verified it
manual testing
VS test (updated to reflect current flow) - before this commit only queue was tested
regression test

Details if related

@lgtm-com
Copy link

lgtm-com bot commented Nov 15, 2022

This pull request introduces 1 alert when merging 50281fb into 28aa309 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

stephenxs
stephenxs previously approved these changes Nov 17, 2022
@liat-grozovik
Copy link
Collaborator

@marian-pritsak could you please help to review and approve?
@dbarashinvd can we merge without a test? can you please share the public PR test update of that?

@dbarashinvd
Copy link
Contributor Author

dbarashinvd commented Nov 28, 2022

@marian-pritsak could you please help to review and approve? @dbarashinvd can we merge without a test? can you please share the public PR test update of that?

This is the verification test
sonic-net/sonic-mgmt#6907

@prsunny prsunny merged commit b865352 into sonic-net:master Dec 16, 2022
StormLiangMS pushed a commit to StormLiangMS/sonic-swss that referenced this pull request Dec 27, 2022
*Align watermark flow with port configuration correct the queue, watermark and pg-drop counterpoll functionality according to cli commands
StormLiangMS pushed a commit that referenced this pull request Dec 27, 2022
*Align watermark flow with port configuration correct the queue, watermark and pg-drop counterpoll functionality according to cli commands
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Dec 28, 2022
Update sonic-swss submodule pointer to include the following:
* 782a2ef Align watermark flow with port configuration ([sonic-net#2525](sonic-net/sonic-swss#2525))
* dca78d8 [Fdbsyncd] Bug Fix for remote MAC move to local MAC and Fix for Static MAC advertisement in EVPN. ([sonic-net#2521](sonic-net/sonic-swss#2521))
* 28aa309 [fpm] Fix FpmLink to read all netlink messages from FPM message ([sonic-net#2492](sonic-net/sonic-swss#2492))

Signed-off-by: dprital <[email protected]>
liat-grozovik pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Dec 29, 2022
Update sonic-swss submodule pointer to include the following:
* 782a2ef Align watermark flow with port configuration ([#2525](sonic-net/sonic-swss#2525))
* dca78d8 [Fdbsyncd] Bug Fix for remote MAC move to local MAC and Fix for Static MAC advertisement in EVPN. ([#2521](sonic-net/sonic-swss#2521))
* 28aa309 [fpm] Fix FpmLink to read all netlink messages from FPM message ([#2492](sonic-net/sonic-swss#2492))

Signed-off-by: dprital <[email protected]>
@rlhui
Copy link
Contributor

rlhui commented Feb 15, 2023

should this be cherry-picked to 202205 too, as the issue was in 202111?

@dbarashinvd
Copy link
Contributor Author

yes Rita we're working on cherry picking it with conflicts for 202205

liat-grozovik pushed a commit that referenced this pull request Feb 23, 2023
This PR fixes the following issue:
sonic-net/sonic-buildimage#11219
It's a cherry-pick (with conflicts resolution) for 202205 of the following PR: #2525

- What I did
Align watermark flow with port configuration
correct the queue, watermark and pg-drop counterpoll functionality according to cli commands

- Why I did it
the flow before this commit was wrong, when watermark counterpoll was enabled, stats wasn't created in FLEX_COUNTERS_DB
and COUNTERS_DB unless enabling queue counterpoll as well.
in the same way when pg-drop counterpoll was enabled stats weren't added until watermark was enabled.
This is due to the wrong flow where queue and pg maps were generated only when queue or watermark (pg-watermark)
key was detected in flexcountersorch.cpp

- How I verified it
manual testing
VS test (updated to reflect current flow) - before this commit only queue was tested
regression test
@wenyiz2021
Copy link

requesting for 202205 as sonic-net/sonic-buildimage#11219 is intended for 202205 and 202211

@yxieca
Copy link
Contributor

yxieca commented Mar 1, 2023

@dbarashinvd please open separate PR for 202205 branch.

@stephenxs
Copy link
Collaborator

@dbarashinvd please open separate PR for 202205 branch.

@yxieca FYI. #2672 has been created for 202205 backporting. It has been merged.

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.