Create counter for the queue to which the host CPU traffic is sent when create_only_config_db_buffers is enabled by stephenxs · Pull Request #3334 · sonic-net/sonic-swss

stephenxs · 2024-10-20T14:15:16Z

What I did

Why I did it

How I verified it
Verified the following scenarios

No BUFFER_QUEUE covering host CPU queue

       Port    TxQ    Counter/pkts    Counter/bytes    Drop/pkts    Drop/bytes
-----------  -----  --------------  ---------------  -----------  ------------
Ethernet248    UC0               0                0            0           N/A
Ethernet248    UC1               0                0            0           N/A
Ethernet248    UC2               0                0            0           N/A
Ethernet248    UC3               0                0            0           N/A
Ethernet248    UC4               0                0            0           N/A
Ethernet248    UC5               0                0            0           N/A
Ethernet248    UC6               0                0            0           N/A
Ethernet248    UC7             287            61730            0           N/A

admin@r-leopard-simx-74:~$ redis-cli -n 4
127.0.0.1:6379[4]> keys BUFFER_QUEUE*Ethernet248*
1) "BUFFER_QUEUE|Ethernet248|0-2"
2) "BUFFER_QUEUE|Ethernet248|5-6"
3) "BUFFER_QUEUE|Ethernet248|3-4"

Remove irrelevant BUFFER_QUEUE on the port

admin@r-leopard-simx-74:~$ redis-cli -n 4          
127.0.0.1:6379[4]> del BUFFER_QUEUE|Ethernet248|5-6
(integer) 1
127.0.0.1:6379[4]> 
admin@r-leopard-simx-74:~$ queuestat -p Ethernet248
       Port    TxQ    Counter/pkts    Counter/bytes    Drop/pkts    Drop/bytes
-----------  -----  --------------  ---------------  -----------  ------------
Ethernet248    UC0               0                0            0           N/A
Ethernet248    UC1               0                0            0           N/A
Ethernet248    UC2               0                0            0           N/A
Ethernet248    UC3               0                0            0           N/A
Ethernet248    UC4               0                0            0           N/A
Ethernet248    UC7             294            63022            0           N/A

Reconfig BUFFER_QEUUE covering host CPU queue

admin@r-leopard-simx-74:~$ redis-cli -n 4          
127.0.0.1:6379[4]> hset BUFFER_QUEUE|Ethernet248|5-7 profile q_lossy_profile
(integer) 1
127.0.0.1:6379[4]> exit
admin@r-leopard-simx-74:~$ queuestat -p Ethernet248
       Port    TxQ    Counter/pkts    Counter/bytes    Drop/pkts    Drop/bytes
-----------  -----  --------------  ---------------  -----------  ------------
Ethernet248    UC0               0                0            0           N/A
Ethernet248    UC1               0                0            0           N/A
Ethernet248    UC2               0                0            0           N/A
Ethernet248    UC3               0                0            0           N/A
Ethernet248    UC4               0                0            0           N/A
Ethernet248    UC5               0                0            0           N/A
Ethernet248    UC6               0                0            0           N/A
Ethernet248    UC7             298            63934            0           N/A

Remove the configured BUFFER_QUEUE

admin@r-leopard-simx-74:~$ redis-cli -n 4          
127.0.0.1:6379[4]> del BUFFER_QUEUE|Ethernet248|5-7
(integer) 1
127.0.0.1:6379[4]> exit
admin@r-leopard-simx-74:~$ queuestat -p Ethernet248
       Port    TxQ    Counter/pkts    Counter/bytes    Drop/pkts    Drop/bytes
-----------  -----  --------------  ---------------  -----------  ------------
Ethernet248    UC0               0                0            0           N/A
Ethernet248    UC1               0                0            0           N/A
Ethernet248    UC2               0                0            0           N/A
Ethernet248    UC3               0                0            0           N/A
Ethernet248    UC4               0                0            0           N/A
Ethernet248    UC7             305            64818            0           N/A

Details if related

stephenxs · 2024-10-29T02:08:34Z

Failed due to some routing test cases which is not relevant to the PR. Retrying.

stephenxs · 2024-10-29T02:08:43Z

/apzw run

stephenxs · 2024-10-29T02:33:29Z

/azpw run

mssonicbld · 2024-10-29T02:33:32Z

/AzurePipelines run

azure-pipelines · 2024-10-29T02:33:42Z

Azure Pipelines successfully started running 1 pipeline(s).

prabhataravind · 2024-11-05T16:55:54Z

My understanding is that you are setting queue counters for queue 7 (control traffic tx queue). Isn't that already available on sonic today? At least "show queue counters" seems to show queue counters on cpu tx queue already. The first output in your verifications also show q7 counters, so what does this PR change affect? @bingwang-ms for viz

stephenxs · 2024-11-05T22:45:34Z

