Fix SNMP dropping some of the queue counter when create_only_config_d… #303

DavidZagury · 2023-12-20T10:36:32Z

…b_buffers is set to true

This happened because the MIB assumed that half the queues configured are for mcast. When create_only_config_db_buffers is set to true this is not the case

- What I did
The ciscoSwitchQosMIB MIB assumed that all the counter are configured and that half of the configured queues are for mcast.
This is no longer true, the feature "polling only configured ports buffer queue" make it possible for port to not have MC counters.
This wrong assumption caused issue sonic-net/sonic-buildimage#17448

To fix this, I instead used the BUFFER_MAX_PARAM_TABLE to find the max possible queues
- How I did it

- How to verify it
Inside the SNMP docker run snmp walk:
$ snmpwalk -v2c -c msft 10.64.247.240 1.3.6.1.4.1.9.9.580.1.5.5.1.4.1
Check that the results is not missing counters from any queue

- Description for the changelog

…b_buffers is set to true This happened because the MIB assumed that half the queues configured are for mcast. To remove this assumpetion we will check the max number of queues from STATE_DB.BUFFER_MAX_PARAM_TABLE|<port>.max_queues

…8 and not (16 + 1 ) % 2 = 9

dprital · 2023-12-25T09:22:31Z

@qiluo-msft , Can you please review ?

qiluo-msft · 2023-12-26T23:44:35Z

        Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_queue_tables, self.db_conn)

I see a bug inside init_sync_d_queue_tables. Instead of spliting a queue_name, we can use "COUNTERS_QUEUE_PORT_MAP", "COUNTERS_QUEUE_INDEX_MAP" to get port index, queue index. #Pending

Refers to: src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py:92 in 3d13de4. [](commit_id = 3d13de4, deletion_comment = False)

tests/mock_tables/counters_db.json

…plete UC and incomplete MC.

dprital · 2023-12-27T17:33:20Z

@qiluo-msft , Can you please set the label of : "Request for 202311 branch" ?

src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py

tests/test_queues_stat.py

src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py

Add UT for multi asic

DavidZagury · 2024-01-01T09:56:10Z

        Namespace.get_sync_d_from_all_namespace(mibs.init_sync_d_queue_tables, self.db_conn)
I see a bug inside init_sync_d_queue_tables. Instead of spliting a queue_name, we can use "COUNTERS_QUEUE_PORT_MAP", "COUNTERS_QUEUE_INDEX_MAP" to get port index, queue index.

Refers to: src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py:92 in 3d13de4. [](commit_id = 3d13de4, deletion_comment = False)

Hi @qiluo-msft - I don't see what is the issue you are seeing here. And I don't know how could we get the port index only by using these tables withouth COUNTERS_QUEUE_NAME_MAP or at lest COUNTERS_PORT_NAME_MAP.
Any way, this code is not touched by this PR, if there is an issue with this, shouldn't we address it in a different PR?

dgsudharsan · 2024-01-02T17:14:11Z

@qiluo-msft @StormLiangMS Can you please add label for 202305?

dprital · 2024-01-04T08:20:55Z

@qiluo-msft , can you please review and approve ?

qiluo-msft

LGTM, please check with other active reviewers.

tests/mock_tables/global_db/state_db.json

StormLiangMS · 2024-01-06T04:39:21Z

@dgsudharsan @DavidZagury could you address comment from Suvarna?

StormLiangMS · 2024-01-06T04:49:16Z

@DavidZagury @dgsudharsan pls test this PR with 202305 image.

…b_buffers is set to true (sonic-net#303) This happened because the MIB assumed that half the queues configured are for mcast. When create_only_config_db_buffers is set to true this is not the case **- What I did** The ciscoSwitchQosMIB MIB assumed that all the counter are configured and that half of the configured queues are for mcast. This is no longer true, the feature "polling only configured ports buffer queue" make it possible for port to not have MC counters. This wrong assumption caused issue sonic-net/sonic-buildimage#17448 To fix this, I instead used the BUFFER_MAX_PARAM_TABLE to find the max possible queues **- How I did it** **- How to verify it** Inside the SNMP docker run snmp walk: $ snmpwalk -v2c -c msft 10.64.247.240 1.3.6.1.4.1.9.9.580.1.5.5.1.4.1 Check that the results is not missing counters from any queue

mssonicbld · 2024-01-09T21:57:17Z

Cherry-pick PR to 202205: #305

…b_buffers is set to true (#303) (#305) This happened because the MIB assumed that half the queues configured are for mcast. When create_only_config_db_buffers is set to true this is not the case **- What I did** The ciscoSwitchQosMIB MIB assumed that all the counter are configured and that half of the configured queues are for mcast. This is no longer true, the feature "polling only configured ports buffer queue" make it possible for port to not have MC counters. This wrong assumption caused issue sonic-net/sonic-buildimage#17448 To fix this, I instead used the BUFFER_MAX_PARAM_TABLE to find the max possible queues **- How I did it** **- How to verify it** Inside the SNMP docker run snmp walk: $ snmpwalk -v2c -c msft 10.64.247.240 1.3.6.1.4.1.9.9.580.1.5.5.1.4.1 Check that the results is not missing counters from any queue Co-authored-by: DavidZagury <[email protected]>

… when create_only_config_db_buffers is set to true (#303)"

… when create_only_config_db_buffers is set to true (#303)" (#308)

…b_buffers is set to true (sonic-net#303) This happened because the MIB assumed that half the queues configured are for mcast. When create_only_config_db_buffers is set to true this is not the case **- What I did** The ciscoSwitchQosMIB MIB assumed that all the counter are configured and that half of the configured queues are for mcast. This is no longer true, the feature "polling only configured ports buffer queue" make it possible for port to not have MC counters. This wrong assumption caused issue sonic-net/sonic-buildimage#17448 To fix this, I instead used the BUFFER_MAX_PARAM_TABLE to find the max possible queues **- How I did it** **- How to verify it** Inside the SNMP docker run snmp walk: $ snmpwalk -v2c -c msft 10.64.247.240 1.3.6.1.4.1.9.9.580.1.5.5.1.4.1 Check that the results is not missing counters from any queue

mssonicbld · 2024-01-20T01:27:21Z

Cherry-pick PR to 202305: #309

…b_buffers is set to true (#303) (#309) This happened because the MIB assumed that half the queues configured are for mcast. When create_only_config_db_buffers is set to true this is not the case **- What I did** The ciscoSwitchQosMIB MIB assumed that all the counter are configured and that half of the configured queues are for mcast. This is no longer true, the feature "polling only configured ports buffer queue" make it possible for port to not have MC counters. This wrong assumption caused issue sonic-net/sonic-buildimage#17448 To fix this, I instead used the BUFFER_MAX_PARAM_TABLE to find the max possible queues **- How I did it** **- How to verify it** Inside the SNMP docker run snmp walk: $ snmpwalk -v2c -c msft 10.64.247.240 1.3.6.1.4.1.9.9.580.1.5.5.1.4.1 Check that the results is not missing counters from any queue Co-authored-by: DavidZagury <[email protected]>

DavidZagury · 2024-01-23T11:24:48Z

@yxieca can we merge this to 202311?

…b_buffers is set to true (sonic-net#303) This happened because the MIB assumed that half the queues configured are for mcast. When create_only_config_db_buffers is set to true this is not the case **- What I did** The ciscoSwitchQosMIB MIB assumed that all the counter are configured and that half of the configured queues are for mcast. This is no longer true, the feature "polling only configured ports buffer queue" make it possible for port to not have MC counters. This wrong assumption caused issue sonic-net/sonic-buildimage#17448 To fix this, I instead used the BUFFER_MAX_PARAM_TABLE to find the max possible queues **- How I did it** **- How to verify it** Inside the SNMP docker run snmp walk: $ snmpwalk -v2c -c msft 10.64.247.240 1.3.6.1.4.1.9.9.580.1.5.5.1.4.1 Check that the results is not missing counters from any queue

mssonicbld · 2024-01-23T16:34:27Z

Cherry-pick PR to 202311: #310

…b_buffers is set to true (#303) This happened because the MIB assumed that half the queues configured are for mcast. When create_only_config_db_buffers is set to true this is not the case **- What I did** The ciscoSwitchQosMIB MIB assumed that all the counter are configured and that half of the configured queues are for mcast. This is no longer true, the feature "polling only configured ports buffer queue" make it possible for port to not have MC counters. This wrong assumption caused issue sonic-net/sonic-buildimage#17448 To fix this, I instead used the BUFFER_MAX_PARAM_TABLE to find the max possible queues **- How I did it** **- How to verify it** Inside the SNMP docker run snmp walk: $ snmpwalk -v2c -c msft 10.64.247.240 1.3.6.1.4.1.9.9.580.1.5.5.1.4.1 Check that the results is not missing counters from any queue

## Description of PR This test is checking that buffer queue counters inside snmp docker are inline when using create_only_config_db_counters optimization. Summary: Accompanies "Fix SNMP dropping some of the queue counter when create_only_config_db_buffers is set to true" (sonic-net/sonic-snmpagent#303) which fixes the issue: "The feature "polling only configured ports buffer queue" will break SNMP" (sonic-net/sonic-buildimage#17448). ### Type of change - [ ] Bug fix - [ ] Testbed and Framework(new/improvement) - [x] Test case(new/improvement) ## Approach #### What is the motivation for this PR? To enhance the bug fix mentioned above, solving an issue with buffer queue counters optimization. #### How did you do it? - Set "create_only_config_db_buffers" to true in config db, to create only relevant counters - Remove one of the buffer queues, Ethernet0|3-4 is chosen arbitrary - Using snmpwalk compare number of queue counters on Ethernet0, assuming there will be 8 less after removing the buffer. (Assuming unicast only, 4 counters for each queue in this case) #### How did you verify/test it? Run the test multiple times on various setups. Run the test while reverting the fix mentioned above to see the different result. ### Supported testbed topology if it's a new test case? Any

DavidZagury added 5 commits December 18, 2023 10:25

Use max queues

71f9e81

Add max_queues value to unit test state db

04c1e36

Fix UT - add missing DB values and fix pq_count - should be 16 % 2 = …

d62f739

…8 and not (16 + 1 ) % 2 = 9

Add UT for port that doe

3d13de4

qiluo-msft requested review from SuvarnaMeenakshi and bingwang-ms December 26, 2023 22:33

qiluo-msft reviewed Dec 27, 2023

View reviewed changes

tests/mock_tables/counters_db.json Show resolved Hide resolved

Add UT for port that with an with incomplete MC and a port with incom…

f45e746

…plete UC and incomplete MC.

qiluo-msft added Request for 202311 Branch Request for 202205 Branch labels Dec 27, 2023

mssonicbld added the Cherry Pick Conflict_202205 label Dec 27, 2023

SuvarnaMeenakshi reviewed Dec 28, 2023

View reviewed changes

src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py Outdated Show resolved Hide resolved

SuvarnaMeenakshi reviewed Dec 28, 2023

View reviewed changes

src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py Outdated Show resolved Hide resolved

SuvarnaMeenakshi reviewed Dec 28, 2023

View reviewed changes

tests/test_queues_stat.py Show resolved Hide resolved

SuvarnaMeenakshi reviewed Dec 28, 2023

View reviewed changes

src/sonic_ax_impl/mibs/vendor/cisco/ciscoSwitchQosMIB.py Show resolved Hide resolved

DavidZagury added 2 commits December 28, 2023 16:05

Add support for multi-asic

aecfeb0

Add support for multi asic system

ab65bd0

Add UT for multi asic

mssonicbld removed the Cherry Pick Conflict_202205 label Dec 28, 2023

StormLiangMS added the Request for 202305 Branch label Jan 6, 2024

qiluo-msft previously approved these changes Jan 6, 2024

View reviewed changes

SuvarnaMeenakshi reviewed Jan 6, 2024

View reviewed changes

tests/mock_tables/global_db/state_db.json Outdated Show resolved Hide resolved

mssonicbld mentioned this pull request Jan 9, 2024

[action] [PR:303] Fix SNMP dropping some of the queue counter when create_only_config_db_buffers is set to true (#303) #305

Merged

mssonicbld added the Created PR to 202205 Branch label Jan 9, 2024

StormLiangMS added a commit that referenced this pull request Jan 19, 2024

Revert "[action] [PR:303] Fix SNMP dropping some of the queue counter…

eae87ff

… when create_only_config_db_buffers is set to true (#303)"

StormLiangMS added a commit that referenced this pull request Jan 19, 2024

Revert "[action] [PR:303] Fix SNMP dropping some of the queue counter…

2efaf2e

… when create_only_config_db_buffers is set to true (#303)" (#308)

StormLiangMS added Approved for 202305 Branch and removed Included in 202305 Branch Approved for 202305 Branch labels Jan 20, 2024

mssonicbld mentioned this pull request Jan 20, 2024

[action] [PR:303] Fix SNMP dropping some of the queue counter when create_only_config_db_buffers is set to true (#303) #309

Merged

mssonicbld added the Created PR to 202305 Branch label Jan 20, 2024

yxieca added the Approved for 202311 Branch label Jan 23, 2024

mssonicbld mentioned this pull request Jan 23, 2024

[action] [PR:303] Fix SNMP dropping some of the queue counter when create_only_config_db_buffers is set to true (#303) #310

Merged

mssonicbld added the Created PR to 202311 Branch label Jan 23, 2024

mssonicbld added Included in 202311 Branch and removed Created PR to 202311 Branch labels Jan 23, 2024

StormLiangMS added the Included in 202305 Branch label Feb 24, 2024

This was referenced Mar 6, 2024

Test PG counters is accurate when using create_only_config_db_buffers… arfeigin/sonic-mgmt#1

Closed

Test polling only configured buffer queue counters sonic-net/sonic-mgmt#11941

Merged

arfeigin mentioned this pull request May 6, 2024

Add buffer queue counters test case to telemetry test sonic-net/sonic-mgmt#12735

Merged

8 tasks

mssonicbld mentioned this pull request Jun 10, 2024

[action] [PR:12735] Add buffer queue counters test case to telemetry test sonic-net/sonic-mgmt#13207

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix SNMP dropping some of the queue counter when create_only_config_d… #303

Fix SNMP dropping some of the queue counter when create_only_config_d… #303

DavidZagury commented Dec 20, 2023

dprital commented Dec 25, 2023

qiluo-msft commented Dec 26, 2023 •

edited

Loading

dprital commented Dec 27, 2023

DavidZagury commented Jan 1, 2024

dgsudharsan commented Jan 2, 2024

dprital commented Jan 4, 2024

qiluo-msft left a comment

StormLiangMS commented Jan 6, 2024

StormLiangMS commented Jan 6, 2024

mssonicbld commented Jan 9, 2024

mssonicbld commented Jan 20, 2024

DavidZagury commented Jan 23, 2024

mssonicbld commented Jan 23, 2024

Fix SNMP dropping some of the queue counter when create_only_config_d… #303

Fix SNMP dropping some of the queue counter when create_only_config_d… #303

Conversation

DavidZagury commented Dec 20, 2023

dprital commented Dec 25, 2023

qiluo-msft commented Dec 26, 2023 • edited Loading

dprital commented Dec 27, 2023

DavidZagury commented Jan 1, 2024

dgsudharsan commented Jan 2, 2024

dprital commented Jan 4, 2024

qiluo-msft left a comment

Choose a reason for hiding this comment

StormLiangMS commented Jan 6, 2024

StormLiangMS commented Jan 6, 2024

mssonicbld commented Jan 9, 2024

mssonicbld commented Jan 20, 2024

DavidZagury commented Jan 23, 2024

mssonicbld commented Jan 23, 2024

qiluo-msft commented Dec 26, 2023 •

edited

Loading