My understanding is that you are setting queue counters for queue 7 (control traffic tx queue). Isn't that already available on sonic today? At least "show queue counters" seems to show queue counters on cpu tx queue already. The first output in your verifications also show q7 counters, so what does this PR change affect? @bingwang-ms for viz

Hi @prabhataravind
This is not accurate. There is a configuration to optimize the buffer performance DEVICE_METADATA|localhost.create_only_config_db_buffers. Using this configuration the buffer counters are created only for the queues and PGs with buffer configured for performance optimization.
However, to have counters for CPU tx queue, we have to configure it in BUFFER_QUEUE, mainly for data traffic, which is a bad coupling.
Using this PR the counter will be created for CPU tx queue without data buffer configuration.

prabhataravind · 2024-11-07T00:29:13Z

create_only_config_db_buffer
Hi @stephenxs, thanks for the clarification. I am assuming you are referring to #2883? If so, the issue should not be observed without this configuration, correct?

Could you please update the PR title/description to indicate that this handles queue counters for CPU TX queue for the specific case where create_only_config_db_buffers configuration is set?

stephenxs · 2024-11-07T03:18:20Z

create_only_config_db_buffer
Hi @stephenxs, thanks for the clarification. I am assuming you are referring to #2883? If so, the issue should not be observed without this configuration, correct?

Could you please update the PR title/description to indicate that this handles queue counters for CPU TX queue for the specific case where create_only_config_db_buffers configuration is set?

yes, I am referring to #2883 and there is no issue without the configuration.

Sure. Done.

prabhataravind

lgtm

prabhataravind · 2024-11-08T06:23:42Z

@prsunny to help merge

stephenxs · 2024-11-08T06:30:55Z

/azpw run

mssonicbld · 2024-11-08T06:30:57Z

/AzurePipelines run

azure-pipelines · 2024-11-08T06:31:07Z

Azure Pipelines successfully started running 1 pipeline(s).

bingwang-ms · 2024-11-12T00:29:06Z

/azp run Azure.sonic-swss

azure-pipelines · 2024-11-12T00:29:13Z

Azure Pipelines successfully started running 1 pipeline(s).

stephenxs · 2024-11-13T10:40:04Z

vs failure is caused by code issue. fixing.

stephenxs · 2024-11-13T14:38:34Z

vs failure is caused by code issue. fixing.

The failure was caused on vs simulating DPU platform. On that platform, the queue IDs of a port are not fetched from SAI. We should not create counters if the queue ID was not available.

stephenxs · 2024-11-15T22:24:19Z

@stephenxs Can you please sync code with master branch? Thanks!

Done. We still need to wait for sonic-net/sonic-sairedis#1459 to be taken into the VS image to pass the VS test.

stephenxs · 2024-11-17T23:14:44Z

@stephenxs Can you please sync code with master branch? Thanks!

Done. We still need to wait for sonic-net/sonic-sairedis#1459 to be taken into the VS image to pass the VS test.

The vs image hasn't been built due to the wrong ipmitool version. @bingwang-ms who can help to fix it?

mv ipmitool_1.8.19-4+deb12u1_amd64.deb ipmitool-dbgsym_1.8.19-4+deb12u1_amd64.deb /sonic/target/debs/bookworm/
dget: retrieving https://deb.debian.org/debian/pool/main/i/ipmitool/ipmitool_1.8.19-4+deb12u1.dsc
mkdir: cannot create directory '/vcache/sonic-slave-bookworm/web': Permission denied
Warning: Failed to verify the package: https://deb.debian.org/debian/pool/main/i/ipmitool/ipmitool_1.8.19-4+deb12u1.dsc, the version is not specified
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
100  1972  100  1972    0     0  30312      0 --:--:-- --:--:-- --:--:-- 30812

stephenxs · 2024-11-19T22:19:05Z

@prabhataravind @prsunny
Could you help to review and merge ?
I fixed vs test issues since last @prabhataravind approval

Thanks

stephenxs · 2024-11-21T14:28:37Z

The vs failed due to the segment failed in orchagent while setting SAI redis attribute SAI_REDIS_SWITCH_ATTR_NOTIFY_SYNCD to SAI_REDIS_NOTIFY_SYNCD_INIT_VIEW

(gdb) bt
#0  0x00007ffff6520af8 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007ffff68b2ac1 in redisvFormatCommand () from /usr/lib/x86_64-linux-gnu/libhiredis.so.0.14
#2  0x00007ffff731243d in swss::RedisCommand::format(char const*, ...) () from /usr/lib/x86_64-linux-gnu/libswsscommon.so.0
#3  0x0000555556b907f7 in swss::RedisPipeline::publish (this=0x555558ed4340) at /usr/include/swss/redispipeline.h:181
#4  0x0000555556b906b5 in swss::RedisPipeline::flush (this=0x555558ed4340) at /usr/include/swss/redispipeline.h:133
#5  0x00007ffff72f9706 in swss::ProducerTable::set(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) () from /usr/lib/x86_64-linux-gnu/libswsscommon.so.0
#6  0x00007ffff7e87b47 in sairedis::RedisChannel::set(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::vector<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) () from /usr/lib/x86_64-linux-gnu/libsairedis.so.0
#7  0x00007ffff7e0eec8 in sairedis::RedisRemoteSaiInterface::notifySyncd(unsigned long, _sai_redis_notify_syncd_t) () from /usr/lib/x86_64-linux-gnu/libsairedis.so.0
#8  0x00007ffff7e1268f in sairedis::RedisRemoteSaiInterface::sai_redis_notify_syncd(unsigned long, _sai_attribute_t const*) () from /usr/lib/x86_64-linux-gnu/libsairedis.so.0
#9  0x00007ffff7e20f06 in sairedis::RedisRemoteSaiInterface::setRedisExtensionAttribute(_sai_object_type_t, unsigned long, _sai_attribute_t const*) () from /usr/lib/x86_64-linux-gnu/libsairedis.so.0
#10 0x00007ffff7e21e88 in sairedis::RedisRemoteSaiInterface::set(_sai_object_type_t, unsigned long, _sai_attribute_t const*) () from /usr/lib/x86_64-linux-gnu/libsairedis.so.0
#11 0x00007ffff7e4d146 in sairedis::Sai::set(_sai_object_type_t, unsigned long, _sai_attribute_t const*) () from /usr/lib/x86_64-linux-gnu/libsairedis.so.0
#12 0x00007ffff7dcd586 in sairedis::ServerSai::set(_sai_object_type_t, unsigned long, _sai_attribute_t const*) () from /usr/lib/x86_64-linux-gnu/libsairedis.so.0
#13 0x00007ffff7dbbe16 in sairedis::ClientServerSai::set(_sai_object_type_t, unsigned long, _sai_attribute_t const*) () from /usr/lib/x86_64-linux-gnu/libsairedis.so.0
#14 0x00007ffff7db3bfe in ?? () from /usr/lib/x86_64-linux-gnu/libsairedis.so.0
#15 0x0000555556b48fc1 in initSaiRedis () at saihelper.cpp:382
#16 0x000055555648d6bc in main (argc=8, argv=0x7fffffffec48) at main.cpp:469

Signed-off-by: Stephen Sun <[email protected]>

…ue was not initialized Signed-off-by: Stephen Sun <[email protected]>

…en create_only_config_db_buffers is enabled (sonic-net#3334) Create counter for the queue to which the host CPU traffic is sent when create_only_config_db_buffers is enabled *There is a configuration to optimize the buffer performance DEVICE_METADATA|localhost.create_only_config_db_buffers. Using this configuration the buffer counters are created only for the queues and PGs with buffer configured for performance optimization. However, to have counters for CPU tx queue, we have to configure it in BUFFER_QUEUE, mainly for data traffic, which is a bad coupling. Using this PR the counter will be created for CPU tx queue without data buffer configuration.

mssonicbld · 2024-12-02T22:02:36Z

Cherry-pick PR to 202405: #3408

…en create_only_config_db_buffers is enabled (#3334) Create counter for the queue to which the host CPU traffic is sent when create_only_config_db_buffers is enabled *There is a configuration to optimize the buffer performance DEVICE_METADATA|localhost.create_only_config_db_buffers. Using this configuration the buffer counters are created only for the queues and PGs with buffer configured for performance optimization. However, to have counters for CPU tx queue, we have to configure it in BUFFER_QUEUE, mainly for data traffic, which is a bad coupling. Using this PR the counter will be created for CPU tx queue without data buffer configuration.

…en create_only_config_db_buffers is enabled (sonic-net#3334) Create counter for the queue to which the host CPU traffic is sent when create_only_config_db_buffers is enabled *There is a configuration to optimize the buffer performance DEVICE_METADATA|localhost.create_only_config_db_buffers. Using this configuration the buffer counters are created only for the queues and PGs with buffer configured for performance optimization. However, to have counters for CPU tx queue, we have to configure it in BUFFER_QUEUE, mainly for data traffic, which is a bad coupling. Using this PR the counter will be created for CPU tx queue without data buffer configuration.

…en create_only_config_db_buffers is enabled (sonic-net#3334) Create counter for the queue to which the host CPU traffic is sent when create_only_config_db_buffers is enabled *There is a configuration to optimize the buffer performance DEVICE_METADATA|localhost.create_only_config_db_buffers. Using this configuration the buffer counters are created only for the queues and PGs with buffer configured for performance optimization. However, to have counters for CPU tx queue, we have to configure it in BUFFER_QUEUE, mainly for data traffic, which is a bad coupling. Using this PR the counter will be created for CPU tx queue without data buffer configuration. Signed-off-by: Baorong Liu <[email protected]>

stephenxs requested a review from prsunny as a code owner October 20, 2024 14:15

DavidZagury added the Request for 202405 Branch label Oct 20, 2024

stephenxs force-pushed the create-counter-for-cpu-queue branch from 52542a4 to 8b37562 Compare October 28, 2024 10:53

prsunny requested a review from prabhataravind October 29, 2024 19:06

stephenxs force-pushed the create-counter-for-cpu-queue branch 2 times, most recently from 3fb27c0 to 3b9b430 Compare October 30, 2024 23:28

stephenxs force-pushed the create-counter-for-cpu-queue branch from 5ab8403 to 226fed5 Compare November 6, 2024 10:17

stephenxs force-pushed the create-counter-for-cpu-queue branch from 226fed5 to c2cea72 Compare November 7, 2024 03:17

stephenxs changed the title ~~Create counter for the queue to which the host CPU traffic is sent~~ Create counter for the queue to which the host CPU traffic is sent when create_only_config_db_buffers is enabled Nov 7, 2024

prabhataravind approved these changes Nov 8, 2024

View reviewed changes

stephenxs force-pushed the create-counter-for-cpu-queue branch 2 times, most recently from f04b1b7 to 27e2c9d Compare November 12, 2024 00:24

stephenxs force-pushed the create-counter-for-cpu-queue branch from 4f96093 to 8cbd3de Compare November 13, 2024 14:37

stephenxs force-pushed the create-counter-for-cpu-queue branch 2 times, most recently from aa3a9fd to 48bc625 Compare November 19, 2024 22:03

stephenxs added 3 commits November 22, 2024 13:51

Create counter for the queue to which the host CPU traffic is sent

1e62ce0

Signed-off-by: Stephen Sun <[email protected]>

Adjust vstest accordingly

545b967

Signed-off-by: Stephen Sun <[email protected]>

Do not initialize counter for host CPU queue if the corresponding que…

dcc7f56

…ue was not initialized Signed-off-by: Stephen Sun <[email protected]>

stephenxs force-pushed the create-counter-for-cpu-queue branch from e24d79d to dcc7f56 Compare November 22, 2024 05:51

prsunny approved these changes Nov 22, 2024

View reviewed changes

prsunny merged commit 3d5ac54 into sonic-net:master Nov 22, 2024

stephenxs deleted the create-counter-for-cpu-queue branch November 22, 2024 22:07

bingwang-ms added the Approved for 202405 Branch label Dec 2, 2024

mssonicbld mentioned this pull request Dec 2, 2024

[action] [PR:3334] Create counter for the queue to which the host CPU traffic is sent when create_only_config_db_buffers is enabled (#3334) #3408

Merged

mssonicbld added the Created PR to 202405 Branch label Dec 2, 2024

mssonicbld added Included in 202405 Branch and removed Created PR to 202405 Branch labels Dec 3, 2024

stephenxs mentioned this pull request Apr 12, 2025

[202411][Mellanox] Support new field supporting_bulk_counter_groups in DEVICE__METADATA|localhost for Mellanox-SN4700-V64 sonic-net/sonic-buildimage#22224

Closed

11 tasks

Conversation

stephenxs commented Oct 20, 2024

Uh oh!

stephenxs commented Oct 29, 2024

Uh oh!

stephenxs commented Oct 29, 2024

Uh oh!

stephenxs commented Oct 29, 2024

Uh oh!

mssonicbld commented Oct 29, 2024

Uh oh!

azure-pipelines bot commented Oct 29, 2024

Uh oh!

prabhataravind commented Nov 5, 2024

Uh oh!

stephenxs commented Nov 5, 2024

Uh oh!

prabhataravind commented Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stephenxs commented Nov 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

prabhataravind left a comment

Choose a reason for hiding this comment

Uh oh!

prabhataravind commented Nov 8, 2024

Uh oh!

stephenxs commented Nov 8, 2024

Uh oh!

mssonicbld commented Nov 8, 2024

Uh oh!

azure-pipelines bot commented Nov 8, 2024

Uh oh!

bingwang-ms commented Nov 12, 2024

Uh oh!

azure-pipelines bot commented Nov 12, 2024

Uh oh!

stephenxs commented Nov 13, 2024

Uh oh!

stephenxs commented Nov 13, 2024

Uh oh!

stephenxs commented Nov 15, 2024

Uh oh!

stephenxs commented Nov 17, 2024

Uh oh!

stephenxs commented Nov 19, 2024

Uh oh!

stephenxs commented Nov 21, 2024

Uh oh!

mssonicbld commented Dec 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

prabhataravind commented Nov 7, 2024 •

edited

Loading

stephenxs commented Nov 7, 2024 •

edited

Loading